From 30f1f1f347b0898d8f4894cd654d3f7a59ceca24 Mon Sep 17 00:00:00 2001 From: thsparks Date: Fri, 4 Apr 2025 11:19:47 -0700 Subject: [PATCH 01/91] Remove block error list as an experiment --- localtypings/pxtarget.d.ts | 2 +- pxteditor/experiments.ts | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index cfcf2a1a26e9..5c682fdb8141 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -503,7 +503,7 @@ declare namespace pxt { tutorialExplicitHints?: boolean; // allow use explicit hints errorList?: boolean; // error list experiment embedBlocksInSnapshot?: boolean; // embed blocks xml in right-click snapshot - blocksErrorList?: boolean; // blocks error list experiment + blocksErrorList?: boolean; // blocks error list identity?: boolean; // login with identity providers assetEditor?: boolean; // enable asset editor view (in blocks/text toggle) disableMemoryWorkspaceWarning?: boolean; // do not warn the user when switching to in memory workspace diff --git a/pxteditor/experiments.ts b/pxteditor/experiments.ts index a9724f2dd874..4c874452f7cd 100644 --- a/pxteditor/experiments.ts +++ b/pxteditor/experiments.ts @@ -172,11 +172,6 @@ export function all(): Experiment[] { name: lf("Error List"), description: lf("Show an error list panel for JavaScript and Python.") }, - { - id: "blocksErrorList", - name: lf("Blocks Error List"), - description: lf("Show an error list panel for Blocks") - }, { id: "timeMachine", name: lf("Time Machine"), From 233a237478ca45f229056bddeaf7c607358a4b70 Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 7 Apr 2025 10:55:25 -0700 Subject: [PATCH 02/91] Some block error list refactoring --- webapp/src/blocks.tsx | 21 +++++++++++++------ webapp/src/errorList.tsx | 44 ++++++++++++++++++++++++---------------- webapp/src/monaco.tsx | 2 +- 3 files changed, 42 insertions(+), 25 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index f44418d304d9..b873ae83620a 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -23,7 +23,7 @@ import { WorkspaceSearch } from "@blockly/plugin-workspace-search"; import Util = pxt.Util; import { DebuggerToolbox } from "./debuggerToolbox"; -import { ErrorList } from "./errorList"; +import { BlockError, ErrorList } from "./errorList"; import { resolveExtensionUrl } from "./extensionManager"; import { experiments, initEditorExtensionsAsync } from "../../pxteditor"; @@ -63,7 +63,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { breakpointsSet: number[]; // the IDs of the breakpoints set. currentFlyoutKey: string; - private errorChangesListeners: pxt.Map<(errors: pxtblockly.BlockDiagnostic[]) => void> = {}; + private errorChangesListeners: pxt.Map<(errors: BlockError[]) => void> = {}; protected intersectionObserver: IntersectionObserver; protected debuggerToolbox: DebuggerToolbox; @@ -881,7 +881,7 @@ export class Editor extends toolboxeditor.ToolboxEditor {
- {showErrorList && } ) @@ -891,11 +891,11 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.parent.fireResize(); } - listenToBlockErrorChanges(handlerKey: string, handler: (errors: pxtblockly.BlockDiagnostic[]) => void) { + listenToBlockErrorChanges(handlerKey: string, handler: (errors: BlockError[]) => void) { this.errorChangesListeners[handlerKey] = handler; } - private onBlockErrorChanges(errors: pxtblockly.BlockDiagnostic[]) { + private onBlockErrorChanges(errors: BlockError[]) { for (let listener of pxt.U.values(this.errorChangesListeners)) { listener(errors) } @@ -1138,6 +1138,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (!tsfile || !tsfile.diagnostics) return; // only show errors + const allErrors: BlockError[] = []; let diags = tsfile.diagnostics.filter(d => d.category == ts.pxtc.DiagnosticCategory.Error); let sourceMap = this.compilationResult.sourceMap; @@ -1149,6 +1150,10 @@ export class Editor extends toolboxeditor.ToolboxEditor { let txt = ts.pxtc.flattenDiagnosticMessageText(diag.messageText, "\n"); b.setWarningText(txt, pxtblockly.PXT_WARNING_ID); setHighlightWarningAsync(b, true); + allErrors.push({ + blockId: bid, + message: txt, + }) } } }) @@ -1160,10 +1165,14 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (b) { b.setWarningText(d.message, pxtblockly.PXT_WARNING_ID); setHighlightWarningAsync(b, true); + allErrors.push({ + blockId: d.blockId, + message: d.message, + }) } } }) - this.onBlockErrorChanges(this.compilationResult.diagnostics); + this.onBlockErrorChanges(allErrors); this.setBreakpointsFromBlocks(); } diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index f9a9e36221b3..03a565f0d416 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -2,6 +2,7 @@ import * as React from "react"; import * as sui from "./sui"; +import * as blocks from "./blocks"; import { fireClickOnEnter } from "./util"; import * as pxtblockly from "../../pxtblocks"; @@ -12,11 +13,18 @@ type GroupedError = { index: number }; +// TODO thsparks : Move into a different file? +export type BlockError = { + blockId: string; + message: string; +} + export interface ErrorListProps { + parent: pxt.editor.IProjectView; // TODO thsparks : Do we need the full parent? isInBlocksEditor: boolean; onSizeChange?: (state: pxt.editor.ErrorListState) => void; listenToErrorChanges?: (key: string, onErrorChanges: (errors: pxtc.KsDiagnostic[]) => void) => void; - listenToBlockErrorChanges?: (key: string, onErrorChanges: (errors: pxtblockly.BlockDiagnostic[]) => void) => void; + listenToBlockErrorChanges?: (key: string, onErrorChanges: (errors: BlockError[]) => void) => void; listenToExceptionChanges?: (handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) => void, goToError?: (errorLocation: pxtc.LocationInfo) => void; startDebugger?: () => void; @@ -26,7 +34,7 @@ export interface ErrorListState { errors?: pxtc.KsDiagnostic[], exception?: pxsim.DebuggerBreakpointMessage, callLocations?: pxtc.LocationInfo[], - blockErrors?: pxtblockly.BlockDiagnostic[] + blockErrors?: BlockError[] } export class ErrorList extends React.Component { @@ -150,7 +158,7 @@ export class ErrorList extends React.Component { const grouped = groupErrors(errors); return
- {grouped.map((e, index) => )} + {grouped.map((e, index) => this.onErrorMessageClick(e.error, index)} />)}
} @@ -163,37 +171,42 @@ export class ErrorList extends React.Component { if (!location) return null; - return + return this.onErrorMessageClick(location, index)} /> })} ; } - listBlockErrors(blockErrors: pxtblockly.BlockDiagnostic[]) { + listBlockErrors(blockErrors: BlockError[]) { return
- {(blockErrors || []).map((e, i) => )} + {(blockErrors || []).map((e, i) => this.focusOnBlock(e.blockId))} />)}
} + + focusOnBlock(blockId: string) { + if(!this.props.parent.isBlocksActive()) return; + + // TODO thsparks : This should probably be moved out into an editor function (maybe one each editor implements itself). + const blocksEditor = this.props.parent.editor as blocks.Editor; + blocksEditor?.editor?.centerOnBlock(blockId); + } } interface ErrorListItemProps { index: number; - revealError?: (location: pxtc.LocationInfo, index: number) => void; + onClick?: () => void; error?: GroupedError; stackframe?: pxsim.StackFrameInfo; location?: pxtc.LocationInfo; - blockError?: pxtblockly.BlockDiagnostic; + blockError?: BlockError; } interface ErrorListItemState { } - class ErrorListItem extends React.Component { constructor(props: ErrorListItemProps) { - super(props) - - this.onErrorListItemClick = this.onErrorListItemClick.bind(this) + super(props); } render() { @@ -205,18 +218,13 @@ class ErrorListItem extends React.Component {message} {(errorCount <= 1) ? null :
{errorCount}
} } - - onErrorListItemClick() { - const location = this.props.stackframe ? this.props.location : this.props.error.error - this.props.revealError(location, this.props.index) - } } function errorMessageStringWithLineNumber(error: pxtc.KsDiagnostic) { diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index 837b01ccdc9c..b1e6153eec25 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -655,7 +655,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { setInsertionSnippet={this.setInsertionSnippet} parent={this.parent} /> - {showErrorList && } From ba54683d2fa4104998be583ea0b3877b3264023e Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 16 Apr 2025 16:26:18 -0700 Subject: [PATCH 03/91] Show runtime exceptions in blocks error panel --- webapp/src/blocks.tsx | 18 +++++++++++++++++- webapp/src/errorList.tsx | 27 +++++++++++++++++---------- 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index b873ae83620a..eaba0ebda6e5 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -64,6 +64,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { currentFlyoutKey: string; private errorChangesListeners: pxt.Map<(errors: BlockError[]) => void> = {}; + private exceptionChangesListeners: pxt.Map<(exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void> = {}; protected intersectionObserver: IntersectionObserver; protected debuggerToolbox: DebuggerToolbox; @@ -79,6 +80,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { super(parent); this.listenToBlockErrorChanges = this.listenToBlockErrorChanges.bind(this) + this.listenToExceptionChanges = this.listenToExceptionChanges.bind(this) this.onErrorListResize = this.onErrorListResize.bind(this) } setBreakpointsMap(breakpoints: pxtc.Breakpoint[], procCallLocations: pxtc.LocationInfo[]): void { @@ -881,7 +883,11 @@ export class Editor extends toolboxeditor.ToolboxEditor {
- {showErrorList && } ) @@ -901,6 +907,16 @@ export class Editor extends toolboxeditor.ToolboxEditor { } } + listenToExceptionChanges(handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) { + this.exceptionChangesListeners[handlerKey] = handler; + } + + public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { + for (let listener of pxt.U.values(this.exceptionChangesListeners)) { + listener(exception, undefined); + } + } + getBlocksAreaDiv() { return document.getElementById('blocksArea'); } diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 03a565f0d416..0be366466ab4 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -63,6 +63,9 @@ export class ErrorList extends React.Component { props.listenToBlockErrorChanges("errorList", this.onBlockErrorsChanged) } else { props.listenToErrorChanges("errorList", this.onErrorsChanged); + } + + if (props.listenToExceptionChanges) { props.listenToExceptionChanges("errorList", this.onExceptionDetected); } } @@ -72,10 +75,10 @@ export class ErrorList extends React.Component { const errorsAvailable = !!errors?.length || !!exception || !!blockErrors?.length; const collapseTooltip = lf("Collapse Error List"); - const errorListContent = !isCollapsed ? (this.props.isInBlocksEditor ? this.listBlockErrors(blockErrors) - : (exception ? this.generateStackTraces(exception) : this.getCompilerErrors(errors))) : undefined; + const errorListContent = !isCollapsed ? (exception ? this.generateStackTraces(exception) + : (this.props.isInBlocksEditor ? this.listBlockErrors(blockErrors) : this.getCompilerErrors(errors))) : undefined; - const errorCount = this.props.isInBlocksEditor ? blockErrors.length : exception ? 1 : errors.length; + const errorCount = exception ? 1 : this.props.isInBlocksEditor ? blockErrors.length : errors.length; return ( ) @@ -897,26 +898,47 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.parent.fireResize(); } - listenToBlockErrorChanges(handlerKey: string, handler: (errors: BlockError[]) => void) { - this.errorChangesListeners[handlerKey] = handler; + listenToErrors(handlerKey: string, handler: (errors: ErrorDisplayInfo[]) => void) { + this.errorListeners[handlerKey] = handler; } private onBlockErrorChanges(errors: BlockError[]) { - for (let listener of pxt.U.values(this.errorChangesListeners)) { - listener(errors) + const displayInfo = errors.map(this.getDisplayInfoForBlockError); + for (let listener of pxt.U.values(this.errorListeners)) { + listener(displayInfo); } } - listenToExceptionChanges(handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) { - this.exceptionChangesListeners[handlerKey] = handler; + public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { + const displayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); + for (let listener of pxt.U.values(this.errorListeners)) { + listener([displayInfo]); + } } - public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { - for (let listener of pxt.U.values(this.exceptionChangesListeners)) { - listener(exception, undefined); + private getDisplayInfoForBlockError(error: BlockError): ErrorDisplayInfo { + return { + message: error.message, + onClick: () => this.editor.centerOnBlock(error.blockId) } } + // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. + private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { + const message = pxt.Util.rlf(exception.exceptionMessage); + const childItems: ErrorDisplayInfo[] = exception.stackframes?.map(frame => { + return { + message: lf("at {0}", frame.funcInfo.functionName), + // TODO thsparks : Can I get the block id from the frame for an onClick? + }; + }) ?? undefined; + + return { + message, + childItems + }; + } + getBlocksAreaDiv() { return document.getElementById('blocksArea'); } diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 0be366466ab4..3a19bd139243 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -2,39 +2,29 @@ import * as React from "react"; import * as sui from "./sui"; -import * as blocks from "./blocks"; import { fireClickOnEnter } from "./util"; - -import * as pxtblockly from "../../pxtblocks"; +import { classList } from "../../react-common/components/util"; type GroupedError = { - error: pxtc.KsDiagnostic, + error: ErrorDisplayInfo, count: number, index: number }; -// TODO thsparks : Move into a different file? -export type BlockError = { - blockId: string; - message: string; -} +export type ErrorDisplayInfo = { + message: string, + childItems?: ErrorDisplayInfo[], + onClick?: () => void +}; export interface ErrorListProps { - parent: pxt.editor.IProjectView; // TODO thsparks : Do we need the full parent? - isInBlocksEditor: boolean; onSizeChange?: (state: pxt.editor.ErrorListState) => void; - listenToErrorChanges?: (key: string, onErrorChanges: (errors: pxtc.KsDiagnostic[]) => void) => void; - listenToBlockErrorChanges?: (key: string, onErrorChanges: (errors: BlockError[]) => void) => void; - listenToExceptionChanges?: (handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) => void, - goToError?: (errorLocation: pxtc.LocationInfo) => void; + listenToErrors?: (key: string, onErrors: (errors: ErrorDisplayInfo[]) => void) => void; startDebugger?: () => void; } export interface ErrorListState { isCollapsed: boolean, - errors?: pxtc.KsDiagnostic[], - exception?: pxsim.DebuggerBreakpointMessage, - callLocations?: pxtc.LocationInfo[], - blockErrors?: BlockError[] + errors?: ErrorDisplayInfo[], } export class ErrorList extends React.Component { @@ -44,66 +34,52 @@ export class ErrorList extends React.Component { this.state = { isCollapsed: true, - errors: [], - exception: null, - blockErrors: [] + errors: [] } - this.onCollapseClick = this.onCollapseClick.bind(this) - this.onErrorsChanged = this.onErrorsChanged.bind(this) - this.onBlockErrorsChanged = this.onBlockErrorsChanged.bind(this) - this.onExceptionDetected = this.onExceptionDetected.bind(this) - this.onErrorMessageClick = this.onErrorMessageClick.bind(this) - this.onDisplayStateChange = this.onDisplayStateChange.bind(this) - this.getCompilerErrors = this.getCompilerErrors.bind(this) - this.generateStackTraces = this.generateStackTraces.bind(this) - this.listBlockErrors = this.listBlockErrors.bind(this) + this.onCollapseClick = this.onCollapseClick.bind(this); + this.onErrorsChanged = this.onErrorsChanged.bind(this); - if (props.isInBlocksEditor) { - props.listenToBlockErrorChanges("errorList", this.onBlockErrorsChanged) - } else { - props.listenToErrorChanges("errorList", this.onErrorsChanged); - } - - if (props.listenToExceptionChanges) { - props.listenToExceptionChanges("errorList", this.onExceptionDetected); - } + props.listenToErrors("errorList", this.onErrorsChanged); } render() { - const { isCollapsed, errors, exception, blockErrors } = this.state; - const errorsAvailable = !!errors?.length || !!exception || !!blockErrors?.length; + const { startDebugger } = this.props; + const { isCollapsed, errors } = this.state; + const errorsAvailable = !!errors?.length; const collapseTooltip = lf("Collapse Error List"); - const errorListContent = !isCollapsed ? (exception ? this.generateStackTraces(exception) - : (this.props.isInBlocksEditor ? this.listBlockErrors(blockErrors) : this.getCompilerErrors(errors))) : undefined; - - const errorCount = exception ? 1 : this.props.isInBlocksEditor ? blockErrors.length : errors.length; + const groupedErrors = groupErrors(errors); + const errorListContent = !isCollapsed ? groupedErrors.map((e, i) => ) : undefined; + const errorCount = errors.length; return ( - - {showErrorList && } ) } - listenToExceptionChanges(handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) { - this.exceptionChangesListeners[handlerKey] = handler; + listenToErrors(handlerKey: string, handler: (errors: ErrorDisplayInfo[]) => void) { + this.errorListeners[handlerKey] = handler; } public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { - for (let listener of pxt.U.values(this.exceptionChangesListeners)) { - listener(exception, this.callLocations); + const exceptionDisplayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); + for (let listener of pxt.U.values(this.errorListeners)) { + listener([exceptionDisplayInfo]); } } - listenToErrorChanges(handlerKey: string, handler: (errors: pxtc.KsDiagnostic[]) => void) { - this.errorChangesListeners[handlerKey] = handler; + private onErrorChanges(errors: pxtc.KsDiagnostic[]) { + const errorDisplayInfo: ErrorDisplayInfo[] = errors.map(this.getDisplayInfoForError); + for (let listener of pxt.U.values(this.errorListeners)) { + listener(errorDisplayInfo); + } + } + + // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. + private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { + const message = pxt.Util.rlf(exception.exceptionMessage); + + const childItems: ErrorDisplayInfo[] = exception.stackframes?.map(frame => { + const location = this.callLocations[frame.callLocationId]; + if (!location) return undefined; + return { + message: lf("at {0} (line {1})", frame.funcInfo.functionName, location.line + 1), + onClick: () => this.goToError(location) + }; + }).filter(a => !!a) ?? undefined; + + return { + message, + childItems + }; + } + + // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. + private getDisplayInfoForError(error: pxtc.KsDiagnostic): ErrorDisplayInfo { + const message = lf("Line {0}: {1}", error.endLine ? error.endLine + 1 : error.line + 1, error.messageText); + return { + message, + onClick: () => this.goToError(error) + }; } goToError(error: pxtc.LocationInfo) { @@ -700,12 +731,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.parent.toggleDebugging() } - private onErrorChanges(errors: pxtc.KsDiagnostic[]) { - for (let listener of pxt.U.values(this.errorChangesListeners)) { - listener(errors); - } - } - public showPackageDialog() { pxt.tickEvent("monaco.addpackage", undefined, { interactiveConsent: true }); this.hideFlyout(); From 74327f9fb5e11a3e1505ee9e1eaf5a6bb24cd5c5 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 17 Apr 2025 16:27:10 -0700 Subject: [PATCH 05/91] Fix up display of errors with stack frames --- theme/errorList.less | 4 +++- webapp/src/blocks.tsx | 6 +++--- webapp/src/errorList.tsx | 27 +++++++++++++++++---------- webapp/src/monaco.tsx | 6 +++--- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/theme/errorList.less b/theme/errorList.less index 999599ae9c8d..562634a1d16a 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -63,13 +63,15 @@ // errors .ui.selection.list { color: var(--pxt-target-foreground1); - cursor: pointer; margin: 0; + padding: 0; .item { border-radius: unset; padding: 0.5em 1em; color: var(--pxt-target-foreground1); + cursor: pointer; + line-height: 1.2rem; &:hover, &:focus { background-color: var(--pxt-neutral-alpha20) !important; diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 35285519e633..84b52d9b7d9f 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -23,7 +23,7 @@ import { WorkspaceSearch } from "@blockly/plugin-workspace-search"; import Util = pxt.Util; import { DebuggerToolbox } from "./debuggerToolbox"; -import { ErrorDisplayInfo, ErrorList } from "./errorList"; +import { ErrorDisplayInfo, ErrorList, StackFrameDisplayInfo } from "./errorList"; import { resolveExtensionUrl } from "./extensionManager"; import { experiments, initEditorExtensionsAsync } from "../../pxteditor"; @@ -926,7 +926,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { const message = pxt.Util.rlf(exception.exceptionMessage); - const childItems: ErrorDisplayInfo[] = exception.stackframes?.map(frame => { + const stackFrames: StackFrameDisplayInfo[] = exception.stackframes?.map(frame => { return { message: lf("at {0}", frame.funcInfo.functionName), // TODO thsparks : Can I get the block id from the frame for an onClick? @@ -935,7 +935,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { return { message, - childItems + stackFrames }; } diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 3a19bd139243..6c24d0ed9c25 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -11,9 +11,14 @@ type GroupedError = { index: number }; +export type StackFrameDisplayInfo = { + message: string, + onClick?: () => void, +} + export type ErrorDisplayInfo = { message: string, - childItems?: ErrorDisplayInfo[], + stackFrames?: StackFrameDisplayInfo[], onClick?: () => void }; @@ -110,6 +115,7 @@ export class ErrorList extends React.Component { interface ErrorListItemProps { index: number; errorGroup: GroupedError; + className?: string; } interface ErrorListItemState { @@ -121,32 +127,33 @@ class ErrorListItem extends React.Component + return error.stackFrames ? ( +
+ tabIndex={0} + role={error.onClick ? "button" : undefined}> {error.message}
- {(error.childItems).map((childErr, index) => { + {(error.stackFrames).map((childErr, index) => { const errGrp = {error: childErr, count: 1, index: 0}; - return + return })}
) : ( -
{message} {(errorGroup.count <= 1) ? null :
{errorGroup.count}
}
@@ -177,7 +184,7 @@ function groupErrors(errors: ErrorDisplayInfo[]): GroupedError[] { function getErrorKey(error: ErrorDisplayInfo): string { let key = error.message; - for (const child of error.childItems || []) { + for (const child of error.stackFrames || []) { key += getErrorKey(child); } return key; diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index 07b46724d91b..0017c6bc315c 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -22,7 +22,7 @@ import { DebuggerToolbox } from "./debuggerToolbox"; import { amendmentToInsertSnippet, listenForEditAmendments, createLineReplacementPyAmendment } from "./monacoEditAmendments"; import { MonacoFlyout } from "./monacoFlyout"; -import { ErrorList, ErrorDisplayInfo as ErrorDisplayInfo } from "./errorList"; +import { ErrorList, ErrorDisplayInfo as ErrorDisplayInfo, StackFrameDisplayInfo } from "./errorList"; import * as auth from "./auth"; import * as pxteditor from "../../pxteditor"; @@ -685,7 +685,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { const message = pxt.Util.rlf(exception.exceptionMessage); - const childItems: ErrorDisplayInfo[] = exception.stackframes?.map(frame => { + const stackFrames: StackFrameDisplayInfo[] = exception.stackframes?.map(frame => { const location = this.callLocations[frame.callLocationId]; if (!location) return undefined; return { @@ -696,7 +696,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { return { message, - childItems + stackFrames }; } From 5fdc6369e473779c3e93f3b6fde69a6ad58c3110 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 17 Apr 2025 16:43:55 -0700 Subject: [PATCH 06/91] Fix when "debug this project" is shown --- webapp/src/errorList.tsx | 3 +- webapp/src/errroListRef.tsx | 264 ++++++++++++++++++++++++++++++++++++ 2 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 webapp/src/errroListRef.tsx diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 6c24d0ed9c25..6029024fcada 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -57,6 +57,7 @@ export class ErrorList extends React.Component { const groupedErrors = groupErrors(errors); const errorListContent = !isCollapsed ? groupedErrors.map((e, i) => ) : undefined; const errorCount = errors.length; + const canDebug = startDebugger && !!errors.find(a => a.stackFrames?.length); return ( {!isCollapsed &&
- {startDebugger &&
+ {canDebug &&
{lf("Debug this project")}
} diff --git a/webapp/src/errroListRef.tsx b/webapp/src/errroListRef.tsx new file mode 100644 index 000000000000..ec935f2247b4 --- /dev/null +++ b/webapp/src/errroListRef.tsx @@ -0,0 +1,264 @@ +/// + +import * as React from "react"; +import * as sui from "./sui"; +import * as blocks from "./blocks"; +import { fireClickOnEnter } from "./util"; + +import * as pxtblockly from "../../pxtblocks"; + +type GroupedError = { + error: pxtc.KsDiagnostic, + count: number, + index: number +}; + +// TODO thsparks : Move into a different file? +export type BlockError = { + blockId: string; + message: string; +} + +export interface ErrorListProps { + parent: pxt.editor.IProjectView; // TODO thsparks : Do we need the full parent? + isInBlocksEditor: boolean; + onSizeChange?: (state: pxt.editor.ErrorListState) => void; + listenToErrorChanges?: (key: string, onErrorChanges: (errors: pxtc.KsDiagnostic[]) => void) => void; + listenToBlockErrorChanges?: (key: string, onErrorChanges: (errors: BlockError[]) => void) => void; + listenToExceptionChanges?: (handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) => void, + goToError?: (errorLocation: pxtc.LocationInfo) => void; + startDebugger?: () => void; +} +export interface ErrorListState { + isCollapsed: boolean, + errors?: pxtc.KsDiagnostic[], + exception?: pxsim.DebuggerBreakpointMessage, + callLocations?: pxtc.LocationInfo[], + blockErrors?: BlockError[] +} + +export class ErrorList2 extends React.Component { + + constructor(props: ErrorListProps) { + super(props); + + this.state = { + isCollapsed: true, + errors: [], + exception: null, + blockErrors: [] + } + + this.onCollapseClick = this.onCollapseClick.bind(this) + this.onErrorsChanged = this.onErrorsChanged.bind(this) + this.onBlockErrorsChanged = this.onBlockErrorsChanged.bind(this) + this.onExceptionDetected = this.onExceptionDetected.bind(this) + this.onErrorMessageClick = this.onErrorMessageClick.bind(this) + this.onDisplayStateChange = this.onDisplayStateChange.bind(this) + this.getCompilerErrors = this.getCompilerErrors.bind(this) + this.generateStackTraces = this.generateStackTraces.bind(this) + this.listBlockErrors = this.listBlockErrors.bind(this) + + if (props.isInBlocksEditor) { + props.listenToBlockErrorChanges("errorList", this.onBlockErrorsChanged) + } else { + props.listenToErrorChanges("errorList", this.onErrorsChanged); + } + + if (props.listenToExceptionChanges) { + props.listenToExceptionChanges("errorList", this.onExceptionDetected); + } + } + + render() { + const { isCollapsed, errors, exception, blockErrors } = this.state; + const errorsAvailable = !!errors?.length || !!exception || !!blockErrors?.length; + const collapseTooltip = lf("Collapse Error List"); + + const errorListContent = !isCollapsed ? (exception ? this.generateStackTraces(exception) + : (this.props.isInBlocksEditor ? this.listBlockErrors(blockErrors) : this.getCompilerErrors(errors))) : undefined; + + const errorCount = exception ? 1 : this.props.isInBlocksEditor ? blockErrors.length : errors.length; + + return ( + + ) + } + + onDisplayStateChange() { + const { errors, exception, isCollapsed } = this.state; + // notify parent on possible size change so siblings (monaco) + // can resize if needed + + const noValueToDisplay = !(errors?.length || exception); + + if (this.props.onSizeChange) { + this.props.onSizeChange( + noValueToDisplay ? + undefined + : isCollapsed ? + pxt.editor.ErrorListState.HeaderOnly + : pxt.editor.ErrorListState.Expanded + ); + } + } + + onCollapseClick() { + pxt.tickEvent('errorlist.collapse', null, { interactiveConsent: true }) + this.setState({ + isCollapsed: !this.state.isCollapsed + }, this.onDisplayStateChange); + } + + onErrorMessageClick(e: pxtc.LocationInfo, index: number) { + if (this.props.goToError) { + pxt.tickEvent('errorlist.goto', { errorIndex: index }, { interactiveConsent: true }); + this.props.goToError(e); + } + } + + onErrorsChanged(errors: pxtc.KsDiagnostic[]) { + this.setState({ + errors, + isCollapsed: errors?.length == 0 || this.state.isCollapsed, + exception: null + }, this.onDisplayStateChange); + } + + onBlockErrorsChanged(blockErrors: pxtblockly.BlockDiagnostic[]) { + this.setState({ + blockErrors, + isCollapsed: blockErrors?.length == 0 || this.state.isCollapsed, + exception: null + }, this.onDisplayStateChange); + } + + onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage, callLocations: pxtc.LocationInfo[]) { + this.setState({ + isCollapsed: false, + exception, + callLocations + }) + } + + getCompilerErrors(errors: pxtc.KsDiagnostic[]) { + function errorKey(error: pxtc.KsDiagnostic): string { + // React likes have a "key" for each element so that it can smartly only + // re-render what changes. Think of it like a hashcode/ + return `${error.messageText}-${error.fileName}-${error.line}-${error.column}` + } + + const grouped = groupErrors(errors); + return
+ {grouped.map((e, index) => this.onErrorMessageClick(e.error, index)} />)} +
+ } + + generateStackTraces(exception: pxsim.DebuggerBreakpointMessage) { + return
+
{pxt.Util.rlf(exception.exceptionMessage)}
+
+ {(exception.stackframes || []).map((sf, index) => { + const location = this.state.callLocations?.[sf.callLocationId]; + + return this.onErrorMessageClick(location, index)} /> + })} +
+
; + } + + listBlockErrors(blockErrors: BlockError[]) { + return
+ {(blockErrors || []).map((e, i) => this.focusOnBlock(e.blockId))} />)} +
+ } + + focusOnBlock(blockId: string) { + if(!this.props.parent.isBlocksActive()) return; + + // TODO thsparks : This should probably be moved out into an editor function (maybe one each editor implements itself). + const blocksEditor = this.props.parent.editor as blocks.Editor; + blocksEditor?.editor?.centerOnBlock(blockId); + } +} + +interface ErrorListItemProps { + index: number; + onClick?: () => void; + error?: GroupedError; + stackframe?: pxsim.StackFrameInfo; + location?: pxtc.LocationInfo; + blockError?: BlockError; +} + +interface ErrorListItemState { +} + +class ErrorListItem extends React.Component { + constructor(props: ErrorListItemProps) { + super(props); + } + + render() { + const { error, stackframe, location, blockError } = this.props + + const message = blockError ? lf("{0}", blockError.message) + : stackframe ? stackFrameMessageStringWithLineNumber(stackframe, location) : + errorMessageStringWithLineNumber(error.error); + const errorCount = (stackframe || blockError) ? 1 : error.count; + + return
+ {message} {(errorCount <= 1) ? null :
{errorCount}
} +
+ } +} + +function errorMessageStringWithLineNumber(error: pxtc.KsDiagnostic) { + return lf("Line {0}: {1}", error.endLine ? error.endLine + 1 : error.line + 1, error.messageText); +} + +function stackFrameMessageStringWithLineNumber(stackframe: pxsim.StackFrameInfo, location?: pxtc.LocationInfo) { + if (!location) { + return lf("at {0}", stackframe.funcInfo.functionName); + } else { + return lf("at {0} (line {1})", stackframe.funcInfo.functionName, location.line + 1); + } +} + +function groupErrors(errors: pxtc.KsDiagnostic[]) { + const grouped = new Map(); + let index = 0; + for (const error of errors) { + const key = errorMessageStringWithLineNumber(error); + if (!grouped.has(key)) { + grouped.set(key, { + index: index++, + count: 1, + error + }); + } + else { + grouped.get(key).count++; + } + } + const sorted: GroupedError[] = []; + grouped.forEach(value => sorted.push(value)); + return sorted.sort((a, b) => a.index - b.index); +} From 2e342cdfd3aaab5cc65e66c4220a46390469067c Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 17 Apr 2025 16:46:43 -0700 Subject: [PATCH 07/91] Remove errorListRef file, committed by mistake --- webapp/src/errroListRef.tsx | 264 ------------------------------------ 1 file changed, 264 deletions(-) delete mode 100644 webapp/src/errroListRef.tsx diff --git a/webapp/src/errroListRef.tsx b/webapp/src/errroListRef.tsx deleted file mode 100644 index ec935f2247b4..000000000000 --- a/webapp/src/errroListRef.tsx +++ /dev/null @@ -1,264 +0,0 @@ -/// - -import * as React from "react"; -import * as sui from "./sui"; -import * as blocks from "./blocks"; -import { fireClickOnEnter } from "./util"; - -import * as pxtblockly from "../../pxtblocks"; - -type GroupedError = { - error: pxtc.KsDiagnostic, - count: number, - index: number -}; - -// TODO thsparks : Move into a different file? -export type BlockError = { - blockId: string; - message: string; -} - -export interface ErrorListProps { - parent: pxt.editor.IProjectView; // TODO thsparks : Do we need the full parent? - isInBlocksEditor: boolean; - onSizeChange?: (state: pxt.editor.ErrorListState) => void; - listenToErrorChanges?: (key: string, onErrorChanges: (errors: pxtc.KsDiagnostic[]) => void) => void; - listenToBlockErrorChanges?: (key: string, onErrorChanges: (errors: BlockError[]) => void) => void; - listenToExceptionChanges?: (handlerKey: string, handler: (exception: pxsim.DebuggerBreakpointMessage, locations: pxtc.LocationInfo[]) => void) => void, - goToError?: (errorLocation: pxtc.LocationInfo) => void; - startDebugger?: () => void; -} -export interface ErrorListState { - isCollapsed: boolean, - errors?: pxtc.KsDiagnostic[], - exception?: pxsim.DebuggerBreakpointMessage, - callLocations?: pxtc.LocationInfo[], - blockErrors?: BlockError[] -} - -export class ErrorList2 extends React.Component { - - constructor(props: ErrorListProps) { - super(props); - - this.state = { - isCollapsed: true, - errors: [], - exception: null, - blockErrors: [] - } - - this.onCollapseClick = this.onCollapseClick.bind(this) - this.onErrorsChanged = this.onErrorsChanged.bind(this) - this.onBlockErrorsChanged = this.onBlockErrorsChanged.bind(this) - this.onExceptionDetected = this.onExceptionDetected.bind(this) - this.onErrorMessageClick = this.onErrorMessageClick.bind(this) - this.onDisplayStateChange = this.onDisplayStateChange.bind(this) - this.getCompilerErrors = this.getCompilerErrors.bind(this) - this.generateStackTraces = this.generateStackTraces.bind(this) - this.listBlockErrors = this.listBlockErrors.bind(this) - - if (props.isInBlocksEditor) { - props.listenToBlockErrorChanges("errorList", this.onBlockErrorsChanged) - } else { - props.listenToErrorChanges("errorList", this.onErrorsChanged); - } - - if (props.listenToExceptionChanges) { - props.listenToExceptionChanges("errorList", this.onExceptionDetected); - } - } - - render() { - const { isCollapsed, errors, exception, blockErrors } = this.state; - const errorsAvailable = !!errors?.length || !!exception || !!blockErrors?.length; - const collapseTooltip = lf("Collapse Error List"); - - const errorListContent = !isCollapsed ? (exception ? this.generateStackTraces(exception) - : (this.props.isInBlocksEditor ? this.listBlockErrors(blockErrors) : this.getCompilerErrors(errors))) : undefined; - - const errorCount = exception ? 1 : this.props.isInBlocksEditor ? blockErrors.length : errors.length; - - return ( - - ) - } - - onDisplayStateChange() { - const { errors, exception, isCollapsed } = this.state; - // notify parent on possible size change so siblings (monaco) - // can resize if needed - - const noValueToDisplay = !(errors?.length || exception); - - if (this.props.onSizeChange) { - this.props.onSizeChange( - noValueToDisplay ? - undefined - : isCollapsed ? - pxt.editor.ErrorListState.HeaderOnly - : pxt.editor.ErrorListState.Expanded - ); - } - } - - onCollapseClick() { - pxt.tickEvent('errorlist.collapse', null, { interactiveConsent: true }) - this.setState({ - isCollapsed: !this.state.isCollapsed - }, this.onDisplayStateChange); - } - - onErrorMessageClick(e: pxtc.LocationInfo, index: number) { - if (this.props.goToError) { - pxt.tickEvent('errorlist.goto', { errorIndex: index }, { interactiveConsent: true }); - this.props.goToError(e); - } - } - - onErrorsChanged(errors: pxtc.KsDiagnostic[]) { - this.setState({ - errors, - isCollapsed: errors?.length == 0 || this.state.isCollapsed, - exception: null - }, this.onDisplayStateChange); - } - - onBlockErrorsChanged(blockErrors: pxtblockly.BlockDiagnostic[]) { - this.setState({ - blockErrors, - isCollapsed: blockErrors?.length == 0 || this.state.isCollapsed, - exception: null - }, this.onDisplayStateChange); - } - - onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage, callLocations: pxtc.LocationInfo[]) { - this.setState({ - isCollapsed: false, - exception, - callLocations - }) - } - - getCompilerErrors(errors: pxtc.KsDiagnostic[]) { - function errorKey(error: pxtc.KsDiagnostic): string { - // React likes have a "key" for each element so that it can smartly only - // re-render what changes. Think of it like a hashcode/ - return `${error.messageText}-${error.fileName}-${error.line}-${error.column}` - } - - const grouped = groupErrors(errors); - return
- {grouped.map((e, index) => this.onErrorMessageClick(e.error, index)} />)} -
- } - - generateStackTraces(exception: pxsim.DebuggerBreakpointMessage) { - return
-
{pxt.Util.rlf(exception.exceptionMessage)}
-
- {(exception.stackframes || []).map((sf, index) => { - const location = this.state.callLocations?.[sf.callLocationId]; - - return this.onErrorMessageClick(location, index)} /> - })} -
-
; - } - - listBlockErrors(blockErrors: BlockError[]) { - return
- {(blockErrors || []).map((e, i) => this.focusOnBlock(e.blockId))} />)} -
- } - - focusOnBlock(blockId: string) { - if(!this.props.parent.isBlocksActive()) return; - - // TODO thsparks : This should probably be moved out into an editor function (maybe one each editor implements itself). - const blocksEditor = this.props.parent.editor as blocks.Editor; - blocksEditor?.editor?.centerOnBlock(blockId); - } -} - -interface ErrorListItemProps { - index: number; - onClick?: () => void; - error?: GroupedError; - stackframe?: pxsim.StackFrameInfo; - location?: pxtc.LocationInfo; - blockError?: BlockError; -} - -interface ErrorListItemState { -} - -class ErrorListItem extends React.Component { - constructor(props: ErrorListItemProps) { - super(props); - } - - render() { - const { error, stackframe, location, blockError } = this.props - - const message = blockError ? lf("{0}", blockError.message) - : stackframe ? stackFrameMessageStringWithLineNumber(stackframe, location) : - errorMessageStringWithLineNumber(error.error); - const errorCount = (stackframe || blockError) ? 1 : error.count; - - return
- {message} {(errorCount <= 1) ? null :
{errorCount}
} -
- } -} - -function errorMessageStringWithLineNumber(error: pxtc.KsDiagnostic) { - return lf("Line {0}: {1}", error.endLine ? error.endLine + 1 : error.line + 1, error.messageText); -} - -function stackFrameMessageStringWithLineNumber(stackframe: pxsim.StackFrameInfo, location?: pxtc.LocationInfo) { - if (!location) { - return lf("at {0}", stackframe.funcInfo.functionName); - } else { - return lf("at {0} (line {1})", stackframe.funcInfo.functionName, location.line + 1); - } -} - -function groupErrors(errors: pxtc.KsDiagnostic[]) { - const grouped = new Map(); - let index = 0; - for (const error of errors) { - const key = errorMessageStringWithLineNumber(error); - if (!grouped.has(key)) { - grouped.set(key, { - index: index++, - count: 1, - error - }); - } - else { - grouped.get(key).count++; - } - } - const sorted: GroupedError[] = []; - grouped.forEach(value => sorted.push(value)); - return sorted.sort((a, b) => a.index - b.index); -} From 7cbc80695f263279a9c811eab51916fde579d4eb Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 17 Apr 2025 17:51:28 -0700 Subject: [PATCH 08/91] Remove unused variable --- webapp/src/errorList.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 6029024fcada..aab57f230dac 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -52,7 +52,6 @@ export class ErrorList extends React.Component { const { startDebugger } = this.props; const { isCollapsed, errors } = this.state; const errorsAvailable = !!errors?.length; - const collapseTooltip = lf("Collapse Error List"); const groupedErrors = groupErrors(errors); const errorListContent = !isCollapsed ? groupedErrors.map((e, i) => ) : undefined; From f0ba3f5dd53e020628e851b394c2ec52767db924 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 17 Apr 2025 17:52:14 -0700 Subject: [PATCH 09/91] Pass errors in as props rather than using a listener function --- webapp/src/blocks.tsx | 17 ++++---------- webapp/src/errorList.tsx | 50 ++++++++++++++++++++++------------------ webapp/src/monaco.tsx | 17 ++++---------- 3 files changed, 36 insertions(+), 48 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 84b52d9b7d9f..2412a24486b2 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -67,8 +67,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { breakpointsSet: number[]; // the IDs of the breakpoints set. currentFlyoutKey: string; - private errorListeners: pxt.Map<(errors: ErrorDisplayInfo[]) => void> = {}; protected intersectionObserver: IntersectionObserver; + private errors: ErrorDisplayInfo[] = []; protected debuggerToolbox: DebuggerToolbox; protected highlightedStatement: pxtc.LocationInfo; @@ -82,7 +82,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { constructor(parent: IProjectView) { super(parent); - this.listenToErrors = this.listenToErrors.bind(this) this.onErrorListResize = this.onErrorListResize.bind(this) this.getDisplayInfoForBlockError = this.getDisplayInfoForBlockError.bind(this) this.getDisplayInfoForException = this.getDisplayInfoForException.bind(this) @@ -888,7 +887,7 @@ export class Editor extends toolboxeditor.ToolboxEditor {
{showErrorList && }
) @@ -898,22 +897,14 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.parent.fireResize(); } - listenToErrors(handlerKey: string, handler: (errors: ErrorDisplayInfo[]) => void) { - this.errorListeners[handlerKey] = handler; - } - private onBlockErrorChanges(errors: BlockError[]) { const displayInfo = errors.map(this.getDisplayInfoForBlockError); - for (let listener of pxt.U.values(this.errorListeners)) { - listener(displayInfo); - } + this.errors = displayInfo; } public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { const displayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); - for (let listener of pxt.U.values(this.errorListeners)) { - listener([displayInfo]); - } + this.errors = [displayInfo]; } private getDisplayInfoForBlockError(error: BlockError): ErrorDisplayInfo { diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index aab57f230dac..85b59d76a8f2 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -24,12 +24,11 @@ export type ErrorDisplayInfo = { export interface ErrorListProps { onSizeChange?: (state: pxt.editor.ErrorListState) => void; - listenToErrors?: (key: string, onErrors: (errors: ErrorDisplayInfo[]) => void) => void; + errors: ErrorDisplayInfo[]; startDebugger?: () => void; } export interface ErrorListState { isCollapsed: boolean, - errors?: ErrorDisplayInfo[], } export class ErrorList extends React.Component { @@ -38,19 +37,31 @@ export class ErrorList extends React.Component { super(props); this.state = { - isCollapsed: true, - errors: [] + isCollapsed: true } this.onCollapseClick = this.onCollapseClick.bind(this); - this.onErrorsChanged = this.onErrorsChanged.bind(this); + } + + componentDidUpdate(prevProps: Readonly, prevState: Readonly, snapshot?: any): void { + // Auto-expand if there are new errors + if (this.props.errors.length > 0 && this.state.isCollapsed) { + let shouldExpand = this.props.errors.length > prevProps.errors.length; + + if (!shouldExpand) { + // Compare errors to see if there are new ones + shouldExpand = this.props.errors.some(e => !prevProps.errors.some(prev => getErrorKey(e) === getErrorKey(prev))); + } - props.listenToErrors("errorList", this.onErrorsChanged); + if (shouldExpand) { + this.setState({ isCollapsed: false }, this.onDisplayStateChange); + } + } } render() { - const { startDebugger } = this.props; - const { isCollapsed, errors } = this.state; + const { startDebugger, errors } = this.props; + const { isCollapsed } = this.state; const errorsAvailable = !!errors?.length; const groupedErrors = groupErrors(errors); @@ -80,7 +91,8 @@ export class ErrorList extends React.Component { } onDisplayStateChange() { - const { errors, isCollapsed } = this.state; + const { errors } = this.props; + const { isCollapsed } = this.state; // notify parent on possible size change so siblings (monaco) // can resize if needed @@ -98,16 +110,14 @@ export class ErrorList extends React.Component { } onCollapseClick() { - pxt.tickEvent('errorlist.collapse', null, { interactiveConsent: true }) - this.setState({ - isCollapsed: !this.state.isCollapsed - }, this.onDisplayStateChange); - } + if (this.state.isCollapsed) { + pxt.tickEvent('errorlist.expand', null, { interactiveConsent: true }) + } else { + pxt.tickEvent('errorlist.collapse', null, { interactiveConsent: true }) + } - onErrorsChanged(errors: ErrorDisplayInfo[]) { this.setState({ - errors, - isCollapsed: errors?.length == 0 || this.state.isCollapsed, + isCollapsed: !this.state.isCollapsed }, this.onDisplayStateChange); } } @@ -183,9 +193,5 @@ function groupErrors(errors: ErrorDisplayInfo[]): GroupedError[] { } function getErrorKey(error: ErrorDisplayInfo): string { - let key = error.message; - for (const child of error.stackFrames || []) { - key += getErrorKey(child); - } - return key; + return error.message + (error.stackFrames ? error.stackFrames.map(f => f.message).join('') : ''); } \ No newline at end of file diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index 0017c6bc315c..824060240cf4 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -366,7 +366,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { private highlightDecorations: string[] = []; private highlightedBreakpoint: number; private editAmendmentsListener: monaco.IDisposable | undefined; - private errorListeners: pxt.Map<(errors: ErrorDisplayInfo[]) => void> = {}; + private errors: ErrorDisplayInfo[] = []; private callLocations: pxtc.LocationInfo[]; private userPreferencesSubscriber: data.DataSubscriber = { @@ -383,7 +383,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { super(parent); this.setErrorListState = this.setErrorListState.bind(this); - this.listenToErrors = this.listenToErrors.bind(this); this.getDisplayInfoForException = this.getDisplayInfoForException.bind(this); this.goToError = this.goToError.bind(this); this.startDebugger = this.startDebugger.bind(this); @@ -656,29 +655,21 @@ export class Editor extends toolboxeditor.ToolboxEditor { parent={this.parent} /> {showErrorList && } ) } - listenToErrors(handlerKey: string, handler: (errors: ErrorDisplayInfo[]) => void) { - this.errorListeners[handlerKey] = handler; - } - public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { const exceptionDisplayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); - for (let listener of pxt.U.values(this.errorListeners)) { - listener([exceptionDisplayInfo]); - } + this.errors = [exceptionDisplayInfo]; } private onErrorChanges(errors: pxtc.KsDiagnostic[]) { const errorDisplayInfo: ErrorDisplayInfo[] = errors.map(this.getDisplayInfoForError); - for (let listener of pxt.U.values(this.errorListeners)) { - listener(errorDisplayInfo); - } + this.errors = errorDisplayInfo; } // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. From abf27c7773a485162cfb6436d8a813fa1941d371 Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 21 Apr 2025 11:29:02 -0700 Subject: [PATCH 10/91] Simplified key for errors --- webapp/src/errorList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 85b59d76a8f2..b3a68b643d34 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -193,5 +193,5 @@ function groupErrors(errors: ErrorDisplayInfo[]): GroupedError[] { } function getErrorKey(error: ErrorDisplayInfo): string { - return error.message + (error.stackFrames ? error.stackFrames.map(f => f.message).join('') : ''); + return JSON.stringify(error); } \ No newline at end of file From 822135f24d8ef87dbb656033b242b04bd5e3c920 Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 21 Apr 2025 11:29:56 -0700 Subject: [PATCH 11/91] Error key adjustment --- webapp/src/errorList.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index b3a68b643d34..052a38b767ed 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -193,5 +193,8 @@ function groupErrors(errors: ErrorDisplayInfo[]): GroupedError[] { } function getErrorKey(error: ErrorDisplayInfo): string { - return JSON.stringify(error); + return JSON.stringify({ + message: error.message, + stackFrames: error.stackFrames + }); } \ No newline at end of file From e422def979216ba7f1aceb013d4981b67210e8b0 Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 21 Apr 2025 11:30:35 -0700 Subject: [PATCH 12/91] Remove to do items. These functions end up being fairly specific, so moving to a shared location doesn't make as much sense anymore. --- webapp/src/blocks.tsx | 1 - webapp/src/monaco.tsx | 2 -- 2 files changed, 3 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 2412a24486b2..0c3d6704efc8 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -914,7 +914,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { } } - // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { const message = pxt.Util.rlf(exception.exceptionMessage); const stackFrames: StackFrameDisplayInfo[] = exception.stackframes?.map(frame => { diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index 824060240cf4..415cfbb2ae05 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -672,7 +672,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.errors = errorDisplayInfo; } - // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { const message = pxt.Util.rlf(exception.exceptionMessage); @@ -691,7 +690,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { }; } - // TODO thsparks - maybe move this into the errorList but keep ErrorDisplayInfo abstraction. private getDisplayInfoForError(error: pxtc.KsDiagnostic): ErrorDisplayInfo { const message = lf("Line {0}: {1}", error.endLine ? error.endLine + 1 : error.line + 1, error.messageText); return { From 33f4fda40b20d5d60d6d5d926ef48e8940e2b4b6 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 22 Apr 2025 10:31:20 -0700 Subject: [PATCH 13/91] More readable block stacks, with ability to focus on blocks in the stack --- webapp/src/blocks.tsx | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index b2b1f8cebab5..b14932b4c657 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -917,11 +917,28 @@ export class Editor extends toolboxeditor.ToolboxEditor { private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { const message = pxt.Util.rlf(exception.exceptionMessage); const stackFrames: StackFrameDisplayInfo[] = exception.stackframes?.map(frame => { + const locInfo = frame.funcInfo as pxtc.FunctionLocationInfo; + if (!locInfo?.functionName) { + return undefined; + } + + const blockId = this.compilationResult ? pxtblockly.findBlockIdByLine(this.compilationResult.sourceMap, { start: locInfo.line, length: locInfo.endLine - locInfo.line }) : undefined; + if (!blockId) { + return undefined; + } + + // Get human-readable block text + const block = this.editor.getBlockById(blockId); + if (!block) { + return undefined; + } + const blockText = this.getBlockText(block); + return { - message: lf("at {0}", frame.funcInfo.functionName), - // TODO thsparks : Can I get the block id from the frame for an onClick? + message: blockText ? lf("at the '{0}' block", blockText) : lf("at {0}", locInfo.functionName), + onClick: () => this.highlightStatement(locInfo, exception) }; - }) ?? undefined; + }).filter(f => !!f) ?? undefined; return { message, @@ -929,6 +946,21 @@ export class Editor extends toolboxeditor.ToolboxEditor { }; } + private getBlockText(block: Blockly.BlockSvg): string { + const fieldValues = []; + for (const input of block.inputList) { + if (input.fieldRow.length > 0) { + for (const field of input.fieldRow) { + const text = field.getText(); + if (text) { + fieldValues.push(text); + } + } + } + } + return fieldValues.join(" "); + } + getBlocksAreaDiv() { return document.getElementById('blocksArea'); } From 40fefaad5c8c86fbbfc826409322d5e6e15199b9 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 22 Apr 2025 10:54:06 -0700 Subject: [PATCH 14/91] Some cleanup --- webapp/src/blocks.tsx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index b14932b4c657..228f88ea25ae 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -897,20 +897,23 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.parent.fireResize(); } + onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { + const displayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); + this.errors = [displayInfo]; + } + private onBlockErrorChanges(errors: BlockError[]) { const displayInfo = errors.map(this.getDisplayInfoForBlockError); this.errors = displayInfo; } - public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { - const displayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); - this.errors = [displayInfo]; - } - private getDisplayInfoForBlockError(error: BlockError): ErrorDisplayInfo { return { message: error.message, - onClick: () => this.editor.centerOnBlock(error.blockId) + onClick: () => { + // Block is already highlighted, just center it. + this.editor.centerOnBlock(error.blockId); + } } } From 154aed5bc5e60cf77068ab37a54117672d528390 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 22 Apr 2025 11:26:52 -0700 Subject: [PATCH 15/91] More code tidying --- webapp/src/blocks.tsx | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 228f88ea25ae..8043918d8f8b 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -46,11 +46,6 @@ interface CopyDataEntry { headerId: string; } -interface BlockError { - blockId: string; - message: string; -} - export class Editor extends toolboxeditor.ToolboxEditor { editor: Blockly.WorkspaceSvg; currFile: pkg.File; @@ -902,17 +897,23 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.errors = [displayInfo]; } - private onBlockErrorChanges(errors: BlockError[]) { - const displayInfo = errors.map(this.getDisplayInfoForBlockError); - this.errors = displayInfo; + private onBlockErrorChanges(errors: ErrorDisplayInfo[]) { + this.errors = errors; } - private getDisplayInfoForBlockError(error: BlockError): ErrorDisplayInfo { + private getDisplayInfoForBlockError(blockId: string, message: string): ErrorDisplayInfo { return { - message: error.message, + message: message, onClick: () => { - // Block is already highlighted, just center it. - this.editor.centerOnBlock(error.blockId); + // Block may already be highlighted, so we want to set highlighted directly to true + // as opposed to using `highlightBlock`, which toggles. + const block = this.editor.getBlockById(blockId); + if (!block) { + return; + } + + block.setHighlighted(true); + this.editor.centerOnBlock(blockId); } } } @@ -1201,7 +1202,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (!tsfile || !tsfile.diagnostics) return; // only show errors - const allErrors: BlockError[] = []; + const errorDisplayInfo: ErrorDisplayInfo[] = []; let diags = tsfile.diagnostics.filter(d => d.category == ts.pxtc.DiagnosticCategory.Error); let sourceMap = this.compilationResult.sourceMap; @@ -1213,10 +1214,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { let txt = ts.pxtc.flattenDiagnosticMessageText(diag.messageText, "\n"); b.setWarningText(txt, pxtblockly.PXT_WARNING_ID); setHighlightWarningAsync(b, true); - allErrors.push({ - blockId: bid, - message: txt, - }) + errorDisplayInfo.push(this.getDisplayInfoForBlockError(bid, txt)); } } }) @@ -1228,14 +1226,11 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (b) { b.setWarningText(d.message, pxtblockly.PXT_WARNING_ID); setHighlightWarningAsync(b, true); - allErrors.push({ - blockId: d.blockId, - message: d.message, - }) + errorDisplayInfo.push(this.getDisplayInfoForBlockError(d.blockId, d.message)); } } }) - this.onBlockErrorChanges(allErrors); + this.onBlockErrorChanges(errorDisplayInfo); this.setBreakpointsFromBlocks(); } From 116f4f8daeb60e59e9d2d20a629b8a2ecb58a876 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 22 Apr 2025 14:55:42 -0700 Subject: [PATCH 16/91] Clear errors when unloading a file --- webapp/src/blocks.tsx | 6 +++++- webapp/src/monaco.tsx | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 8043918d8f8b..5ad8fe62d220 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -940,7 +940,10 @@ export class Editor extends toolboxeditor.ToolboxEditor { return { message: blockText ? lf("at the '{0}' block", blockText) : lf("at {0}", locInfo.functionName), - onClick: () => this.highlightStatement(locInfo, exception) + onClick: () => { + this.clearHighlightedStatements(); + this.highlightStatement(locInfo, exception); + } }; }).filter(f => !!f) ?? undefined; @@ -1178,6 +1181,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { unloadFileAsync(): Promise { this.delayLoadXml = undefined; + this.errors = []; if (this.toolbox) this.toolbox.clearSearch(); return Promise.resolve(); } diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index 0db01aef6cc9..db95de9a75a9 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -1487,6 +1487,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { unloadFileAsync(): Promise { if (this.toolbox) this.toolbox.clearSearch(); + this.errors = []; if (this.currFile && this.currFile.getName() == "this/" + pxt.CONFIG_NAME) { // Reload the header if a change was made to the config file: pxt.json return this.parent.reloadHeaderAsync(); From 972cef49958be427b3288e4cebfb8cf6622d4082 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 22 Apr 2025 17:17:50 -0700 Subject: [PATCH 17/91] Move getBlockText to pxtblocks for easier reuse --- pxtblocks/getBlockText.ts | 16 ++++++++++++++++ pxtblocks/index.ts | 1 + 2 files changed, 17 insertions(+) create mode 100644 pxtblocks/getBlockText.ts diff --git a/pxtblocks/getBlockText.ts b/pxtblocks/getBlockText.ts new file mode 100644 index 000000000000..3d4e91ab2210 --- /dev/null +++ b/pxtblocks/getBlockText.ts @@ -0,0 +1,16 @@ +import * as Blockly from "blockly"; + +export function getBlockText(block: Blockly.BlockSvg): string { + const fieldValues = []; + for (const input of block.inputList) { + if (input.fieldRow.length > 0) { + for (const field of input.fieldRow) { + const text = field.getText(); + if (text) { + fieldValues.push(text); + } + } + } + } + return fieldValues.join(" "); +} diff --git a/pxtblocks/index.ts b/pxtblocks/index.ts index 51687440b96b..675816eed2b8 100644 --- a/pxtblocks/index.ts +++ b/pxtblocks/index.ts @@ -21,6 +21,7 @@ export * from "./legacyMutations"; export * from "./blockDragger"; export * from "./workspaceSearch"; export * from "./monkeyPatches"; +export * from "./getBlockText"; import * as contextMenu from "./contextMenu"; import * as external from "./external"; From bb7ddedc0e0b2eddfa4c824e959429cb3806a514 Mon Sep 17 00:00:00 2001 From: thsparks Date: Fri, 25 Apr 2025 14:50:01 -0700 Subject: [PATCH 18/91] Hacking together editor tour for error help --- localtypings/pxtarget.d.ts | 1 + localtypings/pxteditor.d.ts | 1 + .../components/controls/TeachingBubble.tsx | 9 +- webapp/src/app.tsx | 5 +- webapp/src/blocks.tsx | 65 +++--- webapp/src/components/errorHelpButton.tsx | 186 ++++++++++++++++++ webapp/src/components/onboarding/Tour.tsx | 2 + webapp/src/errorList.tsx | 17 +- webapp/src/monaco.tsx | 4 +- 9 files changed, 257 insertions(+), 33 deletions(-) create mode 100644 webapp/src/components/errorHelpButton.tsx diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index 5c682fdb8141..851756454636 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1338,6 +1338,7 @@ declare namespace pxt.tour { location: BubbleLocation; sansQuery?: string; // Use this to exclude an element from the cutout sansLocation?: BubbleLocation; // relative location of element to exclude + onStepBegin?: () => void; } const enum BubbleLocation { Above, diff --git a/localtypings/pxteditor.d.ts b/localtypings/pxteditor.d.ts index 8cd36c8b3aa4..1ea25a62866a 100644 --- a/localtypings/pxteditor.d.ts +++ b/localtypings/pxteditor.d.ts @@ -1056,6 +1056,7 @@ declare namespace pxt.editor { showLightbox(): void; hideLightbox(): void; showOnboarding(): void; + showTour(steps: pxt.tour.BubbleStep[]): void; hideOnboarding(): void; showKeymap(show: boolean): void; toggleKeymap(): void; diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index de8ebb2220d5..145497cc7719 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -57,16 +57,21 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { } useEffect(() => { + if (props.targetContent.onStepBegin) { + props.targetContent.onStepBegin(); + } positionBubbleAndCutout(); setStepsVisibility(); window.addEventListener("resize", positionBubbleAndCutout); return () => { window.removeEventListener("resize", positionBubbleAndCutout); } - }, [stepNumber]); + }, [stepNumber, targetContent]); const setStepsVisibility = () => { const steps = document.querySelector(".teaching-bubble-steps") as HTMLElement; + if (!steps) return; + if (stepNumber > totalSteps || totalSteps === 1) { steps.style.visibility = "hidden"; } else { @@ -362,7 +367,7 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { ); return ReactDOM.createPortal( - {stepNumber === totalSteps + 1 && } + {/* {stepNumber === totalSteps + 1 && } */}
diff --git a/webapp/src/app.tsx b/webapp/src/app.tsx index e3852fe3b49d..26526ab15eb7 100644 --- a/webapp/src/app.tsx +++ b/webapp/src/app.tsx @@ -5246,8 +5246,11 @@ export class ProjectView async showOnboarding() { const tourSteps: pxt.tour.BubbleStep[] = await parseTourStepsAsync(pxt.appTarget.appTheme?.tours?.editor) - this.setState({ onboarding: tourSteps }) + this.showTour(tourSteps); + } + async showTour(steps: pxt.tour.BubbleStep[]) { + this.setState({ onboarding: steps }); } /////////////////////////////////////////////////////////// diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 5ad8fe62d220..15f5c29ce479 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -36,6 +36,7 @@ import SimState = pxt.editor.SimState; import { DuplicateOnDragConnectionChecker } from "../../pxtblocks/plugins/duplicateOnDrag"; import { PathObject } from "../../pxtblocks/plugins/renderer/pathObject"; import { Measurements } from "./constants"; +import { ErrorHelpResponse } from "./components/errorHelpButton"; interface CopyDataEntry { version: 1; @@ -78,6 +79,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { super(parent); this.onErrorListResize = this.onErrorListResize.bind(this) + this.handleErrorHelp = this.handleErrorHelp.bind(this) this.getDisplayInfoForBlockError = this.getDisplayInfoForBlockError.bind(this) this.getDisplayInfoForException = this.getDisplayInfoForException.bind(this) } @@ -297,12 +299,17 @@ export class Editor extends toolboxeditor.ToolboxEditor { return this.serializeBlocks(); } - private serializeBlocks(normalize?: boolean): string { + getWorkspaceXmlWithIds(): string { + const xml = this.serializeBlocks(false, true); + return xml; + } + + private serializeBlocks(normalize?: boolean, forceKeepIds?: boolean): string { // store ids when using github - let xml = pxtblockly.saveWorkspaceXml(this.editor, - !normalize && this.parent.state + let xml = pxtblockly.saveWorkspaceXml(this.editor, forceKeepIds || + (!normalize && this.parent.state && this.parent.state.header - && !!this.parent.state.header.githubId); + && !!this.parent.state.header.githubId)); // strip out id, x, y attributes if (normalize) xml = xml.replace(/(x|y|id)="[^"]*"/g, '') pxt.debug(xml) @@ -883,6 +890,8 @@ export class Editor extends toolboxeditor.ToolboxEditor {
{showErrorList && }
) @@ -920,52 +929,54 @@ export class Editor extends toolboxeditor.ToolboxEditor { private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { const message = pxt.Util.rlf(exception.exceptionMessage); - const stackFrames: StackFrameDisplayInfo[] = exception.stackframes?.map(frame => { + const stackFrames: StackFrameDisplayInfo[] = []; + for (const frame of exception.stackframes ?? []) { const locInfo = frame.funcInfo as pxtc.FunctionLocationInfo; if (!locInfo?.functionName) { - return undefined; + continue; } const blockId = this.compilationResult ? pxtblockly.findBlockIdByLine(this.compilationResult.sourceMap, { start: locInfo.line, length: locInfo.endLine - locInfo.line }) : undefined; if (!blockId) { - return undefined; + continue; } // Get human-readable block text const block = this.editor.getBlockById(blockId); if (!block) { - return undefined; + continue; } - const blockText = this.getBlockText(block); + const blockText = pxtblockly.getBlockText(block); - return { + stackFrames.push({ message: blockText ? lf("at the '{0}' block", blockText) : lf("at {0}", locInfo.functionName), onClick: () => { - this.clearHighlightedStatements(); - this.highlightStatement(locInfo, exception); + this.editor.centerOnBlock(blockId); + const tourSteps = [ + { + title: lf("Error Explanation"), + description: lf("Here's some presumably helpful text. This is just a placeholder."), + targetQuery: `g[data-id="${blockId}"]`, + location: pxt.tour.BubbleLocation.Right, + // sansQuery?: string; // Use this to exclude an element from the cutout + // sansLocation?: BubbleLocation; // relative location of element to exclude + } as pxt.tour.BubbleStep + ]; + this.parent.showTour(tourSteps); } - }; - }).filter(f => !!f) ?? undefined; + }); + } return { message, - stackFrames + stackFrames: stackFrames.length > 0 ? stackFrames : undefined, }; } - private getBlockText(block: Blockly.BlockSvg): string { - const fieldValues = []; - for (const input of block.inputList) { - if (input.fieldRow.length > 0) { - for (const field of input.fieldRow) { - const text = field.getText(); - if (text) { - fieldValues.push(text); - } - } - } + private handleErrorHelp(helpResponse: ErrorHelpResponse) { + if (helpResponse.explanationAsTour) { + this.parent.showTour(helpResponse.explanationAsTour); } - return fieldValues.join(" "); } getBlocksAreaDiv() { diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx new file mode 100644 index 000000000000..9097e9cb41ee --- /dev/null +++ b/webapp/src/components/errorHelpButton.tsx @@ -0,0 +1,186 @@ +import * as React from "react"; +import * as pkg from "../package"; +import { ErrorDisplayInfo } from "../errorList"; +import { Button } from "../../../react-common/components/controls/Button"; +import { classList } from "../../../react-common/components/util"; +import { Editor } from "../blocks"; + + +// TODO thsparks - there is probably a better way to do this. +export interface ErrorHelpResponse { + explanationAsText: string; + explanationAsTour: pxt.tour.BubbleStep[]; +} + +export interface ErrorHelpButtonProps { + parent: pxt.editor.IProjectView; + errors: ErrorDisplayInfo[]; + onHelpResponse?: (response: ErrorHelpResponse) => void; +} + +interface ErrorHelpButtonState { + explanation?: ErrorHelpResponse; + loadingHelp?: boolean; +} + +export class ErrorHelpButton extends React.Component< + ErrorHelpButtonProps, + ErrorHelpButtonState +> { + constructor(props: ErrorHelpButtonProps) { + super(props); + this.state = { + explanation: undefined, + loadingHelp: false, + }; + + this.handleHelpClick = this.handleHelpClick.bind(this); + this.aiErrorExplainRequest = this.aiErrorExplainRequest.bind(this); + this.parseResponse = this.parseResponse.bind(this); + this.getErrorsAsText = this.getErrorsAsText.bind(this); + } + + async aiErrorExplainRequest( + code: string, + errors: string[], + lang: string, + target: string + ): Promise { + const url = `/api/copilot/explainerror`; + const data = { code, errors, lang, target }; + let result: string = ""; + + const request = await pxt.auth.AuthClient.staticApiAsync(url, data, "POST"); + if (!request.success) { + throw new Error( + request.err || + lf( + "Unable to reach AI. Error: {0}.\n{1}", + request.statusCode, + request.err + ) + ); + } + result = await request.resp; + + return result; + } + + parseResponse(response: string): ErrorHelpResponse { + // We ask the AI to provide an ordered JSON list with blockIds tied to explanations for each block. + // For example: + // [ + // { + // "blockId": "sampleblockid", + // "message": "Here is a helpful message walking you through the error. This one will be displayed first." + // }, + // { + // "blockId": "sampleblockid2", + // "message": "Here is another helpful message walking you through the error. This one will be displayed second." + // } + // ] + interface AiResponseJsonFormat { + blockId: string; + message: string; + } + + let explanationAsText = ""; + let explanationAsTour: pxt.tour.BubbleStep[] = []; + + console.log("AI response", response); + + // If the response contains any leading or trailing ` characters, remove them + response = response.trim().replace(/^`+|`+$/g, ""); + + const parsedResponse = pxt.Util.jsonTryParse( + response + ) as AiResponseJsonFormat[]; + if (parsedResponse) { + const validBlockIds = this.props.parent.getBlocks().map((b) => b.id); + for (const item of parsedResponse) { + // Check each blockId is valid, and if so, add a tour step for it + const blockId = item.blockId; + if (validBlockIds.includes(blockId)) { + const tourStep = { + title: lf("Error Explanation"), + description: item.message, + targetQuery: `g[data-id="${blockId}"]`, + location: pxt.tour.BubbleLocation.Right, + onStepBegin: () => { + (this.props.parent.editor as Editor).editor.centerOnBlock(blockId); + } + // sansQuery?: string; // Use this to exclude an element from the cutout + // sansLocation?: BubbleLocation; // relative location of element to exclude + } as pxt.tour.BubbleStep; + + explanationAsTour.push(tourStep); + } else { + // TODO - handle this case + console.error("Invalid blockId in AI response", blockId, parsedResponse, response); + } + } + } else { + explanationAsText = response; // TODO - handle this case better + } + + return { + explanationAsText, + explanationAsTour: + explanationAsTour.length > 0 ? explanationAsTour : undefined, + }; + } + + async handleHelpClick() { + this.setState({ + loadingHelp: true, + }); + + const mainFileName = this.props.parent.isBlocksActive() + ? pxt.MAIN_BLOCKS + : pxt.MAIN_TS; + const lang = this.props.parent.isBlocksActive() ? "blocks" : "typescript"; + const target = pxt.appTarget.nickname || pxt.appTarget.name; + const mainPkg = pkg.mainEditorPkg(); + const code = (this.props.parent.isBlocksActive() ? (this.props.parent.editor as Editor).getWorkspaceXmlWithIds() : mainPkg.files[mainFileName]?.content) ?? ""; + console.log("Code", code); + + const errors = this.getErrorsAsText(); + console.log("Errors", errors); + + // Call to backend API with code and errors, then set the explanation state + // to the response from the backend + const response = await this.aiErrorExplainRequest( + code, + errors, + lang, + target + ); + + const parsedResponse = this.parseResponse(response); + + this.props.onHelpResponse?.(parsedResponse); + } + + getErrorsAsText(): string[] { + return this.props.errors.map((error) => error.message); + } + + render() { + const { loadingHelp } = this.state; + + // if (loadingHelp) { + // return
; + // } + + return ( +
From 01b6bc43ace1f4bd02d0cd2d23efbdcc60cf265d Mon Sep 17 00:00:00 2001 From: thsparks Date: Fri, 25 Apr 2025 17:36:27 -0700 Subject: [PATCH 21/91] Some refactoring of the error list item --- webapp/src/errorList.tsx | 42 ++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 693466597da2..57d5b1909150 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -140,18 +140,30 @@ class ErrorListItem extends React.Component 0; + const topRowClass = hasStack ? "exceptionMessage" : classList("item", className); - return error.stackFrames ? ( + const itemHeaderRow = isInteractive ? ( +
+ {error.message} + {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} +
+ ) : ( +
+ {error.message} + {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} +
+ ); + + return !hasStack ? itemHeaderRow : (
-
- {error.message} -
+ {itemHeaderRow}
{(error.stackFrames).map((childErr, index) => { const errGrp = {error: childErr, count: 1, index: 0}; @@ -159,15 +171,7 @@ class ErrorListItem extends React.Component
- ) : ( -
- {message} {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} -
- ); + ) } } From 1e05bcd5967b1f341b5662c928347f9607241aaa Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 28 Apr 2025 15:03:00 -0700 Subject: [PATCH 22/91] Button for item row when it's interactive --- theme/errorList.less | 2 ++ webapp/src/errorList.tsx | 17 +++++++++-------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/theme/errorList.less b/theme/errorList.less index 562634a1d16a..ea5439be4be3 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -72,6 +72,8 @@ color: var(--pxt-target-foreground1); cursor: pointer; line-height: 1.2rem; + width: 100%; + text-align: start; &:hover, &:focus { background-color: var(--pxt-neutral-alpha20) !important; diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 57d5b1909150..d9083f532619 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -4,6 +4,7 @@ import * as React from "react"; import * as sui from "./sui"; import { fireClickOnEnter } from "./util"; import { classList } from "../../react-common/components/util"; +import { Button } from "../../react-common/components/controls/Button"; type GroupedError = { error: ErrorDisplayInfo, @@ -145,15 +146,15 @@ class ErrorListItem extends React.Component - {error.message} - {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} -
+ title={lf("Go to error: {0}", error.message)} + aria-label={lf("Go to error: {0}", error.message)}> + <> + {error.message} + {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} + + ) : (
{error.message} From 7b277abf41e25687fadf64523686903f7a18f58c Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 28 Apr 2025 15:23:38 -0700 Subject: [PATCH 23/91] Wrap message in span (and button content in div) --- webapp/src/errorList.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index d9083f532619..3c60d585d8e4 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -150,10 +150,10 @@ class ErrorListItem extends React.Component - <> - {error.message} +
+ {error.message} {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} - +
) : (
From 16dee9e737e1793337291c9441fa9d7ddf95101e Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 28 Apr 2025 15:33:46 -0700 Subject: [PATCH 24/91] Combine error counter elements, add missing span --- webapp/src/errorList.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 3c60d585d8e4..03e6ea4d4878 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -144,6 +144,7 @@ class ErrorListItem extends React.Component 0; const topRowClass = hasStack ? "exceptionMessage" : classList("item", className); + const errorCounter = (errorGroup.count <= 1) ? null :
{errorGroup.count}
; const itemHeaderRow = isInteractive ? ( ) : (
- {error.message} - {(errorGroup.count <= 1) ? null :
{errorGroup.count}
} + {error.message} + {errorCounter}
); From 0960d4991a5d7ca9c4033d1c4ba8808bf38bb663 Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 13:39:32 -0700 Subject: [PATCH 25/91] Adding getFieldDescription API, still need to improve the getBlockText function. --- pxtblocks/contextMenu/blockItems.ts | 19 ++++++++++++++ pxtblocks/fields/field_asset.ts | 4 +++ pxtblocks/fields/field_autocomplete.ts | 4 +++ pxtblocks/fields/field_base.ts | 8 ++++++ pxtblocks/fields/field_colour.ts | 5 ++++ pxtblocks/fields/field_gridpicker.ts | 4 +++ pxtblocks/fields/field_imagedropdown.ts | 4 +++ pxtblocks/fields/field_ledmatrix.ts | 4 +++ pxtblocks/fields/field_melodySandbox.ts | 4 +++ pxtblocks/fields/field_note.ts | 26 ++++++++++++------- pxtblocks/fields/field_scopedvalueselector.ts | 4 +++ pxtblocks/fields/field_sound_effect.ts | 4 +++ pxtblocks/fields/field_styledlabel.ts | 4 +++ pxtblocks/fields/field_textdropdown.ts | 4 +++ pxtblocks/fields/field_textinput.ts | 4 +++ pxtblocks/fields/field_tileset.ts | 4 +++ pxtblocks/fields/field_toggle.ts | 4 +++ pxtblocks/fields/field_tsexpression.ts | 4 +++ pxtblocks/fields/field_utils.ts | 5 ++++ pxtblocks/getBlockText.ts | 12 ++++++++- pxtblocks/plugins/math/fieldSlider.ts | 4 +++ 21 files changed, 125 insertions(+), 10 deletions(-) diff --git a/pxtblocks/contextMenu/blockItems.ts b/pxtblocks/contextMenu/blockItems.ts index 487578a05e82..3cc5916f5904 100644 --- a/pxtblocks/contextMenu/blockItems.ts +++ b/pxtblocks/contextMenu/blockItems.ts @@ -1,6 +1,7 @@ /// import * as Blockly from "blockly"; import { openHelpUrl } from "../external"; +import { getBlockText } from "../getBlockText"; // Lower weight is higher in context menu export enum BlockContextWeight { @@ -23,6 +24,24 @@ export function registerBlockitems() { registerCollapseExpandBlock(); registerHelp(); + // TODO : Remove - this is just temporary to easily view block strings while testing + const tempDisplayBlockText: Blockly.ContextMenuRegistry.RegistryItem = { + displayText(scope: Blockly.ContextMenuRegistry.Scope) { + return getBlockText(scope.block); + }, + preconditionFn(scope: Blockly.ContextMenuRegistry.Scope) { + return "enabled"; + }, + callback(scope: Blockly.ContextMenuRegistry.Scope) { + const blockText = getBlockText(scope.block); + console.log(blockText); + }, + scopeType: Blockly.ContextMenuRegistry.ScopeType.BLOCK, + id: 'tempDisplayBlockText', + weight: 1, + }; + Blockly.ContextMenuRegistry.registry.register(tempDisplayBlockText); + // Fix the weights of the builtin options we do use Blockly.ContextMenuRegistry.registry.getItem("blockDelete").weight = BlockContextWeight.DeleteBlock; Blockly.ContextMenuRegistry.registry.getItem("blockComment").weight = BlockContextWeight.AddComment; diff --git a/pxtblocks/fields/field_asset.ts b/pxtblocks/fields/field_asset.ts index a8220cb02f05..d780315a95d3 100644 --- a/pxtblocks/fields/field_asset.ts +++ b/pxtblocks/fields/field_asset.ts @@ -136,6 +136,10 @@ export abstract class FieldAssetEditor extends Blockly.Field implements FieldCustom this.valueText = this.onValueChanged(this.valueText); } + /** + * Returns a human-readable description of the field's current value. + */ + getFieldDescription(): string { + return this.getDisplayText_(); + } + protected getAnchorDimensions() { const boundingBox = this.getScaledBBox() as any; if (this.sourceBlock_.RTL) { @@ -117,4 +124,5 @@ export abstract class FieldBase extends Blockly.Field implements FieldCustom if (!block) return undefined; return block.getField(fieldName); } + } \ No newline at end of file diff --git a/pxtblocks/fields/field_colour.ts b/pxtblocks/fields/field_colour.ts index 70fc2adcdf63..96b224b5fac0 100644 --- a/pxtblocks/fields/field_colour.ts +++ b/pxtblocks/fields/field_colour.ts @@ -90,6 +90,11 @@ export class FieldColorNumber extends FieldColour implements FieldCustom { return this.value_; } + getFieldDescription(): string { + const value = this.getValue(); + return value ? lf("color ${0}", value) : lf("color"); + } + /** * Defines whether this field should take up the full block or not. * diff --git a/pxtblocks/fields/field_gridpicker.ts b/pxtblocks/fields/field_gridpicker.ts index 7cca25385821..668159f07adb 100644 --- a/pxtblocks/fields/field_gridpicker.ts +++ b/pxtblocks/fields/field_gridpicker.ts @@ -296,6 +296,10 @@ export class FieldGridPicker extends Blockly.FieldDropdown implements FieldCusto return newValue; } + getFieldDescription(): string { + return this.getValue(); + } + /** * Closes the gridpicker. */ diff --git a/pxtblocks/fields/field_imagedropdown.ts b/pxtblocks/fields/field_imagedropdown.ts index a36733d47b91..5ee1383c85b1 100644 --- a/pxtblocks/fields/field_imagedropdown.ts +++ b/pxtblocks/fields/field_imagedropdown.ts @@ -201,6 +201,10 @@ export class FieldImageDropdown extends FieldDropdown implements FieldCustom { super.doValueUpdate_(newValue); } + getFieldDescription(): string { + return lf("image"); + } + /** * Callback for when a button is clicked inside the drop-down. * Should be bound to the FieldIconMenu. diff --git a/pxtblocks/fields/field_ledmatrix.ts b/pxtblocks/fields/field_ledmatrix.ts index f488d1850ed9..9e5342d5e5ee 100644 --- a/pxtblocks/fields/field_ledmatrix.ts +++ b/pxtblocks/fields/field_ledmatrix.ts @@ -435,6 +435,10 @@ export class FieldMatrix extends Blockly.Field implements FieldCustom { return `\`\n${FieldMatrix.TAB}${text}\n${FieldMatrix.TAB}\``; } + getFieldDescription(): string { + return lf("LED Grid"); + } + // Restores the block state from the text value of the field private restoreStateFromString() { let r = this.value_ as string; diff --git a/pxtblocks/fields/field_melodySandbox.ts b/pxtblocks/fields/field_melodySandbox.ts index 6492b009951f..ba3d2d9cc493 100644 --- a/pxtblocks/fields/field_melodySandbox.ts +++ b/pxtblocks/fields/field_melodySandbox.ts @@ -115,6 +115,10 @@ export class FieldCustomMelody extends Blockly.Fie else return this.getValue(); } + getFieldDescription(): string { + return lf("melody"); + } + // This will be run when the field is created (i.e. when it appears on the workspace) protected onInit() { this.render_(); diff --git a/pxtblocks/fields/field_note.ts b/pxtblocks/fields/field_note.ts index 7d54941c4ff5..3ee5745fbe19 100644 --- a/pxtblocks/fields/field_note.ts +++ b/pxtblocks/fields/field_note.ts @@ -209,17 +209,25 @@ export class FieldNote extends Blockly.FieldNumber implements FieldCustom { if (this.isExpanded) { return "" + this.value_; } else { - const note = +this.value_; - for (let i = 0; i < this.nKeys_; i++) { - if (Math.abs(this.getKeyFreq(i + this.minNote_) - note) < this.eps) { - return this.getKeyName(i + this.minNote_); - } + return this.getNoteString(); + } + } + + getFieldDescription(): string { + return this.getNoteString() || lf("note"); + } + + private getNoteString() { + const note = +this.value_; + for (let i = 0; i < this.nKeys_; i++) { + if (Math.abs(this.getKeyFreq(i + this.minNote_) - note) < this.eps) { + return this.getKeyName(i + this.minNote_); } - let text = note.toString(); - if (!isNaN(note)) - text += " Hz"; - return text; } + let text = note.toString(); + if (!isNaN(note)) + text += " Hz"; + return text; } /** diff --git a/pxtblocks/fields/field_scopedvalueselector.ts b/pxtblocks/fields/field_scopedvalueselector.ts index fca1d8f42442..0dd1b518cfdf 100644 --- a/pxtblocks/fields/field_scopedvalueselector.ts +++ b/pxtblocks/fields/field_scopedvalueselector.ts @@ -136,4 +136,8 @@ export class FieldScopedValueSelector extends Blockly.FieldLabel implements Fiel if (!this.sourceBlock_ || !this.sourceBlock_.workspace || this.sourceBlock_.disposed) return; if (ev.type === Blockly.Events.BLOCK_DRAG) return this.onDragEvent(ev as Blockly.Events.BlockDrag); } + + getFieldDescription(): string { + return this.scopedValue || this.defl || lf("value"); + } } diff --git a/pxtblocks/fields/field_sound_effect.ts b/pxtblocks/fields/field_sound_effect.ts index 960a4d9b19e2..a86c6e576085 100644 --- a/pxtblocks/fields/field_sound_effect.ts +++ b/pxtblocks/fields/field_sound_effect.ts @@ -272,6 +272,10 @@ export class FieldSoundEffect extends FieldBase { this.size_.width = TOTAL_WIDTH + X_PADDING; } + getFieldDescription(): string { + return lf("sound effect"); + } + protected updateSiblingBlocks(sound: pxt.assets.Sound) { this.setNumberInputValue(this.options.durationInputName, sound.duration); this.setNumberInputValue(this.options.startFrequencyInputName, sound.startFrequency); diff --git a/pxtblocks/fields/field_styledlabel.ts b/pxtblocks/fields/field_styledlabel.ts index 31c914eace36..64e2bc6a2a2c 100644 --- a/pxtblocks/fields/field_styledlabel.ts +++ b/pxtblocks/fields/field_styledlabel.ts @@ -14,6 +14,10 @@ export class FieldStyledLabel extends Blockly.FieldLabel implements FieldCustom constructor(value: string, options?: StyleOptions, opt_validator?: Function) { super(value, getClass(options)); } + + getFieldDescription(): string { + return this.getText(); + } } function getClass(options?: StyleOptions) { diff --git a/pxtblocks/fields/field_textdropdown.ts b/pxtblocks/fields/field_textdropdown.ts index d6820cfc456a..aadcaebb0902 100644 --- a/pxtblocks/fields/field_textdropdown.ts +++ b/pxtblocks/fields/field_textdropdown.ts @@ -73,6 +73,10 @@ export class BaseFieldTextDropdown extends Blockly.FieldTextInput { return typeof this.menuGenerator_ === 'function'; } + getFieldDescription(): string { + return this.getText(); + } + protected dropdownDispose_() { this.dropDownOpen_ = false; if (this.menu_) { diff --git a/pxtblocks/fields/field_textinput.ts b/pxtblocks/fields/field_textinput.ts index b7391ac3179a..1b19dbb80cfb 100644 --- a/pxtblocks/fields/field_textinput.ts +++ b/pxtblocks/fields/field_textinput.ts @@ -7,4 +7,8 @@ export class FieldTextInput extends Blockly.FieldTextInput implements FieldCusto constructor(value: string, options: FieldCustomOptions, opt_validator?: Blockly.FieldValidator) { super(value, opt_validator); } + + getFieldDescription(): string { + return this.getValue(); + } } \ No newline at end of file diff --git a/pxtblocks/fields/field_tileset.ts b/pxtblocks/fields/field_tileset.ts index 487696f2f57a..f3f6519c4fba 100644 --- a/pxtblocks/fields/field_tileset.ts +++ b/pxtblocks/fields/field_tileset.ts @@ -314,6 +314,10 @@ export class FieldTileset extends FieldImages implements FieldCustom { this.localTile = asset as pxt.Tile; super.loadState(pxt.getTSReferenceForAsset(asset)); } + + getFieldDescription(): string { + return lf("tileset"); + } } function constructTransparentTile(): TilesetDropdownOption { diff --git a/pxtblocks/fields/field_toggle.ts b/pxtblocks/fields/field_toggle.ts index e0d2b840db54..b41dbc21e7bd 100644 --- a/pxtblocks/fields/field_toggle.ts +++ b/pxtblocks/fields/field_toggle.ts @@ -130,6 +130,10 @@ export class BaseFieldToggle extends Blockly.FieldNumber implements FieldCustom return this.falseText; } + getFieldDescription(): string { + return this.getDisplayText_(); + } + updateSize_() { switch (this.getOutputShape()) { case provider.SHAPES.ROUND: diff --git a/pxtblocks/fields/field_tsexpression.ts b/pxtblocks/fields/field_tsexpression.ts index 5b30e5a6f70a..3ab230dbfe7c 100644 --- a/pxtblocks/fields/field_tsexpression.ts +++ b/pxtblocks/fields/field_tsexpression.ts @@ -38,6 +38,10 @@ export class FieldTsExpression extends Blockly.FieldTextInput implements FieldCu return this.pythonMode ? pxt.Util.lf("") : this.getValue(); } + getFieldDescription(): string { + return this.pythonMode ? pxt.Util.lf("") : pxt.Util.lf(""); + } + applyColour() { if (this.sourceBlock_ && this.getConstants()?.FULL_BLOCK_FIELDS) { if (this.borderRect_) { diff --git a/pxtblocks/fields/field_utils.ts b/pxtblocks/fields/field_utils.ts index 6199e596adaa..fd94e10964ef 100644 --- a/pxtblocks/fields/field_utils.ts +++ b/pxtblocks/fields/field_utils.ts @@ -9,6 +9,11 @@ export interface FieldCustom { isFieldCustom_: boolean; saveOptions?(): pxt.Map; restoreOptions?(map: pxt.Map): void; + + /** + * Returns a human-readable description of the field's current value. + */ + getFieldDescription(): string; } export interface FieldCustomOptions { diff --git a/pxtblocks/getBlockText.ts b/pxtblocks/getBlockText.ts index 3d4e91ab2210..1c3db84214cc 100644 --- a/pxtblocks/getBlockText.ts +++ b/pxtblocks/getBlockText.ts @@ -1,15 +1,25 @@ import * as Blockly from "blockly"; +import { FieldCustom } from "./fields/field_utils"; export function getBlockText(block: Blockly.BlockSvg): string { const fieldValues = []; for (const input of block.inputList) { if (input.fieldRow.length > 0) { for (const field of input.fieldRow) { - const text = field.getText(); + const text = (field as FieldCustom & Blockly.Field).isFieldCustom_ + ? (field as any).getFieldDescription() + : field.getText(); + if (text) { fieldValues.push(text); } } + + // Check if this input has a connected block + if (input.connection && input.connection.targetBlock()) { + const connectedBlock = input.connection.targetBlock(); + fieldValues.push(`[${getBlockText(connectedBlock as Blockly.BlockSvg)}]`); + } } } return fieldValues.join(" "); diff --git a/pxtblocks/plugins/math/fieldSlider.ts b/pxtblocks/plugins/math/fieldSlider.ts index c1e6b131dbdc..90437273f114 100644 --- a/pxtblocks/plugins/math/fieldSlider.ts +++ b/pxtblocks/plugins/math/fieldSlider.ts @@ -54,6 +54,10 @@ export class FieldSlider extends Blockly.FieldNumber { this.step_ = parseFloat(step) || undefined; } + getFieldDescription(): string { + return this.getValue() + ""; + } + // Mostly the same as FieldNumber, but doesn't constrain min and max protected override doClassValidation_(newValue?: any): number { if (newValue === null) { From 491e6a0160f2af9f0b7060379b46a38a854d2757 Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 14:14:53 -0700 Subject: [PATCH 26/91] Hide expand / shrink buttons from display text --- pxtblocks/composableMutations.ts | 9 +++++---- pxtblocks/fields/field_imagenotext.ts | 19 +++++++++++++++++++ pxtblocks/getBlockText.ts | 5 ++++- 3 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 pxtblocks/fields/field_imagenotext.ts diff --git a/pxtblocks/composableMutations.ts b/pxtblocks/composableMutations.ts index d3f0e52b8d74..21d73f2ab6fa 100644 --- a/pxtblocks/composableMutations.ts +++ b/pxtblocks/composableMutations.ts @@ -7,6 +7,7 @@ import { optionalDummyInputPrefix, optionalInputWithFieldPrefix } from "./consta import { FieldArgumentVariable } from "./fields"; import { setVarFieldValue } from "./loader"; import { UpdateBeforeRenderMixin } from "./plugins/renderer"; +import { FieldImageNoText } from "./fields/field_imagenotext"; export interface ComposableMutation { // Set to save mutations. Should return an XML element @@ -103,7 +104,7 @@ export function initVariableArgsBlock(b: Blockly.Block, handlerArgs: pxt.blocks. }); function addPlusButton() { - i.appendField(new Blockly.FieldImage((b as any).ADD_IMAGE_DATAURI, 24, 24, lf("Add argument"), + i.appendField(new FieldImageNoText((b as any).ADD_IMAGE_DATAURI, 24, 24, lf("Add argument"), () => { currentlyVisible = Math.min(currentlyVisible + 1, handlerArgs.length); updateShape(); @@ -258,7 +259,7 @@ export function initExpandableBlock(info: pxtc.BlocksInfo, b: Blockly.Block, def function addButton(name: string, uri: string, alt: string, delta: number) { b.appendDummyInput(name) - .appendField(new Blockly.FieldImage(uri, 24, 24, alt, () => updateShape(delta), false)) + .appendField(new FieldImageNoText(uri, 24, 24, alt, () => updateShape(delta), false)); } function updateButtons() { @@ -284,8 +285,8 @@ export function initExpandableBlock(info: pxtc.BlocksInfo, b: Blockly.Block, def function addPlusAndMinusButtons() { b.appendDummyInput(buttonAddRemName) - .appendField(new Blockly.FieldImage((b as any).REMOVE_IMAGE_DATAURI, 24, 24, lf("Hide optional arguments"), () => updateShape(-1 * buttonDelta), false)) - .appendField(new Blockly.FieldImage((b as any).ADD_IMAGE_DATAURI, 24, 24, lf("Reveal optional arguments"), () => updateShape(buttonDelta), false)) + .appendField(new FieldImageNoText((b as any).REMOVE_IMAGE_DATAURI, 24, 24, lf("Hide optional arguments"), () => updateShape(-1 * buttonDelta), false)) + .appendField(new FieldImageNoText((b as any).ADD_IMAGE_DATAURI, 24, 24, lf("Reveal optional arguments"), () => updateShape(buttonDelta), false)) } function addPlusButton() { diff --git a/pxtblocks/fields/field_imagenotext.ts b/pxtblocks/fields/field_imagenotext.ts new file mode 100644 index 000000000000..7e640458af75 --- /dev/null +++ b/pxtblocks/fields/field_imagenotext.ts @@ -0,0 +1,19 @@ + +import * as Blockly from "blockly"; +import { FieldCustom } from "./field_utils"; + +/** + * An image field on the block that does not have a text label. + * These do not produce text for display strings, but may have a visual element (like an expand button). + */ +export class FieldImageNoText extends Blockly.FieldImage implements FieldCustom { + isFieldCustom_: boolean = true; + + constructor(src: string | typeof Blockly.Field.SKIP_SETUP, width: string | number, height: string | number, alt?: string, onClick?: (p1: Blockly.FieldImage) => void, flipRtl?: boolean, config?: Blockly.FieldImageConfig) { + super(src, width, height, alt, onClick, flipRtl, config); + } + + getFieldDescription(): string { + return undefined; + } +} \ No newline at end of file diff --git a/pxtblocks/getBlockText.ts b/pxtblocks/getBlockText.ts index 1c3db84214cc..21d294580e20 100644 --- a/pxtblocks/getBlockText.ts +++ b/pxtblocks/getBlockText.ts @@ -18,7 +18,10 @@ export function getBlockText(block: Blockly.BlockSvg): string { // Check if this input has a connected block if (input.connection && input.connection.targetBlock()) { const connectedBlock = input.connection.targetBlock(); - fieldValues.push(`[${getBlockText(connectedBlock as Blockly.BlockSvg)}]`); + const innerBlockText = getBlockText(connectedBlock as Blockly.BlockSvg); + if (innerBlockText) { + fieldValues.push(`[${innerBlockText}]`); + } } } } From b0ac1e2bca03dd1dec10d610994f8c75d771a3ad Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 15:03:27 -0700 Subject: [PATCH 27/91] Fix display text for ifElse --- pxtblocks/plugins/logic/ifElse.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pxtblocks/plugins/logic/ifElse.ts b/pxtblocks/plugins/logic/ifElse.ts index a54061497240..882b869e1511 100644 --- a/pxtblocks/plugins/logic/ifElse.ts +++ b/pxtblocks/plugins/logic/ifElse.ts @@ -1,5 +1,6 @@ import * as Blockly from "blockly"; import { InlineSvgsExtensionBlock } from "../functions"; +import { FieldImageNoText } from "../../fields/field_imagenotext"; type IfElseMixinType = typeof IF_ELSE_MIXIN; @@ -162,7 +163,7 @@ const IF_ELSE_MIXIN = { .appendField(Blockly.Msg.CONTROLS_IF_MSG_THEN); this.appendDummyInput('IFBUTTONS' + i) .appendField( - new Blockly.FieldImage(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", removeElseIf, false)) + new FieldImageNoText(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", removeElseIf, false)) .setAlign(Blockly.inputs.Align.RIGHT); this.appendStatementInput('DO' + i); } @@ -172,7 +173,7 @@ const IF_ELSE_MIXIN = { this.appendDummyInput('ELSEBUTTONS') .setAlign(Blockly.inputs.Align.RIGHT) .appendField( - new Blockly.FieldImage(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", this.removeElse_.bind(this), false)); + new FieldImageNoText(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", this.removeElse_.bind(this), false)); this.appendStatementInput('ELSE'); } if (this.getInput('ADDBUTTON')) this.removeInput('ADDBUTTON'); @@ -189,7 +190,7 @@ const IF_ELSE_MIXIN = { }(); this.appendDummyInput('ADDBUTTON') .appendField( - new Blockly.FieldImage(this.ADD_IMAGE_DATAURI, 24, 24, "*", addElseIf, false)); + new FieldImageNoText(this.ADD_IMAGE_DATAURI, 24, 24, "*", addElseIf, false)); }, /** * Reconstructs the block with all child blocks attached. From 07f34effed855b0b05991c92ae85bda6d153efca Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 15:03:42 -0700 Subject: [PATCH 28/91] Hide hidden fields from block display string --- pxtblocks/getBlockText.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pxtblocks/getBlockText.ts b/pxtblocks/getBlockText.ts index 21d294580e20..b9b0f9495bf0 100644 --- a/pxtblocks/getBlockText.ts +++ b/pxtblocks/getBlockText.ts @@ -4,8 +4,16 @@ import { FieldCustom } from "./fields/field_utils"; export function getBlockText(block: Blockly.BlockSvg): string { const fieldValues = []; for (const input of block.inputList) { + if (!input.isVisible()) { + continue; + } + if (input.fieldRow.length > 0) { for (const field of input.fieldRow) { + if (!field.isVisible()) { + continue; + } + const text = (field as FieldCustom & Blockly.Field).isFieldCustom_ ? (field as any).getFieldDescription() : field.getText(); From 333feb99e4a7ee51122f8b1986559ae5cc7e2d9e Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 15:04:06 -0700 Subject: [PATCH 29/91] Adjust melody representation --- pxtblocks/fields/field_melodySandbox.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pxtblocks/fields/field_melodySandbox.ts b/pxtblocks/fields/field_melodySandbox.ts index ba3d2d9cc493..36d42ea5c414 100644 --- a/pxtblocks/fields/field_melodySandbox.ts +++ b/pxtblocks/fields/field_melodySandbox.ts @@ -116,7 +116,8 @@ export class FieldCustomMelody extends Blockly.Fie } getFieldDescription(): string { - return lf("melody"); + const melodyString = this.melody.getStringRepresentation()?.replace(/-/g, "")?.trim(); + return melodyString || lf("empty"); } // This will be run when the field is created (i.e. when it appears on the workspace) From bdc032a7c023d49ede314c7250ebbadf1bbf6047 Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 17:27:23 -0700 Subject: [PATCH 30/91] Do not show blocks inside the 'mouth' when getting block text. --- pxtblocks/getBlockText.ts | 54 +++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/pxtblocks/getBlockText.ts b/pxtblocks/getBlockText.ts index b9b0f9495bf0..b8a62f93d5ed 100644 --- a/pxtblocks/getBlockText.ts +++ b/pxtblocks/getBlockText.ts @@ -1,37 +1,41 @@ import * as Blockly from "blockly"; import { FieldCustom } from "./fields/field_utils"; +/** + * Provides a string representation of the text a user would normally see on a block. + */ export function getBlockText(block: Blockly.BlockSvg): string { - const fieldValues = []; - for (const input of block.inputList) { - if (!input.isVisible()) { - continue; - } - - if (input.fieldRow.length > 0) { - for (const field of input.fieldRow) { - if (!field.isVisible()) { - continue; + const fieldValues = []; + for (const input of block.inputList) { + if (!input.isVisible()) { + continue; } - const text = (field as FieldCustom & Blockly.Field).isFieldCustom_ - ? (field as any).getFieldDescription() - : field.getText(); + if (input.fieldRow.length > 0) { + for (const field of input.fieldRow) { + if (!field.isVisible()) { + continue; + } - if (text) { - fieldValues.push(text); + const text = (field as FieldCustom & Blockly.Field).isFieldCustom_ + ? (field as any).getFieldDescription() + : field.getText(); + + if (text) { + fieldValues.push(text); + } + } } - } - // Check if this input has a connected block - if (input.connection && input.connection.targetBlock()) { - const connectedBlock = input.connection.targetBlock(); - const innerBlockText = getBlockText(connectedBlock as Blockly.BlockSvg); - if (innerBlockText) { - fieldValues.push(`[${innerBlockText}]`); + // Check if this input has a connected block + if (input.connection && input.connection.targetBlock() && input.connection.type === Blockly.INPUT_VALUE) { + const connectedBlock = input.connection.targetBlock(); + const innerBlockText = getBlockText(connectedBlock as Blockly.BlockSvg); + if (innerBlockText) { + fieldValues.push(`[${innerBlockText}]`); + } } - } } - } - return fieldValues.join(" "); + + return fieldValues.join(" "); } From f94e3baae51f22c664c6a7619418e1588fb23fbe Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 17:27:41 -0700 Subject: [PATCH 31/91] Use correct function (and truncate strings) in getDisplayInfoForException --- webapp/src/blocks.tsx | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index c8f41249c1bc..9ecebf022093 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -936,7 +936,11 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (!block) { return undefined; } - const blockText = this.getBlockText(block); + + let blockText = pxtblockly.getBlockText(block); + if (blockText.length > 100) { + blockText = blockText.substring(0, 97) + "..."; + } return { message: blockText ? lf("at the '{0}' block", blockText) : lf("at {0}", locInfo.functionName), @@ -953,21 +957,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { }; } - private getBlockText(block: Blockly.BlockSvg): string { - const fieldValues = []; - for (const input of block.inputList) { - if (input.fieldRow.length > 0) { - for (const field of input.fieldRow) { - const text = field.getText(); - if (text) { - fieldValues.push(text); - } - } - } - } - return fieldValues.join(" "); - } - getBlocksAreaDiv() { return document.getElementById('blocksArea'); } From 6678c90af80c78e9a1d67a51f3f392e7ed9e44ee Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 18:34:58 -0700 Subject: [PATCH 32/91] Fix a few more image fields that shouldn't have display text --- pxtblocks/builtins/functions.ts | 3 ++- pxtblocks/plugins/arrays/createList.ts | 5 +++-- pxtblocks/plugins/text/join.ts | 5 +++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pxtblocks/builtins/functions.ts b/pxtblocks/builtins/functions.ts index 5cc20882d861..bf0ea46ef4c6 100644 --- a/pxtblocks/builtins/functions.ts +++ b/pxtblocks/builtins/functions.ts @@ -10,6 +10,7 @@ import { domToWorkspaceNoEvents } from "../importer"; import { DUPLICATE_ON_DRAG_MUTATION_KEY } from "../plugins/duplicateOnDrag"; import { PathObject } from "../plugins/renderer/pathObject"; +import { FieldImageNoText } from "../fields/field_imagenotext"; export function initFunctions() { const msg = Blockly.Msg; @@ -424,7 +425,7 @@ function initReturnStatement(b: Blockly.Block) { function addButton(name: string, uri: string, alt: string) { b.appendDummyInput(name) - .appendField(new Blockly.FieldImage(uri, 24, 24, alt, () => { + .appendField(new FieldImageNoText(uri, 24, 24, alt, () => { const oldMutation = mutationString(); returnValueVisible = !returnValueVisible; diff --git a/pxtblocks/plugins/arrays/createList.ts b/pxtblocks/plugins/arrays/createList.ts index 764b835d2817..574a5e5ba14d 100644 --- a/pxtblocks/plugins/arrays/createList.ts +++ b/pxtblocks/plugins/arrays/createList.ts @@ -2,6 +2,7 @@ import * as Blockly from "blockly"; import { FUNCTION_CALL_OUTPUT_BLOCK_TYPE } from "../functions/constants"; import { CommonFunctionBlock } from "../functions/commonFunctionMixin"; import { InlineSvgsExtensionBlock } from "../functions"; +import { FieldImageNoText } from "../../fields/field_imagenotext"; type ListCreateMixinType = typeof LIST_CREATE_MIXIN; @@ -220,9 +221,9 @@ const LIST_CREATE_MIXIN = { if (this.getInput('BUTTONS')) this.removeInput('BUTTONS'); const buttons = this.appendDummyInput('BUTTONS'); if (this.itemCount_ > 0) { - buttons.appendField(new Blockly.FieldImage(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", remove, false)); + buttons.appendField(new FieldImageNoText(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", remove, false)); } - buttons.appendField(new Blockly.FieldImage(this.ADD_IMAGE_DATAURI, 24, 24, "*", add, false)); + buttons.appendField(new FieldImageNoText(this.ADD_IMAGE_DATAURI, 24, 24, "*", add, false)); /* Switch to vertical list when the list is too long */ const showHorizontalList = this.itemCount_ <= this.horizontalAfter_; diff --git a/pxtblocks/plugins/text/join.ts b/pxtblocks/plugins/text/join.ts index 219dc162efc9..4cde989ea0c8 100644 --- a/pxtblocks/plugins/text/join.ts +++ b/pxtblocks/plugins/text/join.ts @@ -1,5 +1,6 @@ import * as Blockly from "blockly"; import { InlineSvgsExtensionBlock } from "../functions"; +import { FieldImageNoText } from "../../fields/field_imagenotext"; type TextJoinMixinType = typeof TEXT_JOIN_MUTATOR_MIXIN; @@ -143,9 +144,9 @@ const TEXT_JOIN_MUTATOR_MIXIN = { if (this.getInput('BUTTONS')) this.removeInput('BUTTONS'); const buttons = this.appendDummyInput('BUTTONS'); if (this.itemCount_ > 1) { - buttons.appendField(new Blockly.FieldImage(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", remove, false)); + buttons.appendField(new FieldImageNoText(this.REMOVE_IMAGE_DATAURI, 24, 24, "*", remove, false)); } - buttons.appendField(new Blockly.FieldImage(this.ADD_IMAGE_DATAURI, 24, 24, "*", add, false)); + buttons.appendField(new FieldImageNoText(this.ADD_IMAGE_DATAURI, 24, 24, "*", add, false)); // Switch to vertical list when there are too many items const horizontalLayout = this.itemCount_ <= 4; From 5fca6e2dce874097707429ad409668782c4ee9b4 Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 30 Apr 2025 18:35:08 -0700 Subject: [PATCH 33/91] Remove test code --- pxtblocks/contextMenu/blockItems.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/pxtblocks/contextMenu/blockItems.ts b/pxtblocks/contextMenu/blockItems.ts index 3cc5916f5904..487578a05e82 100644 --- a/pxtblocks/contextMenu/blockItems.ts +++ b/pxtblocks/contextMenu/blockItems.ts @@ -1,7 +1,6 @@ /// import * as Blockly from "blockly"; import { openHelpUrl } from "../external"; -import { getBlockText } from "../getBlockText"; // Lower weight is higher in context menu export enum BlockContextWeight { @@ -24,24 +23,6 @@ export function registerBlockitems() { registerCollapseExpandBlock(); registerHelp(); - // TODO : Remove - this is just temporary to easily view block strings while testing - const tempDisplayBlockText: Blockly.ContextMenuRegistry.RegistryItem = { - displayText(scope: Blockly.ContextMenuRegistry.Scope) { - return getBlockText(scope.block); - }, - preconditionFn(scope: Blockly.ContextMenuRegistry.Scope) { - return "enabled"; - }, - callback(scope: Blockly.ContextMenuRegistry.Scope) { - const blockText = getBlockText(scope.block); - console.log(blockText); - }, - scopeType: Blockly.ContextMenuRegistry.ScopeType.BLOCK, - id: 'tempDisplayBlockText', - weight: 1, - }; - Blockly.ContextMenuRegistry.registry.register(tempDisplayBlockText); - // Fix the weights of the builtin options we do use Blockly.ContextMenuRegistry.registry.getItem("blockDelete").weight = BlockContextWeight.DeleteBlock; Blockly.ContextMenuRegistry.registry.getItem("blockComment").weight = BlockContextWeight.AddComment; From 2c7afdda86875197e0736308ac2ec23c33610b29 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 1 May 2025 10:01:05 -0700 Subject: [PATCH 34/91] Make getFieldDescription optional --- pxtblocks/fields/field_utils.ts | 2 +- pxtblocks/getBlockText.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pxtblocks/fields/field_utils.ts b/pxtblocks/fields/field_utils.ts index fd94e10964ef..a03df4843a77 100644 --- a/pxtblocks/fields/field_utils.ts +++ b/pxtblocks/fields/field_utils.ts @@ -13,7 +13,7 @@ export interface FieldCustom { /** * Returns a human-readable description of the field's current value. */ - getFieldDescription(): string; + getFieldDescription?(): string; } export interface FieldCustomOptions { diff --git a/pxtblocks/getBlockText.ts b/pxtblocks/getBlockText.ts index b8a62f93d5ed..75b14860b727 100644 --- a/pxtblocks/getBlockText.ts +++ b/pxtblocks/getBlockText.ts @@ -17,8 +17,8 @@ export function getBlockText(block: Blockly.BlockSvg): string { continue; } - const text = (field as FieldCustom & Blockly.Field).isFieldCustom_ - ? (field as any).getFieldDescription() + const text = (field as FieldCustom & Blockly.Field).getFieldDescription + ? (field as FieldCustom & Blockly.Field).getFieldDescription() : field.getText(); if (text) { From 5ad568e33f76ddae54283796fe00f68e0124a6b7 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 1 May 2025 13:36:04 -0700 Subject: [PATCH 35/91] Consistent way of highlighting and centering on blocks when clicking in the problem window. --- webapp/src/blocks.tsx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 9ecebf022093..a7b257f3c0ba 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -905,14 +905,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { return { message: message, onClick: () => { - // Block may already be highlighted, so we want to set highlighted directly to true - // as opposed to using `highlightBlock`, which toggles. - const block = this.editor.getBlockById(blockId); - if (!block) { - return; - } - - block.setHighlighted(true); + this.clearHighlightedStatements(); + this.editor.highlightBlock(blockId); this.editor.centerOnBlock(blockId, true); } } @@ -946,7 +940,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { message: blockText ? lf("at the '{0}' block", blockText) : lf("at {0}", locInfo.functionName), onClick: () => { this.clearHighlightedStatements(); - this.highlightStatement(locInfo, exception); + this.editor.highlightBlock(blockId); + this.editor.centerOnBlock(blockId, true); } }; }).filter(f => !!f) ?? undefined; From 385cbb5e1aeecf0d3e7a4841cc1fc8fa57db669a Mon Sep 17 00:00:00 2001 From: thsparks Date: Fri, 2 May 2025 12:45:22 -0700 Subject: [PATCH 36/91] Update code fence removal --- webapp/src/blocks.tsx | 1 + webapp/src/components/errorHelpButton.tsx | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index a3455b1db027..17f3a851581c 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -962,6 +962,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { stackFrames }; } + private handleErrorHelp(helpResponse: ErrorHelpResponse) { if (helpResponse.explanationAsTour) { this.parent.showTour(helpResponse.explanationAsTour); diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx index 9097e9cb41ee..595b8e485565 100644 --- a/webapp/src/components/errorHelpButton.tsx +++ b/webapp/src/components/errorHelpButton.tsx @@ -89,8 +89,8 @@ export class ErrorHelpButton extends React.Component< console.log("AI response", response); - // If the response contains any leading or trailing ` characters, remove them - response = response.trim().replace(/^`+|`+$/g, ""); + // If the response contains markdown code "fencing" (i.e. ```json ```), remove it + response = response.trim().replace(/^`{3,}(?:\w+)?\s*|`{3,}$/g, ""); const parsedResponse = pxt.Util.jsonTryParse( response From 0f618eaeb60623f087e62017f14fc7a869eacd95 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 6 May 2025 16:36:23 -0700 Subject: [PATCH 37/91] Allow for unset targetQueries in editor tour --- localtypings/pxtarget.d.ts | 2 +- react-common/components/controls/TeachingBubble.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index 7bc646d67499..b5b94c081ef7 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1341,7 +1341,7 @@ declare namespace pxt.tour { interface BubbleStep { title: string; description: string; - targetQuery: string; + targetQuery?: string; location: BubbleLocation; sansQuery?: string; // Use this to exclude an element from the cutout sansLocation?: BubbleLocation; // relative location of element to exclude diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index 145497cc7719..1d353e7a5b99 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -87,7 +87,7 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { bubbleArrowOutline.style.border = "none"; const bubbleBounds = bubble.getBoundingClientRect(); let cutoutBounds: CutoutBounds; - if (targetContent.targetQuery === "nothing") { + if (!targetContent.targetQuery || targetContent.targetQuery === "nothing") { cutoutBounds = { top: window.innerHeight / 2, bottom: 0, From cdf1e42b419785cbb6312c590e4ec4e48df9ae64 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 6 May 2025 16:36:46 -0700 Subject: [PATCH 38/91] Basic help button styling --- theme/errorList.less | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/theme/errorList.less b/theme/errorList.less index ea5439be4be3..2203b807a08c 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -108,6 +108,11 @@ } } } + + .error-help-button { + margin: 0 1rem; + padding: 0.7rem; + } } // collapsed error list From d0ee6fd2c24ac5abbd8e39545afb50fe4a5853ef Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 6 May 2025 16:37:30 -0700 Subject: [PATCH 39/91] Update content sent to AI --- webapp/src/components/errorHelpButton.tsx | 67 ++++++++++++++--------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx index 595b8e485565..633e93663f2a 100644 --- a/webapp/src/components/errorHelpButton.tsx +++ b/webapp/src/components/errorHelpButton.tsx @@ -38,16 +38,18 @@ export class ErrorHelpButton extends React.Component< this.aiErrorExplainRequest = this.aiErrorExplainRequest.bind(this); this.parseResponse = this.parseResponse.bind(this); this.getErrorsAsText = this.getErrorsAsText.bind(this); + this.cleanBlocksCode = this.cleanBlocksCode.bind(this); } async aiErrorExplainRequest( code: string, - errors: string[], + errors: string, lang: string, - target: string + target: string, + outputFormat: string ): Promise { const url = `/api/copilot/explainerror`; - const data = { code, errors, lang, target }; + const data = { lang, code, errors, target, outputFormat }; let result: string = ""; const request = await pxt.auth.AuthClient.staticApiAsync(url, data, "POST"); @@ -98,26 +100,28 @@ export class ErrorHelpButton extends React.Component< if (parsedResponse) { const validBlockIds = this.props.parent.getBlocks().map((b) => b.id); for (const item of parsedResponse) { - // Check each blockId is valid, and if so, add a tour step for it + const tourStep = { + title: lf("Error Explanation"), + description: item.message, + location: pxt.tour.BubbleLocation.Center + } as pxt.tour.BubbleStep; + + // Check each blockId is specified & valid, and if so, add a tour step for it const blockId = item.blockId; - if (validBlockIds.includes(blockId)) { - const tourStep = { - title: lf("Error Explanation"), - description: item.message, - targetQuery: `g[data-id="${blockId}"]`, - location: pxt.tour.BubbleLocation.Right, - onStepBegin: () => { + if (blockId) { + if (validBlockIds.includes(blockId)) { + tourStep.targetQuery = `g[data-id="${blockId}"]`; + tourStep.location = pxt.tour.BubbleLocation.Right; + tourStep.onStepBegin = () => { (this.props.parent.editor as Editor).editor.centerOnBlock(blockId); } - // sansQuery?: string; // Use this to exclude an element from the cutout - // sansLocation?: BubbleLocation; // relative location of element to exclude - } as pxt.tour.BubbleStep; - - explanationAsTour.push(tourStep); - } else { - // TODO - handle this case - console.error("Invalid blockId in AI response", blockId, parsedResponse, response); + } else { + // TODO - handle this case + console.error("Invalid blockId in AI response", blockId, parsedResponse, response); + } } + + explanationAsTour.push(tourStep); } } else { explanationAsText = response; // TODO - handle this case better @@ -134,14 +138,12 @@ export class ErrorHelpButton extends React.Component< this.setState({ loadingHelp: true, }); - - const mainFileName = this.props.parent.isBlocksActive() - ? pxt.MAIN_BLOCKS - : pxt.MAIN_TS; +; const lang = this.props.parent.isBlocksActive() ? "blocks" : "typescript"; const target = pxt.appTarget.nickname || pxt.appTarget.name; const mainPkg = pkg.mainEditorPkg(); - const code = (this.props.parent.isBlocksActive() ? (this.props.parent.editor as Editor).getWorkspaceXmlWithIds() : mainPkg.files[mainFileName]?.content) ?? ""; + const code = (this.props.parent.isBlocksActive() ? this.cleanBlocksCode((this.props.parent.editor as Editor).getWorkspaceXmlWithIds()) : mainPkg.files[pxt.MAIN_TS]?.content) ?? ""; + const outputFormat = this.props.parent.isBlocksActive() ? "tour_json" : "text"; console.log("Code", code); const errors = this.getErrorsAsText(); @@ -153,7 +155,8 @@ export class ErrorHelpButton extends React.Component< code, errors, lang, - target + target, + outputFormat ); const parsedResponse = this.parseResponse(response); @@ -161,8 +164,18 @@ export class ErrorHelpButton extends React.Component< this.props.onHelpResponse?.(parsedResponse); } - getErrorsAsText(): string[] { - return this.props.errors.map((error) => error.message); + cleanBlocksCode(code: string): string { + // Remove any image content (it's bulky and not useful for the AI) + const updatedCode = code.replace(/img`[^`]*`/g, "img`[trimmed for brevity]`"); + return updatedCode; + } + + /** + * Converts the list of errors into a string format for the AI to reference. + * Adds block ids to the error message if they exist. + */ + getErrorsAsText(): string { + return JSON.stringify(this.props.errors); } render() { From 1ebb6f7012ee97ac3746ae414f953db11003a8e9 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 6 May 2025 16:37:49 -0700 Subject: [PATCH 40/91] Allow errors to contain metadata like the block id --- webapp/src/blocks.tsx | 6 ++++++ webapp/src/errorList.tsx | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 469ae6e0902c..64007d3d6b29 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -920,6 +920,9 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.clearHighlightedStatements(); this.editor.highlightBlock(blockId); this.editor.centerOnBlock(blockId, true); + }, + metadata: { + blockId: blockId } } } @@ -954,6 +957,9 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.clearHighlightedStatements(); this.editor.highlightBlock(blockId); this.editor.centerOnBlock(blockId, true); + }, + metadata: { + blockId: blockId } }; }).filter(f => !!f) ?? undefined; diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 857e80e9e559..36fed310d10a 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -7,6 +7,13 @@ import { classList } from "../../react-common/components/util"; import { ErrorHelpButton, ErrorHelpResponse } from "./components/errorHelpButton"; import { Button } from "../../react-common/components/controls/Button"; +/** + * A collection of optional metadata that can be attached to an error. + */ +type ErrorMetadata = { + blockId?: string, +} + type GroupedError = { error: ErrorDisplayInfo, count: number, @@ -15,12 +22,14 @@ type GroupedError = { export type StackFrameDisplayInfo = { message: string, + metadata?: ErrorMetadata, onClick?: () => void, } export type ErrorDisplayInfo = { message: string, stackFrames?: StackFrameDisplayInfo[], + metadata?: ErrorMetadata, onClick?: () => void }; From f4836bd29cb737e79ddbb4c5dc3e3e636101a5af Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 6 May 2025 17:26:12 -0700 Subject: [PATCH 41/91] Basic (temporary) text display. Response handling needs significant cleanup. --- theme/errorList.less | 6 ++++++ webapp/src/errorList.tsx | 19 ++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/theme/errorList.less b/theme/errorList.less index 2203b807a08c..feaf04f56106 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -113,6 +113,12 @@ margin: 0 1rem; padding: 0.7rem; } + + .explanationText { + margin: 1rem; + padding: 0.5rem; + background-color: var(--pxt-neutral-alpha10); + } } // collapsed error list diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 36fed310d10a..890a943cb5ec 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -42,6 +42,7 @@ export interface ErrorListProps { } export interface ErrorListState { isCollapsed: boolean, + responseText?: string, } export class ErrorList extends React.Component { @@ -50,7 +51,8 @@ export class ErrorList extends React.Component { super(props); this.state = { - isCollapsed: true + isCollapsed: true, + responseText: undefined, } this.onCollapseClick = this.onCollapseClick.bind(this); @@ -74,14 +76,23 @@ export class ErrorList extends React.Component { } handleHelpResponse = (response: ErrorHelpResponse) => { + // TODO thsparks : Clean up this mess. + if (this.props.onHelpResponse) { this.props.onHelpResponse(response); } + + if (response.explanationAsText) { + this.setState({ + responseText: response.explanationAsText + }); + } + } render() { const { startDebugger, errors, onHelpResponse } = this.props; - const { isCollapsed } = this.state; + const { isCollapsed, responseText } = this.state; const errorsAvailable = !!errors?.length; const groupedErrors = groupErrors(errors); @@ -94,7 +105,7 @@ export class ErrorList extends React.Component {

{lf("Problems")}

{errorCount}
- {onHelpResponse && } +
@@ -105,6 +116,8 @@ export class ErrorList extends React.Component {
} + {responseText &&
{responseText}
} +
{errorListContent}
From ccdc493b95a2aabfcf36c0f49c0805e9940bcb16 Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 12 May 2025 15:43:31 -0700 Subject: [PATCH 42/91] Beginning some code cleanup/refactoring, still in progress... --- webapp/src/blocks.tsx | 9 -- webapp/src/cloud.ts | 31 +++++ webapp/src/components/errorHelpButton.tsx | 148 ++-------------------- webapp/src/errorHelp.ts | 122 ++++++++++++++++++ webapp/src/errorList.tsx | 19 ++- 5 files changed, 169 insertions(+), 160 deletions(-) create mode 100644 webapp/src/errorHelp.ts diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 055dc6eb292c..e7559fd8713d 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -40,7 +40,6 @@ import { Measurements } from "./constants"; import { flow } from "../../pxtblocks"; import { HIDDEN_CLASS_NAME } from "../../pxtblocks/plugins/flyout/blockInflater"; import { FlyoutButton } from "../../pxtblocks/plugins/flyout/flyoutButton"; -import { ErrorHelpResponse } from "./components/errorHelpButton"; interface CopyDataEntry { version: 1; @@ -86,7 +85,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.onErrorListResize = this.onErrorListResize.bind(this) this.getDisplayInfoForBlockError = this.getDisplayInfoForBlockError.bind(this) this.getDisplayInfoForException = this.getDisplayInfoForException.bind(this) - this.handleErrorHelp = this.handleErrorHelp.bind(this) } setBreakpointsMap(breakpoints: pxtc.Breakpoint[], procCallLocations: pxtc.LocationInfo[]): void { if (!breakpoints || !this.compilationResult) return; @@ -929,7 +927,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { {showErrorList && }
) @@ -1005,12 +1002,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { }; } - private handleErrorHelp(helpResponse: ErrorHelpResponse) { - if (helpResponse.explanationAsTour) { - this.parent.showTour(helpResponse.explanationAsTour); - } - } - getBlocksAreaDiv() { return document.getElementById('blocksArea'); } diff --git a/webapp/src/cloud.ts b/webapp/src/cloud.ts index 4548c215b172..c7abd1f34b13 100644 --- a/webapp/src/cloud.ts +++ b/webapp/src/cloud.ts @@ -678,4 +678,35 @@ export function init() { // subscribe to header changes data.subscribe(onHeaderChangeSubscriber, "header:*"); +} + +/** + * Copilot / AI Requests + */ +export async function aiErrorExplainRequest( + code: string, + errors: string, + lang: string, + target: string, + outputFormat: string +): Promise { + const url = `/api/copilot/explainerror`; + const data = { lang, code, errors, target, outputFormat }; + let result: string = ""; + + const request = await auth.apiAsync(url, data, "POST"); + if (!request.success) { + // TODO thsparks : different errors for different responses, 403, throttling, etc... + throw new Error( + request.err || + lf( + "Unable to reach AI. Error: {0}.\n{1}", + request.statusCode, + request.err + ) + ); + } + result = await request.resp; + + return result; } \ No newline at end of file diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx index 633e93663f2a..02e8bb601ee9 100644 --- a/webapp/src/components/errorHelpButton.tsx +++ b/webapp/src/components/errorHelpButton.tsx @@ -1,16 +1,11 @@ import * as React from "react"; -import * as pkg from "../package"; import { ErrorDisplayInfo } from "../errorList"; import { Button } from "../../../react-common/components/controls/Button"; import { classList } from "../../../react-common/components/util"; -import { Editor } from "../blocks"; +import { ErrorHelper, ErrorHelpResponse } from "../errorHelp"; // TODO thsparks - there is probably a better way to do this. -export interface ErrorHelpResponse { - explanationAsText: string; - explanationAsTour: pxt.tour.BubbleStep[]; -} export interface ErrorHelpButtonProps { parent: pxt.editor.IProjectView; @@ -35,147 +30,20 @@ export class ErrorHelpButton extends React.Component< }; this.handleHelpClick = this.handleHelpClick.bind(this); - this.aiErrorExplainRequest = this.aiErrorExplainRequest.bind(this); - this.parseResponse = this.parseResponse.bind(this); - this.getErrorsAsText = this.getErrorsAsText.bind(this); - this.cleanBlocksCode = this.cleanBlocksCode.bind(this); - } - - async aiErrorExplainRequest( - code: string, - errors: string, - lang: string, - target: string, - outputFormat: string - ): Promise { - const url = `/api/copilot/explainerror`; - const data = { lang, code, errors, target, outputFormat }; - let result: string = ""; - - const request = await pxt.auth.AuthClient.staticApiAsync(url, data, "POST"); - if (!request.success) { - throw new Error( - request.err || - lf( - "Unable to reach AI. Error: {0}.\n{1}", - request.statusCode, - request.err - ) - ); - } - result = await request.resp; - - return result; - } - - parseResponse(response: string): ErrorHelpResponse { - // We ask the AI to provide an ordered JSON list with blockIds tied to explanations for each block. - // For example: - // [ - // { - // "blockId": "sampleblockid", - // "message": "Here is a helpful message walking you through the error. This one will be displayed first." - // }, - // { - // "blockId": "sampleblockid2", - // "message": "Here is another helpful message walking you through the error. This one will be displayed second." - // } - // ] - interface AiResponseJsonFormat { - blockId: string; - message: string; - } - - let explanationAsText = ""; - let explanationAsTour: pxt.tour.BubbleStep[] = []; - - console.log("AI response", response); - - // If the response contains markdown code "fencing" (i.e. ```json ```), remove it - response = response.trim().replace(/^`{3,}(?:\w+)?\s*|`{3,}$/g, ""); - - const parsedResponse = pxt.Util.jsonTryParse( - response - ) as AiResponseJsonFormat[]; - if (parsedResponse) { - const validBlockIds = this.props.parent.getBlocks().map((b) => b.id); - for (const item of parsedResponse) { - const tourStep = { - title: lf("Error Explanation"), - description: item.message, - location: pxt.tour.BubbleLocation.Center - } as pxt.tour.BubbleStep; - - // Check each blockId is specified & valid, and if so, add a tour step for it - const blockId = item.blockId; - if (blockId) { - if (validBlockIds.includes(blockId)) { - tourStep.targetQuery = `g[data-id="${blockId}"]`; - tourStep.location = pxt.tour.BubbleLocation.Right; - tourStep.onStepBegin = () => { - (this.props.parent.editor as Editor).editor.centerOnBlock(blockId); - } - } else { - // TODO - handle this case - console.error("Invalid blockId in AI response", blockId, parsedResponse, response); - } - } - - explanationAsTour.push(tourStep); - } - } else { - explanationAsText = response; // TODO - handle this case better - } - - return { - explanationAsText, - explanationAsTour: - explanationAsTour.length > 0 ? explanationAsTour : undefined, - }; } async handleHelpClick() { this.setState({ loadingHelp: true, }); -; - const lang = this.props.parent.isBlocksActive() ? "blocks" : "typescript"; - const target = pxt.appTarget.nickname || pxt.appTarget.name; - const mainPkg = pkg.mainEditorPkg(); - const code = (this.props.parent.isBlocksActive() ? this.cleanBlocksCode((this.props.parent.editor as Editor).getWorkspaceXmlWithIds()) : mainPkg.files[pxt.MAIN_TS]?.content) ?? ""; - const outputFormat = this.props.parent.isBlocksActive() ? "tour_json" : "text"; - console.log("Code", code); - - const errors = this.getErrorsAsText(); - console.log("Errors", errors); - - // Call to backend API with code and errors, then set the explanation state - // to the response from the backend - const response = await this.aiErrorExplainRequest( - code, - errors, - lang, - target, - outputFormat - ); - const parsedResponse = this.parseResponse(response); - - this.props.onHelpResponse?.(parsedResponse); - } - - cleanBlocksCode(code: string): string { - // Remove any image content (it's bulky and not useful for the AI) - const updatedCode = code.replace(/img`[^`]*`/g, "img`[trimmed for brevity]`"); - return updatedCode; - } - - /** - * Converts the list of errors into a string format for the AI to reference. - * Adds block ids to the error message if they exist. - */ - getErrorsAsText(): string { - return JSON.stringify(this.props.errors); + const helper: ErrorHelper = new ErrorHelper(this.props.parent); + try { + const response = await helper.getHelpAsync(this.props.errors); + this.props.onHelpResponse?.(response); + } catch (e) { + // TODO thsparks - handle error + } } render() { diff --git a/webapp/src/errorHelp.ts b/webapp/src/errorHelp.ts new file mode 100644 index 000000000000..50e6c3a5f196 --- /dev/null +++ b/webapp/src/errorHelp.ts @@ -0,0 +1,122 @@ +import * as pkg from "./package"; +import { Editor } from "./blocks"; +import { ErrorDisplayInfo } from "./errorList"; +import { aiErrorExplainRequest } from "./cloud"; + +export interface ErrorHelpResponse { + explanationAsText: string; + explanationAsTour: pxt.tour.BubbleStep[]; +} + +export class ErrorHelper { + + parent: pxt.editor.IProjectView; + constructor(parent: pxt.editor.IProjectView) { + this.parent = parent; + } + + protected parseResponse(response: string): ErrorHelpResponse { + // We ask the AI to provide an ordered JSON list with blockIds tied to explanations for each block. + // For example: + // [ + // { + // "blockId": "sampleblockid", + // "message": "Here is a helpful message walking you through the error. This one will be displayed first." + // }, + // { + // "blockId": "sampleblockid2", + // "message": "Here is another helpful message walking you through the error. This one will be displayed second." + // } + // ] + interface AiResponseJsonFormat { + blockId: string; + message: string; + } + + let explanationAsText = ""; + let explanationAsTour: pxt.tour.BubbleStep[] = []; + + console.log("AI response", response); + + // If the response contains markdown code "fencing" (i.e. ```json ```), remove it + response = response.trim().replace(/^`{3,}(?:\w+)?\s*|`{3,}$/g, ""); + + const parsedResponse = pxt.Util.jsonTryParse( + response + ) as AiResponseJsonFormat[]; + if (parsedResponse) { + const validBlockIds = this.parent.getBlocks().map((b) => b.id); + for (const item of parsedResponse) { + const tourStep = { + title: lf("Error Explanation"), + description: item.message, + location: pxt.tour.BubbleLocation.Center + } as pxt.tour.BubbleStep; + + // Check each blockId is specified & valid, and if so, add a tour step for it + const blockId = item.blockId; + if (blockId) { + if (validBlockIds.includes(blockId)) { + tourStep.targetQuery = `g[data-id="${blockId}"]`; + tourStep.location = pxt.tour.BubbleLocation.Right; + tourStep.onStepBegin = () => { + (this.parent.editor as Editor).editor.centerOnBlock(blockId); + } + } else { + // TODO - handle this case + console.error("Invalid blockId in AI response", blockId, parsedResponse, response); + } + } + + explanationAsTour.push(tourStep); + } + } else { + explanationAsText = response; // TODO - handle this case better + } + + return { + explanationAsText, + explanationAsTour: + explanationAsTour.length > 0 ? explanationAsTour : undefined, + }; + } + + /** + * Converts the list of errors into a string format for the AI to reference. + * Adds block ids to the error message if they exist. + */ + protected getErrorsAsText(errors: ErrorDisplayInfo[]): string { + return JSON.stringify(errors); + } + + protected cleanBlocksCode(code: string): string { + // Remove any image content (it's bulky and not useful for the AI) + const updatedCode = code.replace(/img`[^`]*`/g, "img`[trimmed for brevity]`"); + return updatedCode; + } + + async getHelpAsync(errors: ErrorDisplayInfo[]): Promise { + const lang = this.parent.isBlocksActive() ? "blocks" : "typescript"; + const target = pxt.appTarget.nickname || pxt.appTarget.name; + const mainPkg = pkg.mainEditorPkg(); + const code = (this.parent.isBlocksActive() ? this.cleanBlocksCode((this.parent.editor as Editor).getWorkspaceXmlWithIds()) : mainPkg.files[pxt.MAIN_TS]?.content) ?? ""; + const outputFormat = this.parent.isBlocksActive() ? "tour_json" : "text"; + console.log("Code", code); + + const errString = this.getErrorsAsText(errors); + console.log("Errors", errString); + + // Call to backend API with code and errors, then set the explanation state + // to the response from the backend + const response = await aiErrorExplainRequest( + code, + errString, + lang, + target, + outputFormat + ); + + const parsedResponse = this.parseResponse(response); + return parsedResponse; + } +} diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 890a943cb5ec..e2fbaec118f4 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -4,8 +4,9 @@ import * as React from "react"; import * as sui from "./sui"; import { fireClickOnEnter } from "./util"; import { classList } from "../../react-common/components/util"; -import { ErrorHelpButton, ErrorHelpResponse } from "./components/errorHelpButton"; +import { ErrorHelpButton } from "./components/errorHelpButton"; import { Button } from "../../react-common/components/controls/Button"; +import { ErrorHelpResponse } from "./errorHelp"; /** * A collection of optional metadata that can be attached to an error. @@ -38,7 +39,6 @@ export interface ErrorListProps { parent: pxt.editor.IProjectView; errors: ErrorDisplayInfo[]; startDebugger?: () => void; - onHelpResponse?: (response: ErrorHelpResponse) => void; } export interface ErrorListState { isCollapsed: boolean, @@ -76,22 +76,19 @@ export class ErrorList extends React.Component { } handleHelpResponse = (response: ErrorHelpResponse) => { - // TODO thsparks : Clean up this mess. - - if (this.props.onHelpResponse) { - this.props.onHelpResponse(response); - } - - if (response.explanationAsText) { + if (response.explanationAsTour) { + this.props.parent.showTour(response.explanationAsTour); + } else if (response.explanationAsText) { this.setState({ responseText: response.explanationAsText }); + } else { + // TODO thsparks - Error } - } render() { - const { startDebugger, errors, onHelpResponse } = this.props; + const { startDebugger, errors } = this.props; const { isCollapsed, responseText } = this.state; const errorsAvailable = !!errors?.length; From 4a1b73128b2300d272143b0f7809b8dde7c21c1e Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 12 May 2025 16:48:33 -0700 Subject: [PATCH 43/91] Some more refactoring --- webapp/src/errorHelp.ts | 103 ++++++++++++++++++++++++++------------- webapp/src/errorList.tsx | 29 +++++++++-- 2 files changed, 94 insertions(+), 38 deletions(-) diff --git a/webapp/src/errorHelp.ts b/webapp/src/errorHelp.ts index 50e6c3a5f196..4ae857120be2 100644 --- a/webapp/src/errorHelp.ts +++ b/webapp/src/errorHelp.ts @@ -3,9 +3,14 @@ import { Editor } from "./blocks"; import { ErrorDisplayInfo } from "./errorList"; import { aiErrorExplainRequest } from "./cloud"; +export interface ErrorExplanationStep { + message: string; // The message to display + elementId?: string; // The id of the element on the page to highlight + onStepBegin?: () => void; +} + export interface ErrorHelpResponse { - explanationAsText: string; - explanationAsTour: pxt.tour.BubbleStep[]; + explanationSteps: ErrorExplanationStep[]; } export class ErrorHelper { @@ -15,26 +20,32 @@ export class ErrorHelper { this.parent = parent; } - protected parseResponse(response: string): ErrorHelpResponse { - // We ask the AI to provide an ordered JSON list with blockIds tied to explanations for each block. - // For example: - // [ - // { - // "blockId": "sampleblockid", - // "message": "Here is a helpful message walking you through the error. This one will be displayed first." - // }, - // { - // "blockId": "sampleblockid2", - // "message": "Here is another helpful message walking you through the error. This one will be displayed second." - // } - // ] + /** + * Converts the AI response data into our structured ErrorHelpResponse format. + * This is used when the AI returns a JSON "tour" response. + */ + protected parseTourResponse(response: string): ErrorHelpResponse { + /* + The AI either provides a JSON list with blockIds tied to explanations for each block. + For example: + [ + { + "blockId": "sampleblockid", + "message": "Here is a helpful message walking you through the error. This one will be displayed first." + }, + { + "blockId": "sampleblockid2", + "message": "Here is another helpful message walking you through the error. This one will be displayed second." + } + ] + */ + interface AiResponseJsonFormat { blockId: string; message: string; } - let explanationAsText = ""; - let explanationAsTour: pxt.tour.BubbleStep[] = []; + let explanationSteps: ErrorExplanationStep[] = []; console.log("AI response", response); @@ -44,40 +55,55 @@ export class ErrorHelper { const parsedResponse = pxt.Util.jsonTryParse( response ) as AiResponseJsonFormat[]; + if (parsedResponse) { + // TODO thsparks - Can we move "validation" of a response up to the parent editor? const validBlockIds = this.parent.getBlocks().map((b) => b.id); for (const item of parsedResponse) { - const tourStep = { - title: lf("Error Explanation"), - description: item.message, - location: pxt.tour.BubbleLocation.Center - } as pxt.tour.BubbleStep; + const step: ErrorExplanationStep = { + message: item.message + }; // Check each blockId is specified & valid, and if so, add a tour step for it const blockId = item.blockId; if (blockId) { if (validBlockIds.includes(blockId)) { - tourStep.targetQuery = `g[data-id="${blockId}"]`; - tourStep.location = pxt.tour.BubbleLocation.Right; - tourStep.onStepBegin = () => { + step.elementId = `g[data-id="${blockId}"]`; + step.onStepBegin = () => { (this.parent.editor as Editor).editor.centerOnBlock(blockId); - } + }; } else { - // TODO - handle this case - console.error("Invalid blockId in AI response", blockId, parsedResponse, response); + + // TODO thsparks - Maybe? + // Try to repair the block id. The AI sometimes sends variable ids instead of blockIds. + // If that variable id is only used in one block, we can assume that is the block we should point to. + // https://developers.google.com/blockly/reference/js/blockly.workspace_class.getvariableusesbyid_1_method#workspacegetvariableusesbyid_method + + pxt.error(`Invalid blockId in AI response`, blockId, parsedResponse); } } - explanationAsTour.push(tourStep); + explanationSteps.push(step); } } else { - explanationAsText = response; // TODO - handle this case better + // TODO thsparks - handle error properly + throw new Error("Invalid response from AI"); } + return { explanationSteps }; + } + + /** + * Converts the AI response data into our structured ErrorHelpResponse format. + * This is used when the AI returns a simple text response. + */ + protected parseTextResponse(response: string): ErrorHelpResponse { return { - explanationAsText, - explanationAsTour: - explanationAsTour.length > 0 ? explanationAsTour : undefined, + explanationSteps: [ + { + message: response + } + ] }; } @@ -89,12 +115,18 @@ export class ErrorHelper { return JSON.stringify(errors); } + /** + * Cleans up blocks code to remove unnecessary and bulky content. + */ protected cleanBlocksCode(code: string): string { // Remove any image content (it's bulky and not useful for the AI) const updatedCode = code.replace(/img`[^`]*`/g, "img`[trimmed for brevity]`"); return updatedCode; } + /** + * Calls the AI-Error-Help API to get assistance with the provided errors. + */ async getHelpAsync(errors: ErrorDisplayInfo[]): Promise { const lang = this.parent.isBlocksActive() ? "blocks" : "typescript"; const target = pxt.appTarget.nickname || pxt.appTarget.name; @@ -106,6 +138,9 @@ export class ErrorHelper { const errString = this.getErrorsAsText(errors); console.log("Errors", errString); + + // TODO thsparks : If error (incl. block ids) is same, check cached response. If ids in that are also valid, return cached response. Else, call API. + // Call to backend API with code and errors, then set the explanation state // to the response from the backend const response = await aiErrorExplainRequest( @@ -116,7 +151,7 @@ export class ErrorHelper { outputFormat ); - const parsedResponse = this.parseResponse(response); + const parsedResponse = outputFormat == "tour_json" ? this.parseTourResponse(response) : this.parseTextResponse(response); return parsedResponse; } } diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index e2fbaec118f4..146d356d84f6 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -75,13 +75,34 @@ export class ErrorList extends React.Component { } } + createTourFromResponse = (response: ErrorHelpResponse) => { + const tourSteps: pxt.tour.BubbleStep[] = []; + for (const item of response.explanationSteps) { + const tourStep = { + title: lf("Error Explanation"), + description: item.message, + location: pxt.tour.BubbleLocation.Center + } as pxt.tour.BubbleStep; + + tourStep.targetQuery = item.elementId; + tourStep.location = pxt.tour.BubbleLocation.Right; + tourStep.onStepBegin = item.onStepBegin; + + tourSteps.push(tourStep); + } + return tourSteps; + } + + handleHelpResponse = (response: ErrorHelpResponse) => { - if (response.explanationAsTour) { - this.props.parent.showTour(response.explanationAsTour); - } else if (response.explanationAsText) { + if (response.explanationSteps.length == 1 && !response.explanationSteps[0].elementId) { + // Response is just a single block of text. Display it in the error window. this.setState({ - responseText: response.explanationAsText + responseText: response.explanationSteps[0].message }); + } else if (response.explanationSteps.length > 0) { + const tourSteps = this.createTourFromResponse(response); + this.props.parent.showTour(tourSteps); } else { // TODO thsparks - Error } From 6bc9ddaef53bc8a0a0ff777f5ca4bf324192323f Mon Sep 17 00:00:00 2001 From: thsparks Date: Mon, 12 May 2025 17:13:13 -0700 Subject: [PATCH 44/91] Remove class from errorHelp, just use functions --- webapp/src/components/errorHelpButton.tsx | 8 +- webapp/src/errorHelp.ts | 250 ++++++++++------------ 2 files changed, 121 insertions(+), 137 deletions(-) diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx index 02e8bb601ee9..8b5c068dd162 100644 --- a/webapp/src/components/errorHelpButton.tsx +++ b/webapp/src/components/errorHelpButton.tsx @@ -2,10 +2,7 @@ import * as React from "react"; import { ErrorDisplayInfo } from "../errorList"; import { Button } from "../../../react-common/components/controls/Button"; import { classList } from "../../../react-common/components/util"; -import { ErrorHelper, ErrorHelpResponse } from "../errorHelp"; - - -// TODO thsparks - there is probably a better way to do this. +import { ErrorHelpResponse, getHelpAsync } from "../errorHelp"; export interface ErrorHelpButtonProps { parent: pxt.editor.IProjectView; @@ -37,9 +34,8 @@ export class ErrorHelpButton extends React.Component< loadingHelp: true, }); - const helper: ErrorHelper = new ErrorHelper(this.props.parent); try { - const response = await helper.getHelpAsync(this.props.errors); + const response = await getHelpAsync(this.props.parent, this.props.errors); this.props.onHelpResponse?.(response); } catch (e) { // TODO thsparks - handle error diff --git a/webapp/src/errorHelp.ts b/webapp/src/errorHelp.ts index 4ae857120be2..c5d8e96b5e69 100644 --- a/webapp/src/errorHelp.ts +++ b/webapp/src/errorHelp.ts @@ -13,145 +13,133 @@ export interface ErrorHelpResponse { explanationSteps: ErrorExplanationStep[]; } -export class ErrorHelper { - - parent: pxt.editor.IProjectView; - constructor(parent: pxt.editor.IProjectView) { - this.parent = parent; - } - - /** - * Converts the AI response data into our structured ErrorHelpResponse format. - * This is used when the AI returns a JSON "tour" response. - */ - protected parseTourResponse(response: string): ErrorHelpResponse { - /* - The AI either provides a JSON list with blockIds tied to explanations for each block. - For example: - [ - { - "blockId": "sampleblockid", - "message": "Here is a helpful message walking you through the error. This one will be displayed first." - }, - { - "blockId": "sampleblockid2", - "message": "Here is another helpful message walking you through the error. This one will be displayed second." - } - ] - */ - - interface AiResponseJsonFormat { - blockId: string; - message: string; +/** + * Converts the AI response data into our structured ErrorHelpResponse format. + * This is used when the AI returns a JSON "tour" response. + */ +function parseTourResponse(parent: pxt.editor.IProjectView, response: string): ErrorHelpResponse { + /* + The AI either provides a JSON list with blockIds tied to explanations for each block. + For example: + [ + { + "blockId": "sampleblockid", + "message": "Here is a helpful message walking you through the error. This one will be displayed first." + }, + { + "blockId": "sampleblockid2", + "message": "Here is another helpful message walking you through the error. This one will be displayed second." } + ] + */ + interface AiResponseJsonFormat { + blockId: string; + message: string; + } - let explanationSteps: ErrorExplanationStep[] = []; - - console.log("AI response", response); - - // If the response contains markdown code "fencing" (i.e. ```json ```), remove it - response = response.trim().replace(/^`{3,}(?:\w+)?\s*|`{3,}$/g, ""); - - const parsedResponse = pxt.Util.jsonTryParse( - response - ) as AiResponseJsonFormat[]; - - if (parsedResponse) { - // TODO thsparks - Can we move "validation" of a response up to the parent editor? - const validBlockIds = this.parent.getBlocks().map((b) => b.id); - for (const item of parsedResponse) { - const step: ErrorExplanationStep = { - message: item.message - }; - - // Check each blockId is specified & valid, and if so, add a tour step for it - const blockId = item.blockId; - if (blockId) { - if (validBlockIds.includes(blockId)) { - step.elementId = `g[data-id="${blockId}"]`; - step.onStepBegin = () => { - (this.parent.editor as Editor).editor.centerOnBlock(blockId); - }; - } else { - - // TODO thsparks - Maybe? - // Try to repair the block id. The AI sometimes sends variable ids instead of blockIds. - // If that variable id is only used in one block, we can assume that is the block we should point to. - // https://developers.google.com/blockly/reference/js/blockly.workspace_class.getvariableusesbyid_1_method#workspacegetvariableusesbyid_method - - pxt.error(`Invalid blockId in AI response`, blockId, parsedResponse); - } + let explanationSteps: ErrorExplanationStep[] = []; + + console.log("AI response", response); + + // If the response contains markdown code "fencing" (i.e. ```json ```), remove it + response = response.trim().replace(/^`{3,}(?:\w+)?\s*|`{3,}$/g, ""); + + const parsedResponse = pxt.Util.jsonTryParse(response) as AiResponseJsonFormat[]; + + if (parsedResponse) { + // TODO thsparks - Can we move "validation" of a response up to the parent editor? + const validBlockIds = parent.getBlocks().map((b) => b.id); + for (const item of parsedResponse) { + const step: ErrorExplanationStep = { + message: item.message, + }; + + // Check each blockId is specified & valid, and if so, add a tour step for it + const blockId = item.blockId; + if (blockId) { + if (validBlockIds.includes(blockId)) { + step.elementId = `g[data-id="${blockId}"]`; + step.onStepBegin = () => { + (parent.editor as Editor).editor.centerOnBlock(blockId); + }; + } else { + // TODO thsparks - Maybe? + // Try to repair the block id. The AI sometimes sends variable ids instead of blockIds. + // If that variable id is only used in one block, we can assume that is the block we should point to. + // https://developers.google.com/blockly/reference/js/blockly.workspace_class.getvariableusesbyid_1_method#workspacegetvariableusesbyid_method + + pxt.error(`Invalid blockId in AI response`, blockId, parsedResponse); } - - explanationSteps.push(step); } - } else { - // TODO thsparks - handle error properly - throw new Error("Invalid response from AI"); - } - return { explanationSteps }; + explanationSteps.push(step); + } + } else { + // TODO thsparks - handle error properly + throw new Error("Invalid response from AI"); } - /** - * Converts the AI response data into our structured ErrorHelpResponse format. - * This is used when the AI returns a simple text response. - */ - protected parseTextResponse(response: string): ErrorHelpResponse { - return { - explanationSteps: [ - { - message: response - } - ] - }; - } + return { explanationSteps }; +} - /** - * Converts the list of errors into a string format for the AI to reference. - * Adds block ids to the error message if they exist. - */ - protected getErrorsAsText(errors: ErrorDisplayInfo[]): string { - return JSON.stringify(errors); - } +/** + * Converts the AI response data into our structured ErrorHelpResponse format. + * This is used when the AI returns a simple text response. + */ +function parseTextResponse(response: string): ErrorHelpResponse { + return { + explanationSteps: [ + { + message: response, + }, + ], + }; +} - /** - * Cleans up blocks code to remove unnecessary and bulky content. - */ - protected cleanBlocksCode(code: string): string { - // Remove any image content (it's bulky and not useful for the AI) - const updatedCode = code.replace(/img`[^`]*`/g, "img`[trimmed for brevity]`"); - return updatedCode; - } +/** + * Converts the list of errors into a string format for the AI to reference. + * Adds block ids to the error message if they exist. + */ +function getErrorsAsText(errors: ErrorDisplayInfo[]): string { + return JSON.stringify(errors); +} - /** - * Calls the AI-Error-Help API to get assistance with the provided errors. - */ - async getHelpAsync(errors: ErrorDisplayInfo[]): Promise { - const lang = this.parent.isBlocksActive() ? "blocks" : "typescript"; - const target = pxt.appTarget.nickname || pxt.appTarget.name; - const mainPkg = pkg.mainEditorPkg(); - const code = (this.parent.isBlocksActive() ? this.cleanBlocksCode((this.parent.editor as Editor).getWorkspaceXmlWithIds()) : mainPkg.files[pxt.MAIN_TS]?.content) ?? ""; - const outputFormat = this.parent.isBlocksActive() ? "tour_json" : "text"; - console.log("Code", code); - - const errString = this.getErrorsAsText(errors); - console.log("Errors", errString); - - - // TODO thsparks : If error (incl. block ids) is same, check cached response. If ids in that are also valid, return cached response. Else, call API. - - // Call to backend API with code and errors, then set the explanation state - // to the response from the backend - const response = await aiErrorExplainRequest( - code, - errString, - lang, - target, - outputFormat - ); - - const parsedResponse = outputFormat == "tour_json" ? this.parseTourResponse(response) : this.parseTextResponse(response); - return parsedResponse; - } +/** + * Cleans up blocks code to remove unnecessary and bulky content. + */ +function cleanBlocksCode(code: string): string { + // Remove any image content (it's bulky and not useful for the AI) + const updatedCode = code.replace(/img`[^`]*`/g, "img`[trimmed for brevity]`"); + return updatedCode; +} + +/** + * Calls the AI-Error-Help API to get assistance with the provided errors. + */ +export async function getHelpAsync( + parent: pxt.editor.IProjectView, + errors: ErrorDisplayInfo[] +): Promise { + const lang = parent.isBlocksActive() ? "blocks" : "typescript"; + const target = pxt.appTarget.nickname || pxt.appTarget.name; + const mainPkg = pkg.mainEditorPkg(); + const code = + (parent.isBlocksActive() + ? cleanBlocksCode((parent.editor as Editor).getWorkspaceXmlWithIds()) + : mainPkg.files[pxt.MAIN_TS]?.content) ?? ""; + const outputFormat = parent.isBlocksActive() ? "tour_json" : "text"; + console.log("Code", code); + + const errString = getErrorsAsText(errors); + console.log("Errors", errString); + + // TODO thsparks : If error (incl. block ids) is same, check cached response. If ids in that are also valid, return cached response. Else, call API. + + // Call to backend API with code and errors, then set the explanation state + // to the response from the backend + const response = await aiErrorExplainRequest(code, errString, lang, target, outputFormat); + + const parsedResponse = + outputFormat == "tour_json" ? parseTourResponse(parent, response) : parseTextResponse(response); + return parsedResponse; } From 0bff91145dda219ebbf770c0e3627a55474aaf4d Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 13 May 2025 15:22:58 -0700 Subject: [PATCH 45/91] Refactor so most of the blocks/text specific checks are happening in their respective editors. --- localtypings/pxteditor.d.ts | 1 + webapp/src/blocks.tsx | 47 ++++++++- webapp/src/cloud.ts | 4 +- webapp/src/components/errorHelpButton.tsx | 23 +---- webapp/src/errorHelp.ts | 117 +++++++++------------- webapp/src/errorList.tsx | 51 ++-------- webapp/src/monaco.tsx | 18 +++- 7 files changed, 127 insertions(+), 134 deletions(-) diff --git a/localtypings/pxteditor.d.ts b/localtypings/pxteditor.d.ts index 781cdb6de42d..d3794a84cfab 100644 --- a/localtypings/pxteditor.d.ts +++ b/localtypings/pxteditor.d.ts @@ -821,6 +821,7 @@ declare namespace pxt.editor { onboarding?: pxt.tour.BubbleStep[]; feedback?: FeedbackState; themePickerOpen?: boolean; + errorListNote?: string; } export interface EditorState { diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index e7559fd8713d..db42c59a51e5 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -27,6 +27,7 @@ import { DebuggerToolbox } from "./debuggerToolbox"; import { ErrorDisplayInfo, ErrorList, StackFrameDisplayInfo } from "./errorList"; import { resolveExtensionUrl } from "./extensionManager"; import { experiments, initEditorExtensionsAsync } from "../../pxteditor"; +import { ErrorHelpTourResponse, getErrorHelpAsTour } from "./errorHelp"; import IProjectView = pxt.editor.IProjectView; @@ -85,6 +86,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { this.onErrorListResize = this.onErrorListResize.bind(this) this.getDisplayInfoForBlockError = this.getDisplayInfoForBlockError.bind(this) this.getDisplayInfoForException = this.getDisplayInfoForException.bind(this) + this.createTourFromResponse = this.createTourFromResponse.bind(this) + this.getErrorHelp = this.getErrorHelp.bind(this) } setBreakpointsMap(breakpoints: pxtc.Breakpoint[], procCallLocations: pxtc.LocationInfo[]): void { if (!breakpoints || !this.compilationResult) return; @@ -927,7 +930,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { {showErrorList && } + onSizeChange={this.onErrorListResize} + getErrorHelp={this.getErrorHelp} />}
) } @@ -1002,6 +1006,47 @@ export class Editor extends toolboxeditor.ToolboxEditor { }; } + /** + * Provides a user-facing explanation of the errors in the error list. + */ + async getErrorHelp() { + const code = this.getWorkspaceXmlWithIds(); + const helpResponse = await getErrorHelpAsTour(this.errors, "blocks", code); + const tour = this.createTourFromResponse(helpResponse); + this.parent.showTour(tour); + } + + /** + * Converts an ErrorHelpTourRespone into an actual tour, which mostly involves + * ensuring all provided ids are valid and setting up the corresponding target queries. + */ + private createTourFromResponse = (response: ErrorHelpTourResponse) => { + const validBlockIds = this.parent.getBlocks().map((b) => b.id); + + const tourSteps: pxt.tour.BubbleStep[] = []; + for (const step of response.explanationSteps) { + const tourStep = { + title: lf("Error Explanation"), + description: step.message, + location: pxt.tour.BubbleLocation.Center + } as pxt.tour.BubbleStep; + + if (validBlockIds.includes(step.elementId)) { + tourStep.targetQuery = `g[data-id="${step.elementId}"]`; + tourStep.location = pxt.tour.BubbleLocation.Right; + tourStep.onStepBegin = () => this.editor.centerOnBlock(step.elementId, true); + } else { + // TODO thsparks - Maybe? + // Try to repair the block id. The AI sometimes sends variable ids instead of blockIds. + // If that variable id is only used in one block, we can assume that is the block we should point to. + // https://developers.google.com/blockly/reference/js/blockly.workspace_class.getvariableusesbyid_1_method#workspacegetvariableusesbyid_method + } + + tourSteps.push(tourStep); + } + return tourSteps; + } + getBlocksAreaDiv() { return document.getElementById('blocksArea'); } diff --git a/webapp/src/cloud.ts b/webapp/src/cloud.ts index c7abd1f34b13..e0e15d3223cc 100644 --- a/webapp/src/cloud.ts +++ b/webapp/src/cloud.ts @@ -686,9 +686,9 @@ export function init() { export async function aiErrorExplainRequest( code: string, errors: string, - lang: string, + lang: "blocks" | "typescript" | "python", target: string, - outputFormat: string + outputFormat: "tour_json" | "text" ): Promise { const url = `/api/copilot/explainerror`; const data = { lang, code, errors, target, outputFormat }; diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx index 8b5c068dd162..7a648a65ca05 100644 --- a/webapp/src/components/errorHelpButton.tsx +++ b/webapp/src/components/errorHelpButton.tsx @@ -2,16 +2,16 @@ import * as React from "react"; import { ErrorDisplayInfo } from "../errorList"; import { Button } from "../../../react-common/components/controls/Button"; import { classList } from "../../../react-common/components/util"; -import { ErrorHelpResponse, getHelpAsync } from "../errorHelp"; +import { ErrorHelpTourResponse } from "../errorHelp"; export interface ErrorHelpButtonProps { parent: pxt.editor.IProjectView; errors: ErrorDisplayInfo[]; - onHelpResponse?: (response: ErrorHelpResponse) => void; + getErrorHelp: () => void; } interface ErrorHelpButtonState { - explanation?: ErrorHelpResponse; + explanation?: ErrorHelpTourResponse; loadingHelp?: boolean; } @@ -25,21 +25,6 @@ export class ErrorHelpButton extends React.Component< explanation: undefined, loadingHelp: false, }; - - this.handleHelpClick = this.handleHelpClick.bind(this); - } - - async handleHelpClick() { - this.setState({ - loadingHelp: true, - }); - - try { - const response = await getHelpAsync(this.props.parent, this.props.errors); - this.props.onHelpResponse?.(response); - } catch (e) { - // TODO thsparks - handle error - } } render() { @@ -52,7 +37,7 @@ export class ErrorHelpButton extends React.Component< return (
) @@ -700,6 +704,17 @@ export class Editor extends toolboxeditor.ToolboxEditor { }; } + /** + * Provides a user-facing explanation of the errors in the error list. + */ + private async getErrorHelp() { + this.parent.setState({errorListNote: undefined}); + const lang = this.fileType == pxt.editor.FileType.Python ? "python" : "typescript"; + const code = this.currFile.content; + const helpResponse = await getErrorHelpAsText(this.errors, lang, code); + this.parent.setState({errorListNote: helpResponse}); + } + goToError(error: pxtc.LocationInfo) { // Use endLine and endColumn to position the cursor // when errors do have them @@ -1500,6 +1515,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { if (this.toolbox) this.toolbox.clearSearch(); this.errors = []; + this.parent.setState({errorListNote: undefined}); if (this.currFile && this.currFile.getName() == "this/" + pxt.CONFIG_NAME) { // Reload the header if a change was made to the config file: pxt.json return this.parent.reloadHeaderAsync(); From 945769ae4c3d4f587362ff8d65843b236485aab6 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 13 May 2025 17:00:01 -0700 Subject: [PATCH 46/91] Allow different tour styles & configurations. Kept style at the bubble level in case we ever have cause to use different colors within the tour itself... --- localtypings/pxtarget.d.ts | 5 +++ localtypings/pxteditor.d.ts | 6 +-- .../components/controls/TeachingBubble.tsx | 6 ++- .../styles/onboarding/TeachingBubble.less | 37 ++++++++++++------- webapp/src/app.tsx | 22 ++++++----- webapp/src/blocks.tsx | 12 ++++-- webapp/src/components/onboarding/Tour.tsx | 14 ++++--- 7 files changed, 65 insertions(+), 37 deletions(-) diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index fc814adcbf63..d740c92db18a 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1347,6 +1347,11 @@ declare namespace pxt.tour { sansQuery?: string; // Use this to exclude an element from the cutout sansLocation?: BubbleLocation; // relative location of element to exclude onStepBegin?: () => void; + bubbleStyle?: "yellow"; // Currently just have default (unset) & yellow styles. May add more in the future... + } + interface TourConfig { + showConfetti?: boolean; + steps: BubbleStep[]; } const enum BubbleLocation { Above, diff --git a/localtypings/pxteditor.d.ts b/localtypings/pxteditor.d.ts index d3794a84cfab..413b3e029494 100644 --- a/localtypings/pxteditor.d.ts +++ b/localtypings/pxteditor.d.ts @@ -818,7 +818,7 @@ declare namespace pxt.editor { screenshoting?: boolean; extensionsVisible?: boolean; isMultiplayerGame?: boolean; // Arcade: Does the current project contain multiplayer blocks? - onboarding?: pxt.tour.BubbleStep[]; + activeTourConfig?: pxt.tour.TourConfig; feedback?: FeedbackState; themePickerOpen?: boolean; errorListNote?: string; @@ -1056,8 +1056,8 @@ declare namespace pxt.editor { showLightbox(): void; hideLightbox(): void; showOnboarding(): void; - showTour(steps: pxt.tour.BubbleStep[]): void; - hideOnboarding(): void; + showTour(config: pxt.tour.TourConfig): void; + closeTour(): void; showKeymap(show: boolean): void; toggleKeymap(): void; signOutGithub(): void; diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index 1d353e7a5b99..67a98d99c620 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -23,6 +23,7 @@ export interface TeachingBubbleProps extends ContainerProps { onClose: () => void; parentElement?: Element; activeTarget?: boolean; // if true, the target is clickable + showConfetti?: boolean; onNext: () => void; onBack: () => void; onFinish: () => void; @@ -363,11 +364,12 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { const classes = classList( "teaching-bubble-container", - className + className, + targetContent.bubbleStyle ); return ReactDOM.createPortal( - {/* {stepNumber === totalSteps + 1 && } */} + {props.showConfetti && stepNumber === totalSteps + 1 && }
diff --git a/react-common/styles/onboarding/TeachingBubble.less b/react-common/styles/onboarding/TeachingBubble.less index e6a9f91b3b42..183e51051f32 100644 --- a/react-common/styles/onboarding/TeachingBubble.less +++ b/react-common/styles/onboarding/TeachingBubble.less @@ -1,3 +1,13 @@ +.teaching-bubble-container { + --teaching-bubble-foreground: var(--pxt-tertiary-foreground); + --teaching-bubble-background: var(--pxt-tertiary-background); +} + +.teaching-bubble-container.yellow { + --teaching-bubble-foreground: var(--pxt-colors-yellow-foreground); + --teaching-bubble-background: var(--pxt-colors-yellow-background); +} + .teaching-bubble-container { position: fixed; top: 0; @@ -30,17 +40,16 @@ position: fixed; max-width: 25rem; min-width: 18.75rem; - background: var(--pxt-tertiary-background); - color: var(--pxt-tertiary-foreground); + background: var(--teaching-bubble-background); + color: var(--teaching-bubble-foreground); box-shadow: 0 0 0 0.1rem; border-radius: 0.5rem; padding: 1rem; z-index: @modalDimmerZIndex; .common-button { - &:focus { - outline: .1rem solid var(--pxt-tertiary-foreground); + outline: 0.1rem solid var(--teaching-bubble-foreground); filter: grayscale(.15) brightness(.85) contrast(1.3); } &.tertiary:focus::after, &:focus::after { @@ -54,7 +63,7 @@ width: 0; height: 0; z-index: @modalDimmerZIndex + 1; - color: var(--pxt-tertiary-background); + color: var(--teaching-bubble-background); } .teaching-bubble-arrow-outline { @@ -62,7 +71,7 @@ width: 0; height: 0; z-index: @modalDimmerZIndex; - color: var(--pxt-tertiary-foreground); + color: var(--teaching-bubble-foreground); } .teaching-bubble-content { @@ -81,25 +90,25 @@ .teaching-bubble-steps { font-size: .9rem; - color: var(--pxt-tertiary-foreground); + color: var(--teaching-bubble-foreground); } .common-button.tertiary { padding: .25rem .5rem; margin: 0 .5rem 0 0; - border: .1rem solid var(--pxt-tertiary-foreground); - background: var(--pxt-tertiary-background); - color: var(--pxt-tertiary-foreground); + border: 0.1rem solid var(--teaching-bubble-foreground) !important; + background: var(--teaching-bubble-background) !important; + color: var(--teaching-bubble-foreground) !important; &.inverted { margin-right: 0; - background: var(--pxt-tertiary-foreground); - color: var(--pxt-tertiary-background); + background: var(--teaching-bubble-foreground) !important; + color: var(--teaching-bubble-background) !important; } } .common-button:focus-visible { - outline: 2px solid var(--pxt-tertiary-foreground); + outline: 2px solid var(--teaching-bubble-foreground); outline-offset: 3px; } @@ -114,7 +123,7 @@ top: 0.5rem; padding: 0.5rem 0 0.25rem 0; background: transparent; - color: var(--pxt-tertiary-foreground); + color: var(--teaching-bubble-foreground); margin: 0; i.right { diff --git a/webapp/src/app.tsx b/webapp/src/app.tsx index fce76c405f87..b8e957001d6b 100644 --- a/webapp/src/app.tsx +++ b/webapp/src/app.tsx @@ -208,7 +208,7 @@ export class ProjectView simState: SimState.Stopped, autoRun: this.autoRunOnStart(), isMultiplayerGame: false, - onboarding: undefined, + activeTourConfig: undefined, mute: pxt.editor.MuteState.Unmuted, feedback: {showing: false, kind: "generic"} // state that tracks if the feedback modal is showing and what kind }; @@ -219,7 +219,7 @@ export class ProjectView this.hidePackageDialog = this.hidePackageDialog.bind(this); this.hwDebug = this.hwDebug.bind(this); this.hideLightbox = this.hideLightbox.bind(this); - this.hideOnboarding = this.hideOnboarding.bind(this); + this.closeTour = this.closeTour.bind(this); this.hideFeedback = this.hideFeedback.bind(this); this.openSimSerial = this.openSimSerial.bind(this); this.openDeviceSerial = this.openDeviceSerial.bind(this); @@ -5234,20 +5234,24 @@ export class ProjectView } /////////////////////////////////////////////////////////// - //////////// Onboarding ///////////// + //////////// Tours ///////////// /////////////////////////////////////////////////////////// - hideOnboarding() { - this.setState({ onboarding: undefined }); + closeTour() { + this.setState({ activeTourConfig: undefined }); } async showOnboarding() { const tourSteps: pxt.tour.BubbleStep[] = await parseTourStepsAsync(pxt.appTarget.appTheme?.tours?.editor) - this.showTour(tourSteps); + const config: pxt.tour.TourConfig = { + steps: tourSteps, + showConfetti: true, + } + this.showTour(config); } - async showTour(steps: pxt.tour.BubbleStep[]) { - this.setState({ onboarding: steps }); + async showTour(config: pxt.tour.TourConfig) { + this.setState({ activeTourConfig: config }); } /////////////////////////////////////////////////////////// @@ -5509,7 +5513,7 @@ export class ProjectView {hideMenuBar ? : undefined} {lightbox ? : undefined} - {this.state.onboarding && } + {this.state.activeTourConfig && } {this.state.themePickerOpen && this.setColorThemeById(theme?.id, true)} onClose={this.hideThemePicker} />}
); diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index db42c59a51e5..8e7395bbb406 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -1012,8 +1012,13 @@ export class Editor extends toolboxeditor.ToolboxEditor { async getErrorHelp() { const code = this.getWorkspaceXmlWithIds(); const helpResponse = await getErrorHelpAsTour(this.errors, "blocks", code); - const tour = this.createTourFromResponse(helpResponse); - this.parent.showTour(tour); + const tourSteps = this.createTourFromResponse(helpResponse); + this.parent.showTour( + { + steps: tourSteps, + showConfetti: false + } + ); } /** @@ -1028,7 +1033,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { const tourStep = { title: lf("Error Explanation"), description: step.message, - location: pxt.tour.BubbleLocation.Center + location: pxt.tour.BubbleLocation.Center, + bubbleStyle: "yellow", } as pxt.tour.BubbleStep; if (validBlockIds.includes(step.elementId)) { diff --git a/webapp/src/components/onboarding/Tour.tsx b/webapp/src/components/onboarding/Tour.tsx index a0159259b42f..0cf1866a7752 100644 --- a/webapp/src/components/onboarding/Tour.tsx +++ b/webapp/src/components/onboarding/Tour.tsx @@ -4,11 +4,12 @@ import { TeachingBubble } from "../../../../react-common/components/controls/Tea export interface TourProps { onClose: () => void; - tourSteps: pxt.tour.BubbleStep[]; + config: pxt.tour.TourConfig; } export const Tour = (props: TourProps) => { - const { onClose, tourSteps } = props; + const { onClose, config } = props; + const { steps, showConfetti } = config; const [currentStep, setCurrentStep] = useState(0); const tourStartTime = useRef(Date.now()); const stepStartTime = useRef(Date.now()); @@ -26,11 +27,11 @@ export const Tour = (props: TourProps) => { } const data = () => ({ - title: tourSteps[currentStep].title, + title: steps[currentStep].title, stepDuration: getStepDuration(), tourDuration: getTourDuration(), step: currentStep + 1, - totalSteps: tourSteps.length + totalSteps: steps.length }); const onNext = () => { @@ -56,12 +57,13 @@ export const Tour = (props: TourProps) => { } return }; \ No newline at end of file From 4804168ce3d6bf43250b000fd4ce48358498f8e7 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 13 May 2025 17:54:46 -0700 Subject: [PATCH 47/91] Require sign-in. May decide to move this check into the ErrorList, not sure yet... --- localtypings/pxteditor.d.ts | 2 +- webapp/src/app.tsx | 4 +-- webapp/src/components/errorHelpButton.tsx | 31 ++++++++++++++++++----- webapp/src/errorList.tsx | 2 +- webapp/src/identity.tsx | 15 +++++++---- 5 files changed, 39 insertions(+), 15 deletions(-) diff --git a/localtypings/pxteditor.d.ts b/localtypings/pxteditor.d.ts index 413b3e029494..4a9098d56fcb 100644 --- a/localtypings/pxteditor.d.ts +++ b/localtypings/pxteditor.d.ts @@ -1070,7 +1070,7 @@ declare namespace pxt.editor { showFeedbackDialog(kind: ocv.FeedbackKind): void; showTurnBackTimeDialogAsync(): Promise; - showLoginDialog(continuationHash?: string): void; + showLoginDialog(continuationHash?: string, dialogMessages?: { signInMessage?: string; signUpMessage?: string }): void; showProfileDialog(location?: string): void; showImportUrlDialog(): void; diff --git a/webapp/src/app.tsx b/webapp/src/app.tsx index b8e957001d6b..a9b429b11ddf 100644 --- a/webapp/src/app.tsx +++ b/webapp/src/app.tsx @@ -4565,8 +4565,8 @@ export class ProjectView } } - showLoginDialog(continuationHash?: string) { - this.loginDialog.show(continuationHash); + showLoginDialog(continuationHash?: string, dialogMessages?: { signInMessage?: string; signUpMessage?: string }) { + this.loginDialog.show(continuationHash, dialogMessages); } showProfileDialog(location?: string) { diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx index 7a648a65ca05..71575ccacf78 100644 --- a/webapp/src/components/errorHelpButton.tsx +++ b/webapp/src/components/errorHelpButton.tsx @@ -1,12 +1,10 @@ -import * as React from "react"; -import { ErrorDisplayInfo } from "../errorList"; +import * as auth from "../auth"; import { Button } from "../../../react-common/components/controls/Button"; import { classList } from "../../../react-common/components/util"; import { ErrorHelpTourResponse } from "../errorHelp"; export interface ErrorHelpButtonProps { parent: pxt.editor.IProjectView; - errors: ErrorDisplayInfo[]; getErrorHelp: () => void; } @@ -15,7 +13,7 @@ interface ErrorHelpButtonState { loadingHelp?: boolean; } -export class ErrorHelpButton extends React.Component< +export class ErrorHelpButton extends auth.Component< ErrorHelpButtonProps, ErrorHelpButtonState > { @@ -25,9 +23,30 @@ export class ErrorHelpButton extends React.Component< explanation: undefined, loadingHelp: false, }; + + this.onHelpClick = this.onHelpClick.bind(this); + } + + onHelpClick = () => { + this.setState({ loadingHelp: true }); + + // Sign-in required. Prompt the user, if they are logged out. + if (!this.isLoggedIn()) { + this.props.parent.showLoginDialog(undefined, { + signInMessage: lf("Sign-in is required to use this feature"), + signUpMessage: lf("Sign-up is required to use this feature"), + }); + return; + } + + this.props.getErrorHelp(); } - render() { + renderCore() { + if (!pxt.auth.hasIdentity()) { + return null; + } + const { loadingHelp } = this.state; // if (loadingHelp) { @@ -37,7 +56,7 @@ export class ErrorHelpButton extends React.Component< return (
) } diff --git a/webapp/src/components/errorHelpButton.tsx b/webapp/src/components/errorHelpButton.tsx deleted file mode 100644 index 71575ccacf78..000000000000 --- a/webapp/src/components/errorHelpButton.tsx +++ /dev/null @@ -1,67 +0,0 @@ -import * as auth from "../auth"; -import { Button } from "../../../react-common/components/controls/Button"; -import { classList } from "../../../react-common/components/util"; -import { ErrorHelpTourResponse } from "../errorHelp"; - -export interface ErrorHelpButtonProps { - parent: pxt.editor.IProjectView; - getErrorHelp: () => void; -} - -interface ErrorHelpButtonState { - explanation?: ErrorHelpTourResponse; - loadingHelp?: boolean; -} - -export class ErrorHelpButton extends auth.Component< - ErrorHelpButtonProps, - ErrorHelpButtonState -> { - constructor(props: ErrorHelpButtonProps) { - super(props); - this.state = { - explanation: undefined, - loadingHelp: false, - }; - - this.onHelpClick = this.onHelpClick.bind(this); - } - - onHelpClick = () => { - this.setState({ loadingHelp: true }); - - // Sign-in required. Prompt the user, if they are logged out. - if (!this.isLoggedIn()) { - this.props.parent.showLoginDialog(undefined, { - signInMessage: lf("Sign-in is required to use this feature"), - signUpMessage: lf("Sign-up is required to use this feature"), - }); - return; - } - - this.props.getErrorHelp(); - } - - renderCore() { - if (!pxt.auth.hasIdentity()) { - return null; - } - - const { loadingHelp } = this.state; - - // if (loadingHelp) { - // return
; - // } - - return ( -
{showErrorList && } + note={this.parent.state.errorListNote} + showLoginDialog={this.parent.showLoginDialog} />}
) From 2ee99744e91c208d54bebbe5d3b0ead6b4a7e961 Mon Sep 17 00:00:00 2001 From: thsparks Date: Wed, 14 May 2025 16:05:55 -0700 Subject: [PATCH 49/91] Loading display (simple for now) --- theme/errorList.less | 38 ++++++++++++++++++++++++++++----- webapp/src/errorList.tsx | 46 +++++++++++++++++++++++++++------------- 2 files changed, 64 insertions(+), 20 deletions(-) diff --git a/theme/errorList.less b/theme/errorList.less index feaf04f56106..1f0a4249efbd 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -15,6 +15,10 @@ padding: 0.75em 1em; height: @errorListHeaderHeight; margin-bottom: 0; + display: flex; + flex-direction: row; + justify-content: flex-start; + align-items: center; &:hover { cursor: pointer; @@ -47,6 +51,35 @@ font-size: 1.3em; } } + + .error-help-container { + flex-grow: 1; + margin: 0 1rem; + + .error-help-button { + padding: 0.7rem; + } + + .error-help-loader { + display: flex; + flex-direction: row; + justify-content: flex-start; + align-items: center; + width: fit-content; + color: var(--pxt-secondary-background); // Inverted to match the button bkg color + cursor: progress; + + // Match button + padding: 0.7rem; + font-weight: 400; + line-height: 1em; + font-size: 16px; + + .analyzing-label { + margin: 0 1rem; + } + } + } } .errorListInner { @@ -109,11 +142,6 @@ } } - .error-help-button { - margin: 0 1rem; - padding: 0.7rem; - } - .explanationText { margin: 1rem; padding: 0.5rem; diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 7c16cc87f2d5..bb6334dbbe9b 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -9,6 +9,7 @@ import { Button } from "../../react-common/components/controls/Button"; /** * A collection of optional metadata that can be attached to an error. + * TODO thsparks - Can this be removed? */ type ErrorMetadata = { blockId?: string, @@ -38,12 +39,13 @@ export interface ErrorListProps { errors: ErrorDisplayInfo[]; note?: string; startDebugger?: () => void; - getErrorHelp?: () => void; + getErrorHelp?: () => Promise; // Should return a promise that resolves when the help is loaded showLoginDialog?: (continuationHash?: string, dialogMessages?: { signInMessage?: string; signUpMessage?: string }) => void; } export interface ErrorListState { isCollapsed: boolean + isLoadingHelp?: boolean } export class ErrorList extends auth.Component { @@ -52,7 +54,8 @@ export class ErrorList extends auth.Component { super(props); this.state = { - isCollapsed: true + isCollapsed: true, + isLoadingHelp: false } this.onCollapseClick = this.onCollapseClick.bind(this); @@ -74,9 +77,7 @@ export class ErrorList extends auth.Component { } } - onHelpClick = () => { - // this.setState({ loadingHelp: true }); - + onHelpClick = async () => { // Sign-in required. Prompt the user, if they are logged out. if (!this.isLoggedIn()) { this.props.showLoginDialog(undefined, { @@ -86,12 +87,16 @@ export class ErrorList extends auth.Component { return; } - this.props.getErrorHelp(); + this.setState({ isLoadingHelp: true }); + + await this.props.getErrorHelp(); + + this.setState({ isLoadingHelp: false }); } renderCore() { const { startDebugger, errors, getErrorHelp, showLoginDialog, note } = this.props; - const { isCollapsed } = this.state; + const { isCollapsed, isLoadingHelp } = this.state; const errorsAvailable = !!errors?.length; const groupedErrors = groupErrors(errors); @@ -101,19 +106,30 @@ export class ErrorList extends auth.Component { const showErrorHelp = !!getErrorHelp && !!showLoginDialog; // && !pxt.appTarget.appTheme.disableErrorHelp; + const helpLoader =
e.stopPropagation()}> +
+ {lf("Analyzing...")} +
; + + const helpButton = ( +
} - {note &&
{note}
} + {note &&
{note}
}
{errorListContent} From b55e5f2306265f6be305461b67b57f6f1df132ac Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 15 May 2025 10:57:58 -0700 Subject: [PATCH 61/91] Clear errorListNote when errors change --- webapp/src/monaco.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index 7a4abd58e641..c17bc7b6b740 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -671,11 +671,13 @@ export class Editor extends toolboxeditor.ToolboxEditor { public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { const exceptionDisplayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); this.errors = [exceptionDisplayInfo]; + this.parent.setState({ errorListNote: undefined }); } private onErrorChanges(errors: pxtc.KsDiagnostic[]) { const errorDisplayInfo: ErrorDisplayInfo[] = errors.map(this.getDisplayInfoForError); this.errors = errorDisplayInfo; + this.parent.setState({ errorListNote: undefined }); } private getDisplayInfoForException(exception: pxsim.DebuggerBreakpointMessage): ErrorDisplayInfo { From 00e6dc78b48b2c55d3a1652b404dc509d5877256 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 15 May 2025 14:16:45 -0700 Subject: [PATCH 62/91] AI Disclaimer for blocks tour --- localtypings/pxtarget.d.ts | 1 + .../components/controls/TeachingBubble.tsx | 4 ++++ .../styles/onboarding/TeachingBubble.less | 17 +++++++++++++++++ webapp/src/blocks.tsx | 1 + 4 files changed, 23 insertions(+) diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index fec248589049..fc29b93a42d4 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1349,6 +1349,7 @@ declare namespace pxt.tour { sansLocation?: BubbleLocation; // relative location of element to exclude onStepBegin?: () => void; bubbleStyle?: "yellow"; // Currently just have default (unset) & yellow styles. May add more in the future... + notice?: string; // Optional notice to show in the bubble } interface TourConfig { steps: BubbleStep[]; diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index bbf7f470d28b..0d3ab82231f2 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -382,6 +382,10 @@ export const TeachingBubble = (props: TeachingBubbleProps) => {
{targetContent.title}

{targetContent.description}

+ {targetContent.notice &&
+ + {targetContent.notice} +
}
{hasSteps &&
{stepNumber} of {totalSteps} diff --git a/react-common/styles/onboarding/TeachingBubble.less b/react-common/styles/onboarding/TeachingBubble.less index a0e1ff174a3d..f8fe99afc9fe 100644 --- a/react-common/styles/onboarding/TeachingBubble.less +++ b/react-common/styles/onboarding/TeachingBubble.less @@ -82,6 +82,23 @@ } } +.teaching-bubble-notice { + display: flex; + flex-direction: row; + align-items: center; + padding: 0.5rem; + margin: 0.5rem 0; + + background-color: var(--pxt-colors-red-alpha10); + color: var(--pxt-neutral-alpha80); + border-radius: 0.5rem; + font-size: 1rem; + + .notice-text { + margin-left: 0.5rem; + } +} + .teaching-bubble-footer { display: flex; flex-direction: row; diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 9a009ce8de87..4b31468405b2 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -1035,6 +1035,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { description: step.message, location: pxt.tour.BubbleLocation.Center, bubbleStyle: "yellow", + notice: lf("AI-generated content may not be perfect. Always double check.") } as pxt.tour.BubbleStep; if (validBlockIds.includes(step.elementId)) { From 95bfaf222a571e493085edb7084c13401c089829 Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 15 May 2025 14:17:04 -0700 Subject: [PATCH 63/91] Add AI disclaimer to text response. --- webapp/src/monaco.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index c17bc7b6b740..b6e165b3e79b 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -714,7 +714,8 @@ export class Editor extends toolboxeditor.ToolboxEditor { const lang = this.fileType == pxt.editor.FileType.Python ? "python" : "typescript"; const code = this.currFile.content; try { - const helpResponse = await getErrorHelpAsText(this.errors, lang, code); + let helpResponse = await getErrorHelpAsText(this.errors, lang, code); + helpResponse += `\n\n${lf("This explanation was generated by AI and may not be perfect. Always double check.")}`; this.parent.setState({errorListNote: helpResponse}); } catch (e) { pxt.reportException(e); From 9a0a3e58a0414f236b5319f05fa7d79d3076e3cf Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 15 May 2025 14:17:31 -0700 Subject: [PATCH 64/91] Preserve newlines in text explanation --- theme/errorList.less | 1 + 1 file changed, 1 insertion(+) diff --git a/theme/errorList.less b/theme/errorList.less index da5ef429aaf0..3a02ba050bb0 100644 --- a/theme/errorList.less +++ b/theme/errorList.less @@ -151,6 +151,7 @@ padding: 0.5rem; border-radius: 0.2rem; background-color: var(--pxt-neutral-alpha10); + white-space: pre-line; // Preserve new lines } } From 1a71c18a7bae03784f2cd5ab8e46ac24c5a06b2d Mon Sep 17 00:00:00 2001 From: thsparks Date: Thu, 15 May 2025 17:09:13 -0700 Subject: [PATCH 65/91] Feedback for editor tour (still needs proper testing) --- localtypings/pxtarget.d.ts | 1 + .../components/controls/TeachingBubble.tsx | 20 +++++++++++ .../styles/onboarding/TeachingBubble.less | 36 +++++++++++++++++++ webapp/src/blocks.tsx | 16 ++++++++- webapp/src/components/onboarding/Tour.tsx | 25 +++++++++++-- 5 files changed, 95 insertions(+), 3 deletions(-) diff --git a/localtypings/pxtarget.d.ts b/localtypings/pxtarget.d.ts index fc29b93a42d4..39391422b68d 100644 --- a/localtypings/pxtarget.d.ts +++ b/localtypings/pxtarget.d.ts @@ -1355,6 +1355,7 @@ declare namespace pxt.tour { steps: BubbleStep[]; showConfetti?: boolean; numberFinalStep?: boolean; // The last step will only be included in the step count if this is true. + onFeedback?: (positive: boolean) => void; // Optional callback for thumbs up/down feedback. Feedback is only shown if this is set. } const enum BubbleLocation { Above, diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index 0d3ab82231f2..285c54764af8 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -30,6 +30,8 @@ export interface TeachingBubbleProps extends ContainerProps { onNext: () => void; onBack: () => void; onFinish: () => void; + onFeedback?: (positive: boolean) => void; + selectedFeedback?: boolean; } export const TeachingBubble = (props: TeachingBubbleProps) => { @@ -45,6 +47,8 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { onNext, onBack, onFinish, + onFeedback, + selectedFeedback, stepNumber, totalSteps, parentElement, @@ -390,6 +394,22 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { {hasSteps &&
{stepNumber} of {totalSteps}
} + { onFeedback &&
+
}
{hasPrevious &&
+ ); +}; diff --git a/react-common/styles/controls/ThumbsFeedback.less b/react-common/styles/controls/ThumbsFeedback.less new file mode 100644 index 000000000000..f348d553c364 --- /dev/null +++ b/react-common/styles/controls/ThumbsFeedback.less @@ -0,0 +1,20 @@ +.feedback-buttons { + display: flex; + flex-direction: row; + align-items: center; + justify-content: center; + + .common-button.feedback-button { + background: none; + padding: 0.1rem 0; + margin: 0 0.2rem 0 0; + + i { + margin: 0; + } + + &:hover:not(.disabled) { + filter: opacity(0.7); + } + } +} \ No newline at end of file diff --git a/react-common/styles/react-common.less b/react-common/styles/react-common.less index 2b0fa532f9d8..023af572bed8 100644 --- a/react-common/styles/react-common.less +++ b/react-common/styles/react-common.less @@ -28,6 +28,7 @@ @import "controls/Accordion.less"; @import "controls/CarouselNav.less"; @import "controls/Feedback.less"; +@import "controls/ThumbsFeedback.less"; @import "theming/base-theme.less"; @import "./react-common-variables.less"; @import "./semantic-ui-overrides.less"; diff --git a/theme/ai-error-explanation-text.less b/theme/ai-error-explanation-text.less new file mode 100644 index 000000000000..a281e8d42634 --- /dev/null +++ b/theme/ai-error-explanation-text.less @@ -0,0 +1,32 @@ +.error-list-ai-explanation-container { + display: flex; + flex-direction: column; + justify-content: flex-start; + align-items: flex-start; + width: 100%; + height: 100%; + white-space: pre-line; // Preserve new lines + + .ai-footer { + display: flex; + flex-direction: row; + justify-content: space-between; + align-items: flex-end; + width: 100%; + height: fit-content; + margin-top: 0.5rem; + font-size: 0.9rem; + line-height: 0.9rem; + + .feedback-button { + i { + font-size: 0.9rem; + } + + &.disabled { + // Override the default disabled coloring + color: var(--pxt-neutral1-foreground); + } + } + } +} \ No newline at end of file diff --git a/theme/pxt.less b/theme/pxt.less index 415de4c0233a..5a6664bf5fbf 100644 --- a/theme/pxt.less +++ b/theme/pxt.less @@ -27,6 +27,7 @@ @import 'errorList'; @import 'asset-editor'; @import 'semantic-ui-overrides'; +@import 'ai-error-explanation-text'; @import 'light'; @import 'accessibility'; diff --git a/webapp/src/components/AIErrorExplanationText.tsx b/webapp/src/components/AIErrorExplanationText.tsx new file mode 100644 index 000000000000..9268228f8932 --- /dev/null +++ b/webapp/src/components/AIErrorExplanationText.tsx @@ -0,0 +1,23 @@ +import { ThumbsFeedback } from "../../../react-common/components/controls/Feedback/ThumbsFeedback"; + +interface AIErrorExplanationTextProps { + explanation: string; + onFeedbackSelected: (positive: boolean) => void; +} +/* Simple component to encapsulate how we display paragraph-form AI generated error explanations. */ +export const AIErrorExplanationText = (props: AIErrorExplanationTextProps) => { + const { + explanation, + onFeedbackSelected, + } = props; + + return ( +
+
{explanation}
+
+
{lf("This explanation was generated by AI, and it may not be perfect. Always double check.")}
+ +
+
+ ); +}; diff --git a/webapp/src/errorList.tsx b/webapp/src/errorList.tsx index 1d67f74f0689..f85fab130894 100644 --- a/webapp/src/errorList.tsx +++ b/webapp/src/errorList.tsx @@ -36,7 +36,7 @@ export type ErrorDisplayInfo = { export interface ErrorListProps { onSizeChange?: (state: pxt.editor.ErrorListState) => void; errors: ErrorDisplayInfo[]; - note?: string; + note?: string | JSX.Element; startDebugger?: () => void; getErrorHelp?: () => Promise; // Should return a promise that resolves when the help is loaded showLoginDialog?: ( diff --git a/webapp/src/monaco.tsx b/webapp/src/monaco.tsx index b6e165b3e79b..d6756a047233 100644 --- a/webapp/src/monaco.tsx +++ b/webapp/src/monaco.tsx @@ -32,6 +32,7 @@ import ErrorListState = pxt.editor.ErrorListState; import * as pxtblockly from "../../pxtblocks"; import { ThemeManager } from "../../react-common/components/theming/themeManager"; import { ErrorHelpException, getErrorHelpAsText } from "./errorHelp"; +import { AIErrorExplanationText } from "./components/AIErrorExplanationText"; const MIN_EDITOR_FONT_SIZE = 10 const MAX_EDITOR_FONT_SIZE = 40 @@ -661,13 +662,22 @@ export class Editor extends toolboxeditor.ToolboxEditor { errors={this.errors} startDebugger={this.startDebugger} getErrorHelp={this.getErrorHelp} - note={this.parent.state.errorListNote} + note={this.parent.state.errorListNote && } showLoginDialog={this.parent.showLoginDialog} />}
) } + private onAIFeedback = (positive: boolean) => { + pxt.tickEvent("errorHelp.feedback", { + positive: positive + "", + type: "text", + responseLength: this.parent.state.errorListNote?.length + "", + errorCount: this.errors.length + }); + } + public onExceptionDetected(exception: pxsim.DebuggerBreakpointMessage) { const exceptionDisplayInfo: ErrorDisplayInfo = this.getDisplayInfoForException(exception); this.errors = [exceptionDisplayInfo]; @@ -715,7 +725,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { const code = this.currFile.content; try { let helpResponse = await getErrorHelpAsText(this.errors, lang, code); - helpResponse += `\n\n${lf("This explanation was generated by AI and may not be perfect. Always double check.")}`; this.parent.setState({errorListNote: helpResponse}); } catch (e) { pxt.reportException(e); From cdffd99ea3b064a6dcb68c9f51b41254fe4d7d5b Mon Sep 17 00:00:00 2001 From: thsparks Date: Fri, 16 May 2025 13:55:06 -0700 Subject: [PATCH 67/91] Use ThumbsFeedback for tour as well --- .../controls/Feedback/ThumbsFeedback.tsx | 14 +++++++++--- .../components/controls/TeachingBubble.tsx | 18 +++------------ .../styles/onboarding/TeachingBubble.less | 22 ++----------------- webapp/src/components/onboarding/Tour.tsx | 11 +++------- 4 files changed, 19 insertions(+), 46 deletions(-) diff --git a/react-common/components/controls/Feedback/ThumbsFeedback.tsx b/react-common/components/controls/Feedback/ThumbsFeedback.tsx index fdda82f6d24e..9f7b1c0461ec 100644 --- a/react-common/components/controls/Feedback/ThumbsFeedback.tsx +++ b/react-common/components/controls/Feedback/ThumbsFeedback.tsx @@ -3,7 +3,7 @@ import { classList } from "../../util"; import { Button } from "../Button"; interface ThumbsFeedbackProps { - onFeedbackSelected: (positive: boolean) => void; + onFeedbackSelected: (positive: boolean | undefined) => void; lockOnSelect?: boolean; positiveFeedbackText?: string; negativeFeedbackText?: string; @@ -23,9 +23,17 @@ export const ThumbsFeedback = (props: ThumbsFeedbackProps) => { } = props; const [selectedFeedback, setSelectedFeedback] = React.useState(undefined); + React.useEffect(() => { + onFeedbackSelected(selectedFeedback); + }, [selectedFeedback]); + const handleFeedbackSelected = (positive: boolean) => { - setSelectedFeedback(positive); - onFeedbackSelected(positive); + if (positive === selectedFeedback) { + // If the user clicks the same feedback button again, reset it + setSelectedFeedback(undefined); + } else { + setSelectedFeedback(positive); + } }; const positiveText = positiveFeedbackText || lf("Helpful"); diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index 285c54764af8..68e64090a35f 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -5,6 +5,7 @@ import { Confetti } from "../animations/Confetti"; import { ContainerProps, classList } from "../util"; import { FocusTrap } from "./FocusTrap"; import { useEffect } from "react"; +import { ThumbsFeedback } from "./Feedback/ThumbsFeedback"; export interface CutoutBounds { @@ -394,21 +395,8 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { {hasSteps &&
{stepNumber} of {totalSteps}
} - { onFeedback &&
-
+ {footer &&
+ {footer} +
}
, parentElement || document.getElementById("root") || document.body) diff --git a/react-common/styles/controls/AIFooter.less b/react-common/styles/controls/AIFooter.less new file mode 100644 index 000000000000..1c7e4c635216 --- /dev/null +++ b/react-common/styles/controls/AIFooter.less @@ -0,0 +1,14 @@ +.ai-footer { + display: flex; + flex-direction: row; + justify-content: space-between; + align-items: center; + width: 100%; + height: fit-content; + font-size: 14px; + line-height: 14px; + + .feedback-button i { + font-size: 14px; + } +} \ No newline at end of file diff --git a/react-common/styles/onboarding/TeachingBubble.less b/react-common/styles/onboarding/TeachingBubble.less index e6382520dae8..cbb0fb1eb04b 100644 --- a/react-common/styles/onboarding/TeachingBubble.less +++ b/react-common/styles/onboarding/TeachingBubble.less @@ -78,19 +78,11 @@ font-size: 1.1rem; p { - margin: 0.5rem 0 0 0; + margin: 0.5rem 0; } } -.teaching-bubble-notice { - margin: 0.8rem 0; - padding: 0; - font-size: 0.9rem; - line-height: 0.9rem; - color: var(--pxt-neutral-alpha80); -} - -.teaching-bubble-footer { +.teaching-bubble-navigation { display: flex; flex-direction: row; align-items: flex-end; @@ -133,6 +125,14 @@ } } +.teaching-bubble-footer { + color: var(--pxt-neutral-alpha80); + margin-top: 0.5rem; + margin: 0.5rem -1rem -1rem -1rem; // negative margins compensate for the padding on the bubble + padding: 0.5rem 1rem; + border-top: 1px solid var(--pxt-neutral-alpha50); +} + .teaching-bubble-close.common-button { position: absolute; right: 0.5rem; @@ -160,7 +160,7 @@ color: @highContrastTextColor; border: solid @highContrastTextColor; - .teaching-bubble-navigation>.common-button { + .teaching-bubble-navigation-buttons>.common-button { color: @highContrastTextColor; border: solid @highContrastTextColor; } diff --git a/react-common/styles/react-common.less b/react-common/styles/react-common.less index 023af572bed8..6be6fc59f4cc 100644 --- a/react-common/styles/react-common.less +++ b/react-common/styles/react-common.less @@ -29,6 +29,7 @@ @import "controls/CarouselNav.less"; @import "controls/Feedback.less"; @import "controls/ThumbsFeedback.less"; +@import "controls/AIFooter.less"; @import "theming/base-theme.less"; @import "./react-common-variables.less"; @import "./semantic-ui-overrides.less"; diff --git a/theme/ai-error-explanation-text.less b/theme/ai-error-explanation-text.less index a281e8d42634..4ec67b3b0180 100644 --- a/theme/ai-error-explanation-text.less +++ b/theme/ai-error-explanation-text.less @@ -1,4 +1,4 @@ -.error-list-ai-explanation-container { +.ai-explanation-container { display: flex; flex-direction: column; justify-content: flex-start; @@ -8,25 +8,11 @@ white-space: pre-line; // Preserve new lines .ai-footer { - display: flex; - flex-direction: row; - justify-content: space-between; - align-items: flex-end; - width: 100%; - height: fit-content; - margin-top: 0.5rem; - font-size: 0.9rem; - line-height: 0.9rem; + margin-top: 1rem; - .feedback-button { - i { - font-size: 0.9rem; - } - - &.disabled { - // Override the default disabled coloring - color: var(--pxt-neutral1-foreground); - } + .feedback-button.disabled { + // Override the default disabled coloring + color: var(--pxt-neutral1-foreground); } } } \ No newline at end of file diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index 83c4bb4fe7bf..e7500316db11 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -41,6 +41,7 @@ import { Measurements } from "./constants"; import { flow } from "../../pxtblocks"; import { HIDDEN_CLASS_NAME } from "../../pxtblocks/plugins/flyout/blockInflater"; import { FlyoutButton } from "../../pxtblocks/plugins/flyout/flyoutButton"; +import { AIFooter } from "../../react-common/components/controls/AIFooter"; interface CopyDataEntry { version: 1; @@ -1042,7 +1043,6 @@ export class Editor extends toolboxeditor.ToolboxEditor { description: step.message, location: pxt.tour.BubbleLocation.Center, bubbleStyle: "yellow", - notice: lf("AI generated content may be incorrect.") } as pxt.tour.BubbleStep; if (step.elementId && validBlockIds.includes(step.elementId)) { @@ -1061,14 +1061,12 @@ export class Editor extends toolboxeditor.ToolboxEditor { steps: tourSteps, showConfetti: false, numberFinalStep: true, - onFeedback: (positive: boolean) => { - this.handleErrorHelpFeedback(positive, { + footer: this.handleErrorHelpFeedback(positive, { type: "tour", tourStepCount: response.explanationSteps.length, errorCount: this.errors.length, invalidBlockIdCount: invalidBlockIdCount, - }); - } + })} feedbackLockOnSelect={true} /> }; } diff --git a/webapp/src/components/AIErrorExplanationText.tsx b/webapp/src/components/AIErrorExplanationText.tsx index d88db3cd1499..4c73aa355960 100644 --- a/webapp/src/components/AIErrorExplanationText.tsx +++ b/webapp/src/components/AIErrorExplanationText.tsx @@ -1,4 +1,4 @@ -import { ThumbsFeedback } from "../../../react-common/components/controls/Feedback/ThumbsFeedback"; +import { AIFooter } from "../../../react-common/components/controls/AIFooter"; interface AIErrorExplanationTextProps { explanation: string; @@ -16,12 +16,9 @@ export const AIErrorExplanationText = (props: AIErrorExplanationTextProps) => { } = props; return ( -
+
{explanation}
-
-
{lf("AI generated content may be incorrect.")}
- -
+
); }; diff --git a/webapp/src/components/onboarding/Tour.tsx b/webapp/src/components/onboarding/Tour.tsx index d95c4bd85b1f..014053936a1c 100644 --- a/webapp/src/components/onboarding/Tour.tsx +++ b/webapp/src/components/onboarding/Tour.tsx @@ -8,9 +8,8 @@ export interface TourProps { export const Tour = (props: TourProps) => { const { onClose, config } = props; - const { steps } = config; + const { steps, footer } = config; const [currentStep, setCurrentStep] = useState(0); - const [selectedFeedback, setSelectedFeedback] = useState(undefined); const tourStartTime = useRef(Date.now()); const stepStartTime = useRef(Date.now()); @@ -44,23 +43,14 @@ export const Tour = (props: TourProps) => { setCurrentStep(currentStep - 1); }; - const handleClose = () => { - // Send feedback once the tour closes - if (config.onFeedback && selectedFeedback !== undefined) { - config.onFeedback(selectedFeedback); - } - - onClose(); - } - const onExit = () => { pxt.tickEvent("tour.exit", data()); - handleClose(); + onClose(); } const onFinish = () => { pxt.tickEvent("tour.finish", data()); - handleClose(); + onClose(); } const isLastStep = currentStep === steps.length - 1; @@ -71,7 +61,6 @@ export const Tour = (props: TourProps) => { targetContent={steps[currentStep]} onNext={onNext} onBack={onBack} - onFeedback={config.onFeedback ? (b) => setSelectedFeedback(b) : undefined} stepNumber={currentStep + 1} totalSteps={totalDisplaySteps} hasPrevious={currentStep > 0} @@ -80,5 +69,6 @@ export const Tour = (props: TourProps) => { onFinish={onFinish} showConfetti={confetti} forceHideSteps={hideSteps} + footer={footer} /> }; \ No newline at end of file From 7d96ad019be614bd11dbc8354b30951ce57acca0 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 20 May 2025 12:27:43 -0700 Subject: [PATCH 90/91] Remove need for negative margins --- react-common/components/controls/TeachingBubble.tsx | 8 ++++---- react-common/styles/onboarding/TeachingBubble.less | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/react-common/components/controls/TeachingBubble.tsx b/react-common/components/controls/TeachingBubble.tsx index 98dc149ff85c..f40335146cd3 100644 --- a/react-common/components/controls/TeachingBubble.tsx +++ b/react-common/components/controls/TeachingBubble.tsx @@ -381,7 +381,7 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { ariaLabel={closeLabel} rightIcon="fas fa-times-circle" /> -
+
{targetContent.title}

{targetContent.description}

@@ -412,10 +412,10 @@ export const TeachingBubble = (props: TeachingBubbleProps) => { />}
- {footer &&
- {footer} -
}
+ {footer &&
+ {footer} +
}
, parentElement || document.getElementById("root") || document.body) } diff --git a/react-common/styles/onboarding/TeachingBubble.less b/react-common/styles/onboarding/TeachingBubble.less index cbb0fb1eb04b..167ab2fe4ad9 100644 --- a/react-common/styles/onboarding/TeachingBubble.less +++ b/react-common/styles/onboarding/TeachingBubble.less @@ -44,7 +44,6 @@ color: var(--teaching-bubble-foreground); box-shadow: 0 0 0 0.1rem; border-radius: 0.5rem; - padding: 1rem; z-index: @modalDimmerZIndex; .common-button { @@ -74,7 +73,8 @@ color: var(--teaching-bubble-foreground); } -.teaching-bubble-content { +.teaching-bubble-body { + padding: 1rem; font-size: 1.1rem; p { @@ -128,7 +128,7 @@ .teaching-bubble-footer { color: var(--pxt-neutral-alpha80); margin-top: 0.5rem; - margin: 0.5rem -1rem -1rem -1rem; // negative margins compensate for the padding on the bubble + margin: 0; // negative margins compensate for the padding on the bubble padding: 0.5rem 1rem; border-top: 1px solid var(--pxt-neutral-alpha50); } From ef404d50a3b64ff50f1bbfa0dad06aa6dc570129 Mon Sep 17 00:00:00 2001 From: thsparks Date: Tue, 20 May 2025 13:43:04 -0700 Subject: [PATCH 91/91] Lock ai feedback in tour after selected. Trying to support changing throughout and submitting at the end was getting too messy. --- react-common/components/controls/AIFooter.tsx | 4 +--- .../components/controls/Feedback/ThumbsFeedback.tsx | 6 ++---- react-common/styles/onboarding/TeachingBubble.less | 5 +++++ webapp/src/blocks.tsx | 2 +- webapp/src/components/AIErrorExplanationText.tsx | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/react-common/components/controls/AIFooter.tsx b/react-common/components/controls/AIFooter.tsx index 9ab8383ef3ac..ac6258544fe0 100644 --- a/react-common/components/controls/AIFooter.tsx +++ b/react-common/components/controls/AIFooter.tsx @@ -3,7 +3,6 @@ import { ThumbsFeedback } from "./Feedback/ThumbsFeedback"; interface AIFooterProps { className?: string; // Optional class name to add to the footer - feedbackLockOnSelect?: boolean; // If true, the user cannot change their feedback selection once made onFeedbackSelected: (positive: boolean | undefined) => void; // Callback function to handle feedback selection } @@ -13,14 +12,13 @@ interface AIFooterProps { export const AIFooter = (props: AIFooterProps) => { const { className, - feedbackLockOnSelect, onFeedbackSelected } = props; return (
{lf("AI generated content may be incorrect.")}
- +
); }; diff --git a/react-common/components/controls/Feedback/ThumbsFeedback.tsx b/react-common/components/controls/Feedback/ThumbsFeedback.tsx index 20bfb1d6c8ce..a014584d58f5 100644 --- a/react-common/components/controls/Feedback/ThumbsFeedback.tsx +++ b/react-common/components/controls/Feedback/ThumbsFeedback.tsx @@ -27,16 +27,14 @@ export const ThumbsFeedback = (props: ThumbsFeedbackProps) => { } = props; const [selectedFeedback, setSelectedFeedback] = React.useState(undefined); - React.useEffect(() => { - onFeedbackSelected(selectedFeedback); - }, [selectedFeedback]); - const handleFeedbackSelected = (positive: boolean) => { if (positive === selectedFeedback) { // If the user clicks the same feedback button again, reset it setSelectedFeedback(undefined); + onFeedbackSelected(undefined); } else { setSelectedFeedback(positive); + onFeedbackSelected(positive); } }; diff --git a/react-common/styles/onboarding/TeachingBubble.less b/react-common/styles/onboarding/TeachingBubble.less index 167ab2fe4ad9..3bd049fb7893 100644 --- a/react-common/styles/onboarding/TeachingBubble.less +++ b/react-common/styles/onboarding/TeachingBubble.less @@ -131,6 +131,11 @@ margin: 0; // negative margins compensate for the padding on the bubble padding: 0.5rem 1rem; border-top: 1px solid var(--pxt-neutral-alpha50); + + .ai-footer .feedback-button.disabled { + // Override the default disabled coloring + color: var(--pxt-neutral-alpha80); + } } .teaching-bubble-close.common-button { diff --git a/webapp/src/blocks.tsx b/webapp/src/blocks.tsx index e7500316db11..0284227e89ec 100644 --- a/webapp/src/blocks.tsx +++ b/webapp/src/blocks.tsx @@ -1066,7 +1066,7 @@ export class Editor extends toolboxeditor.ToolboxEditor { tourStepCount: response.explanationSteps.length, errorCount: this.errors.length, invalidBlockIdCount: invalidBlockIdCount, - })} feedbackLockOnSelect={true} /> + })} /> }; } diff --git a/webapp/src/components/AIErrorExplanationText.tsx b/webapp/src/components/AIErrorExplanationText.tsx index 4c73aa355960..2a5c60a75476 100644 --- a/webapp/src/components/AIErrorExplanationText.tsx +++ b/webapp/src/components/AIErrorExplanationText.tsx @@ -18,7 +18,7 @@ export const AIErrorExplanationText = (props: AIErrorExplanationTextProps) => { return (
{explanation}
- +
); };