Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teacher Tool: Change block display #10202

Merged
merged 34 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
be86d7c
In progress still getting things working...
thsparks Sep 13, 2024
30dff95
Some display changes
thsparks Sep 14, 2024
a5f5c7a
Merge branch 'master' of https://github.com/microsoft/pxt into thspar…
thsparks Sep 19, 2024
fc0c083
TIdy code
thsparks Sep 19, 2024
ae159cd
Attempt to handle blocks with no "parts" attribute. Seems like these …
thsparks Sep 19, 2024
9763e5b
Merge branch 'master' of https://github.com/microsoft/pxt into thspar…
thsparks Sep 20, 2024
8af2714
Comment update
thsparks Sep 20, 2024
8bce495
Refactor getBlockReadableName to getReadableBlockName
thsparks Sep 20, 2024
466d31d
Merge branch 'master' of https://github.com/microsoft/pxt into thspar…
thsparks Sep 20, 2024
12739c2
Cache readable block names
thsparks Sep 20, 2024
7768d0e
Move getBlockDescription and getSnippetName into new file
thsparks Sep 20, 2024
2290097
ReadableBlockName -> BlockTextParts
thsparks Sep 20, 2024
f974f56
Move getBlockTextPartsFromBlocksBlockDefinition into helpers
thsparks Sep 20, 2024
ab84d3d
Refactor monacoFlyout to use shared getBlockTextParts function
thsparks Sep 21, 2024
b74e8ba
Don't cache for now (just because I need it uncached for testing)
thsparks Sep 21, 2024
df4b703
Replace all params with value
thsparks Sep 21, 2024
944acd7
Add snippet names for blocks
thsparks Sep 24, 2024
c131bfb
Use snippet names as fallback
thsparks Sep 24, 2024
6bf02b1
Styling adjustments
thsparks Sep 24, 2024
7799b36
Different name for the square root (and other operations) block. I st…
thsparks Sep 24, 2024
7b5a829
Re-include caching logic
thsparks Sep 24, 2024
d228a56
Remove unnecessary code movement change
thsparks Sep 24, 2024
dbe3493
Minor code cleanup
thsparks Sep 24, 2024
39a8500
Only rely on blockId (not blockData) to display the readable componen…
thsparks Sep 24, 2024
1de0863
Clean up code in app.tsx
thsparks Sep 25, 2024
b19ddac
Do not show duplicate names
thsparks Sep 25, 2024
d0f5fa0
Remove shared use of getSnippetName. It was causing more complexity t…
thsparks Sep 25, 2024
fbeefda
Update some block snippets
thsparks Sep 25, 2024
cae7c4d
blockTextParts -> blockAsText
thsparks Sep 25, 2024
d6673d8
Remove unnecessary newline
thsparks Sep 25, 2024
5e56afc
Fix typo
thsparks Sep 25, 2024
0ba72f2
Prettier
thsparks Sep 25, 2024
52da0b9
Remove unnecessary Promise.resolve
thsparks Sep 25, 2024
2864ed9
Use var for shadow color
thsparks Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions localtypings/pxteditor.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ declare namespace pxt.editor {
| "convertcloudprojectstolocal"
| "setlanguagerestriction"
| "gettoolboxcategories"
| "getblockastext"

| "toggletrace" // EditorMessageToggleTraceRequest
| "togglehighcontrast"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>;
Expand Down
10 changes: 9 additions & 1 deletion pxteditor/editorcontroller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
12 changes: 12 additions & 0 deletions pxtservices/editorDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
{
Expand Down
65 changes: 63 additions & 2 deletions teachertool/src/components/CriteriaInstanceDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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({
Expand All @@ -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}`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions teachertool/src/services/makecodeEditorService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 25 additions & 0 deletions teachertool/src/services/storageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
// ----------------------------------
Expand Down Expand Up @@ -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);
}
}
18 changes: 18 additions & 0 deletions teachertool/src/transforms/loadReadableBlockName.ts
Original file line number Diff line number Diff line change
@@ -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;
}
1 change: 1 addition & 0 deletions theme/themepacks.less
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
48 changes: 47 additions & 1 deletion webapp/src/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -4142,6 +4143,51 @@ export class ProjectView
return { categories };
}

getBlocksWithId(blockId: string): BlockDefinition[] {
const matches: BlockDefinition[] = [];
for (const advanced of [false, true]) {
srietkerk marked this conversation as resolved.
Show resolved Hide resolved
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());

Expand Down
Loading