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

@@ -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<T> {
         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<string>;

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()

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<pxt.editor.BlockAsText | undefined> {
+        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(
             {

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<InlineInputSegmentProps> = ({
     );
 };
 
+interface ReadableBlockNameProps {
+    blockId: string;
+}
+const ReadableBlockName: React.FC<ReadableBlockNameProps> = ({ blockId }) => {
+    const { state: teacherTool } = useContext(AppStateContext);
+    const [blockAsText, setBlockAsText] = useState<pxt.editor.BlockAsText | undefined>(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 (
+            <span
+                key={`block-name-part-${i}`}
+                className={classList(
+                    css["block-name-segment"],
+                    part.kind === "param" ? css["block-name-param"] : css["block-name-label"]
+                )}
+            >
+                {content}
+            </span>
+        );
+    });
+
+    return (
+        <span className={css["block-readable-name"]}>{readableComponent || <div className="common-spinner" />}</span>
+    );
+};
+
 interface BlockInputSegmentProps {
     instance: CriteriaInstance;
     param: CriteriaParameterValue;
@@ -90,6 +149,7 @@ interface BlockData {
 }
 const BlockInputSegment: React.FC<BlockInputSegmentProps> = ({ 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<BlockInputSegmentProps> = ({ instance, param }
     }, [param.value, teacherTool.toolboxCategories]);
 
     const style = blockData ? { backgroundColor: blockData.category.color, color: "white" } : undefined;
+    const blockDisplay = param.value ? <ReadableBlockName blockId={param.value} /> : param.name;
     return (
         <Button
-            label={blockData ? getReadableBlockString(blockData.block.name) : param.value || param.name}
+            label={blockDisplay}
             className={classList(css["block-input-btn"], param.value ? undefined : css["error"])}
             onClick={handleClick}
             title={param.value ? Strings.SelectBlock : `${Strings.SelectBlock}: ${Strings.ValueRequired}`}

teachertool/src/components/styling/BlockPickerModal.module.scss

@@ -80,7 +80,7 @@
                             height: 3rem;
                             border-radius: 0.2rem;
                             color: white;
-                            border: 1px solid rgba(0, 0, 0, 0.5);
+                            border: 1px solid var(--pxt-page-dark-shadow);
                             font-size: 1rem;
                             font-weight: bold;
                             background-color: var(--category-color);

teachertool/src/components/styling/CatalogOverlay.module.scss

@@ -5,7 +5,7 @@
     width: 100%;
     height: 100%;
     z-index: 49; // Above everything except toasts
-    background-color: rgba(0, 0, 0, 0.5);
+    background-color: var(--pxt-page-dark-shadow);
 
     color: var(--pxt-page-foreground);
 

teachertool/src/components/styling/CriteriaInstanceDisplay.module.scss

@@ -49,6 +49,19 @@
                 border: solid 1px var(--pxt-page-foreground);
                 padding: 0.4rem;
 
+                .block-readable-name {
+                    .block-name-segment {
+                        margin-left: 0;
+                        margin-right: 0.2rem;
+                        line-height: normal;
+                    }
+                    .block-name-param {
+                        border-radius: 1rem;
+                        padding: 0.2rem 0.4rem;
+                        background-color: var(--pxt-page-dark-shadow);
+                    }
+                }
+
                 &.error {
                     border: solid 1px var(--pxt-error-accent);
                     background-color: var(--pxt-error-background);

teachertool/src/services/makecodeEditorService.ts

@@ -74,6 +74,11 @@ export async function getToolboxCategoriesAsync(
     return response;
 }
 
+export async function getBlockAsText(blockId: string): Promise<pxt.editor.BlockAsText | undefined> {
+    const response = driver ? await driver.getBlockAsText(blockId) : undefined;
+    return response;
+}
+
 export async function getBlockImageUriFromXmlAsync(xml: string): Promise<string | undefined> {
     const response = driver ? await driver.renderXml(xml) : undefined;
     return response;

teachertool/src/services/storageService.ts

@@ -12,6 +12,7 @@ const RUN_ON_LOAD_KEY = [KEY_PREFIX, "runOnLoad"].join("/");
 const LAST_ACTIVE_CHECKLIST_KEY = [KEY_PREFIX, "lastActiveChecklist"].join("/");
 const SPLIT_POSITION_KEY = [KEY_PREFIX, "splitPosition"].join("/");
 const EXPANDED_CATALOG_TAGS_KEY = [KEY_PREFIX, "expandedCatalogTags"].join("/");
+const BLOCK_AS_TEXT_PREFIX = [KEY_PREFIX, "blockAsText"].join("/");
 
 function getValue(key: string, defaultValue?: string): string | undefined {
     return localStorage.getItem(key) || defaultValue;
@@ -112,6 +113,10 @@ async function deleteChecklistFromIndexedDbAsync(name: string) {
     await db.deleteChecklist(name);
 }
 
+function getBlockAsTextKey(blockId: string): string {
+    return [BLOCK_AS_TEXT_PREFIX, blockId].join("/");
+}
+
 // ----------------------------------
 // Exports
 // ----------------------------------
@@ -235,3 +240,23 @@ export function removeExpandedCatalogTag(tag: string) {
         }
     }
 }
+
+export function getCachedBlockAsText(blockId: string): pxt.editor.BlockAsText | undefined {
+    const key = getBlockAsTextKey(blockId);
+    try {
+        const cachedReadableBlockName = getValue(key);
+        return cachedReadableBlockName ? JSON.parse(cachedReadableBlockName) : undefined;
+    } catch (e) {
+        logError(ErrorCode.localStorageReadError, e);
+        return undefined;
+    }
+}
+
+export function cacheBlockAsText(blockId: string, readableBlockName: pxt.editor.BlockAsText) {
+    const key = getBlockAsTextKey(blockId);
+    try {
+        setValue(key, JSON.stringify(readableBlockName));
+    } catch (e) {
+        logError(ErrorCode.localStorageWriteError, e);
+    }
+}

teachertool/src/transforms/loadReadableBlockName.ts

@@ -0,0 +1,18 @@
+import { getBlockAsText } from "../services/makecodeEditorService";
+import { cacheBlockAsText, getCachedBlockAsText } from "../services/storageService";
+
+export async function loadBlockAsText(blockId: string): Promise<pxt.editor.BlockAsText | undefined> {
+    // Check for cached version.
+    let readableBlockName = getCachedBlockAsText(blockId);
+    if (readableBlockName) {
+        return readableBlockName;
+    }
+
+    // Call into editor service & cache result
+    readableBlockName = await getBlockAsText(blockId);
+    if (readableBlockName) {
+        cacheBlockAsText(blockId, readableBlockName);
+    }
+
+    return readableBlockName;
+}

theme/themepacks.less

@@ -15,6 +15,7 @@
     --pxt-page-foreground: black;
     --pxt-page-foreground-light: #767676;
     --pxt-page-foreground-shadow: rgba(0, 0, 0, 0.08);
+    --pxt-page-dark-shadow: rgba(0, 0, 0, 0.5);
     /// Header bar
     --pxt-headerbar-background: #475569;
     --pxt-headerbar-background-glass: #47556940;

webapp/src/app.tsx

@@ -49,6 +49,7 @@ import * as sidepanel from "./sidepanel";
 import * as qr from "./qr";
 
 import * as monaco from "./monaco"
+import * as toolboxHelpers from "./toolboxHelpers"
 import * as pxtjson from "./pxtjson"
 import * as serial from "./serial"
 import * as blocks from "./blocks"
@@ -80,7 +81,7 @@ import { mergeProjectCode, appendTemporaryAssets } from "./mergeProjects";
 import { Tour } from "./components/onboarding/Tour";
 import { parseTourStepsAsync } from "./onboarding";
 import { initGitHubDb } from "./idbworkspace";
-import { CategoryNameID } from "./toolbox";
+import { BlockDefinition, CategoryNameID } from "./toolbox";
 
 pxt.blocks.requirePxtBlockly = () => pxtblockly as any;
 pxt.blocks.requireBlockly = () => Blockly;
@@ -4142,6 +4143,51 @@ export class ProjectView
         return { categories };
     }
 
+    getBlocksWithId(blockId: string): BlockDefinition[] {
+        const matches: BlockDefinition[] = [];
+        for (const advanced of [false, true]) {
+            for (const category of this.blocksEditor.getToolboxCategories(advanced)) {
+                for (const match of category.blocks?.filter(b => b.attributes.blockId === blockId) ?? []) {
+                    matches.push(match);
+                }
+            }
+        }
+
+        return matches;
+    }
+
+    getBlockAsText(blockId: string): pxt.editor.BlockAsText | undefined {
+        const blocksWithId = this.getBlocksWithId(blockId);
+        let readableName: pxt.editor.BlockAsText = undefined;
+        if (blocksWithId.length === 0) {
+            return undefined;
+        } else if (blocksWithId.length === 1) {
+            const block = blocksWithId[0];
+            readableName = toolboxHelpers.getBlockAsText(block, block.parameters ? block.parameters.slice() : null, false);
+        }
+
+        if (!readableName) {
+            // Found multiple blocks matching the id, or we were unable to generate a readable name from block parameters.
+            // In this case, use the block snippet names to describe the block.
+            const blockSnippets: string[] = [];
+            for (const name of blocksWithId.map(b => b.snippetName || b.name)) {
+                if (blockSnippets.indexOf(name) === -1) {
+                    blockSnippets.push(name);
+                }
+            }
+            readableName = {
+                parts: [
+                    {
+                        kind: "label",
+                        content: blockSnippets.join(" | ")
+                    }
+                ],
+            }
+        }
+
+        return readableName;
+    }
+
     launchFullEditor() {
         Util.assert(pxt.shell.isSandboxMode());