From ea136ece9e3f5134c93226eba75d107aee293d52 Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Fri, 27 Sep 2024 15:07:35 -0700 Subject: [PATCH] Teacher Tool: Change block display (#10202) (#10211) This changes the inline block display for the "Block used count times" criteria in the teacher tool. I've reused the approach taken by the monaco editor to generate the javascript toolbox entries (meaning some of that code has been refactored into a shared helper method). This approach looks at the attributes for a block (mostly the block string), then breaks it up into labels, parameters, and breaks. Those sections are returned and can be displayed however the caller sees fit. I cache the information to minimize any loading time (fewer messages to the iframe) and to reduce the likelihood of ending up with a block where we can't get a display (i.e. a rubric with a block loaded from an extension, opened later from a project without that extension installed). --- localtypings/pxteditor.d.ts | 28 ++++++-- pxteditor/editorcontroller.ts | 10 ++- pxtservices/editorDriver.ts | 12 ++++ .../components/CriteriaInstanceDisplay.tsx | 65 ++++++++++++++++++- .../styling/BlockPickerModal.module.scss | 2 +- .../styling/CatalogOverlay.module.scss | 2 +- .../CriteriaInstanceDisplay.module.scss | 13 ++++ .../src/services/makecodeEditorService.ts | 5 ++ teachertool/src/services/storageService.ts | 25 +++++++ .../src/transforms/loadReadableBlockName.ts | 18 +++++ theme/themepacks.less | 1 + webapp/src/app.tsx | 48 +++++++++++++- webapp/src/blocks.tsx | 1 + webapp/src/blocksSnippets.ts | 39 ++++++++++- webapp/src/monacoFlyout.tsx | 41 +++--------- webapp/src/toolboxHelpers.ts | 48 ++++++++++++++ 16 files changed, 316 insertions(+), 42 deletions(-) create mode 100644 teachertool/src/transforms/loadReadableBlockName.ts create mode 100644 webapp/src/toolboxHelpers.ts diff --git a/localtypings/pxteditor.d.ts b/localtypings/pxteditor.d.ts index 7ba3c537a68e..432421125b76 100644 --- a/localtypings/pxteditor.d.ts +++ b/localtypings/pxteditor.d.ts @@ -75,6 +75,7 @@ declare namespace pxt.editor { | "convertcloudprojectstolocal" | "setlanguagerestriction" | "gettoolboxcategories" + | "getblockastext" | "toggletrace" // EditorMessageToggleTraceRequest | "togglehighcontrast" @@ -453,14 +454,23 @@ declare namespace pxt.editor { advanced?: boolean; } - export interface EditorMessageServiceWorkerRegisteredRequest extends EditorMessageRequest { - action: "serviceworkerregistered"; - } - export interface EditorMessageGetToolboxCategoriesResponse { categories: pxt.editor.ToolboxCategoryDefinition[]; } + export interface EditorMessageGetBlockAsTextRequest extends EditorMessageRequest { + action: "getblockastext"; + blockId: string; + } + + export interface EditorMessageGetBlockAsTextResponse { + blockAsText: pxt.editor.BlockAsText | undefined; + } + + export interface EditorMessageServiceWorkerRegisteredRequest extends EditorMessageRequest { + action: "serviceworkerregistered"; + } + export interface DataStreams { console?: T; messages?: T; @@ -997,6 +1007,7 @@ declare namespace pxt.editor { // getBlocks(): Blockly.Block[]; getBlocks(): any[]; getToolboxCategories(advanced?: boolean): pxt.editor.EditorMessageGetToolboxCategoriesResponse; + getBlockAsText(blockId: string): pxt.editor.BlockAsText | undefined; toggleHighContrast(): void; setHighContrast(on: boolean): void; @@ -1264,6 +1275,15 @@ declare namespace pxt.editor { blockId?: string; } + export interface BlockAsText { + parts: BlockTextPart[]; + } + + export interface BlockTextPart { + kind: "label" | "break" | "param", + content?: string, + } + interface BaseAssetEditorRequest { id?: number; files: pxt.Map; diff --git a/pxteditor/editorcontroller.ts b/pxteditor/editorcontroller.ts index a1bcf94afe0a..8c4e5b7f41ae 100644 --- a/pxteditor/editorcontroller.ts +++ b/pxteditor/editorcontroller.ts @@ -202,13 +202,21 @@ case "renderxml": { resp = results; }); } -case "gettoolboxcategories": { + case "gettoolboxcategories": { const msg = data as pxt.editor.EditorMessageGetToolboxCategoriesRequest; return Promise.resolve() .then(() => { resp = projectView.getToolboxCategories(msg.advanced); }); } + case "getblockastext": { + const msg = data as pxt.editor.EditorMessageGetBlockAsTextRequest; + return Promise.resolve() + .then(() => { + const readableName = projectView.getBlockAsText(msg.blockId); + resp = { blockAsText: readableName } as pxt.editor.EditorMessageGetBlockAsTextResponse; + }); + } case "renderpython": { const rendermsg = data as pxt.editor.EditorMessageRenderPythonRequest; return Promise.resolve() diff --git a/pxtservices/editorDriver.ts b/pxtservices/editorDriver.ts index bb75415ca0a7..e2104b317c92 100644 --- a/pxtservices/editorDriver.ts +++ b/pxtservices/editorDriver.ts @@ -389,6 +389,18 @@ export class EditorDriver extends IframeDriver { return (resp.resp as pxt.editor.EditorMessageGetToolboxCategoriesResponse).categories; } + async getBlockAsText(blockId: string): Promise { + const resp = await this.sendRequest( + { + type: "pxteditor", + action: "getblockastext", + blockId + } as pxt.editor.EditorMessageGetBlockAsTextRequest + ) as pxt.editor.EditorMessageResponse; + + return (resp.resp as pxt.editor.EditorMessageGetBlockAsTextResponse)?.blockAsText; + } + async runValidatorPlan(validatorPlan: pxt.blocks.ValidatorPlan, planLib: pxt.blocks.ValidatorPlan[]) { const resp = await this.sendRequest( { diff --git a/teachertool/src/components/CriteriaInstanceDisplay.tsx b/teachertool/src/components/CriteriaInstanceDisplay.tsx index d5b2be707794..27eb7a785a3f 100644 --- a/teachertool/src/components/CriteriaInstanceDisplay.tsx +++ b/teachertool/src/components/CriteriaInstanceDisplay.tsx @@ -4,7 +4,7 @@ import { CriteriaInstance, CriteriaParameterValue } from "../types/criteria"; import { logDebug } from "../services/loggingService"; import { setParameterValue } from "../transforms/setParameterValue"; import { classList } from "react-common/components/util"; -import { getReadableBlockString, splitCriteriaTemplate } from "../utils"; +import { splitCriteriaTemplate } from "../utils"; import { useContext, useEffect, useMemo, useState } from "react"; import { Input } from "react-common/components/controls/Input"; import { Button } from "react-common/components/controls/Button"; @@ -13,6 +13,7 @@ import { Strings, Ticks } from "../constants"; import { showModal } from "../transforms/showModal"; import { BlockPickerOptions } from "../types/modalOptions"; import { validateParameterValue } from "../utils/validateParameterValue"; +import { loadBlockAsText } from "../transforms/loadReadableBlockName"; interface InlineInputSegmentProps { initialValue: string; @@ -80,6 +81,64 @@ const InlineInputSegment: React.FC = ({ ); }; +interface ReadableBlockNameProps { + blockId: string; +} +const ReadableBlockName: React.FC = ({ blockId }) => { + const { state: teacherTool } = useContext(AppStateContext); + const [blockAsText, setBlockAsText] = useState(undefined); + + useEffect(() => { + async function updateReadableName(blockId: string | undefined) { + let blockReadableName: pxt.editor.BlockAsText | undefined; + if (blockId) { + blockReadableName = blockId ? await loadBlockAsText(blockId) : undefined; + } + + if (blockReadableName) { + setBlockAsText(blockReadableName); + } else if (!teacherTool.toolboxCategories) { + // If teacherTool.toolboxCategories has not loaded yet, we may get the readable component later once it loads. + // Show a spinner (handled below). + setBlockAsText(undefined); + } else { + // TeacherTool.toolboxCategories has loaded and we still don't have a readable component. + // We won't be able to get it, so fallback to the id. + setBlockAsText({ parts: [{ kind: "label", content: blockId }] }); + } + } + + updateReadableName(blockId); + }, [blockId, teacherTool.toolboxCategories]); + + const readableComponent = blockAsText?.parts.map((part, i) => { + let content = ""; + if (part.kind === "param") { + // Mask default values like "hello!" with generic "value" + // This is done to reduce confusion about what is actually being checked. + content = lf("value"); + } else if (part.kind === "label" && part.content) { + content = part.content; + } + + return ( + + {content} + + ); + }); + + return ( + {readableComponent ||
} + ); +}; + interface BlockInputSegmentProps { instance: CriteriaInstance; param: CriteriaParameterValue; @@ -90,6 +149,7 @@ interface BlockData { } const BlockInputSegment: React.FC = ({ instance, param }) => { const { state: teacherTool } = useContext(AppStateContext); + function handleClick() { pxt.tickEvent(Ticks.BlockPickerOpened, { criteriaCatalogId: instance.catalogCriteriaId }); showModal({ @@ -115,9 +175,10 @@ const BlockInputSegment: React.FC = ({ instance, param } }, [param.value, teacherTool.toolboxCategories]); const style = blockData ? { backgroundColor: blockData.category.color, color: "white" } : undefined; + const blockDisplay = param.value ? : param.name; return (
} -} \ No newline at end of file +} diff --git a/webapp/src/toolboxHelpers.ts b/webapp/src/toolboxHelpers.ts new file mode 100644 index 000000000000..b1fec200ac26 --- /dev/null +++ b/webapp/src/toolboxHelpers.ts @@ -0,0 +1,48 @@ +import * as toolbox from "./toolbox"; + +// Breaks a block down into segments that can be displayed in a readable format. +export function getBlockAsText(block: toolbox.BlockDefinition, params: pxtc.ParameterDesc[], isPython: boolean): pxt.editor.BlockAsText | undefined { + let description: pxt.editor.BlockTextPart[] = []; + let compileInfo = pxt.blocks.compileInfo(block as pxtc.SymbolInfo); + let parts = block.attributes._def && block.attributes._def.parts; + if (block.attributes.parentBlock) { + const parent = block.attributes.parentBlock; + const parentBlockParts = [...parent.attributes._def.parts]; + const overrideLocation = parentBlockParts.findIndex((part: any) => part.kind === "param" && part.name === block.attributes.toolboxParentArgument); + if (overrideLocation !== -1) { + parentBlockParts.splice(overrideLocation, 1, ...block.attributes._def.parts); + parts = parentBlockParts; + } + } + + if (parts) { + if (params && + parts.filter((p: any) => p.kind == "param").length > params.length) { + // add empty param when first argument is "this" + params.unshift(null); + } + parts.forEach((part, i) => { + switch (part.kind) { + case "label": + description.push({kind: "label", content: part.text}); + break; + case "break": + description.push({kind: "break"}); + break; + case "param": + let actualParam = compileInfo?.definitionNameToParam[part.name]; + let val = actualParam?.defaultValue + || part.varName + || actualParam?.actualName + || part.name + if (isPython && actualParam?.defaultValue) { + val = pxtc.tsSnippetToPySnippet(val); + } + description.push({kind: "param", content: val}); + break; + } + }) + } + + return description.length > 0 ? { parts: description } : undefined; +}