diff --git a/core/procedures.ts b/core/procedures.ts index a16b0fce44..73f06836cf 100644 --- a/core/procedures.ts +++ b/core/procedures.ts @@ -42,6 +42,8 @@ import {IProcedureModel} from './interfaces/i_procedure_model.js'; import {Msg} from './msg.js'; import {Names} from './names.js'; import {ObservableProcedureMap} from './observable_procedure_map.js'; +import * as deprecation from './utils/deprecation.js'; +import type {FlyoutItemInfo} from './utils/toolbox.js'; import * as utilsXml from './utils/xml.js'; import * as Variables from './variables.js'; import type {Workspace} from './workspace.js'; @@ -238,7 +240,7 @@ export function rename(this: Field, name: string): string { * @param workspace The workspace containing procedures. * @returns Array of XML block elements. */ -export function flyoutCategory(workspace: WorkspaceSvg): Element[] { +function xmlFlyoutCategory(workspace: WorkspaceSvg): Element[] { const xmlList = []; if (Blocks['procedures_defnoreturn']) { // @@ -322,6 +324,109 @@ export function flyoutCategory(workspace: WorkspaceSvg): Element[] { return xmlList; } +/** + * Internal wrapper that returns the contents of the procedure category. + * + * @internal + * @param workspace The workspace to populate procedure blocks for. + */ +export function internalFlyoutCategory( + workspace: WorkspaceSvg, +): FlyoutItemInfo[] { + return flyoutCategory(workspace, false); +} + +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml: true, +): Element[]; +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml: false, +): FlyoutItemInfo[]; +/** + * Construct the blocks required by the flyout for the procedure category. + * + * @param workspace The workspace containing procedures. + * @param useXml True to return the contents as XML, false to use JSON. + * @returns List of flyout contents as either XML or JSON. + */ +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml = true, +): Element[] | FlyoutItemInfo[] { + if (useXml) { + deprecation.warn( + 'The XML return value of Blockly.Procedures.flyoutCategory()', + 'v12', + 'v13', + 'the same method, but handle a return type of FlyoutItemInfo[] (JSON) instead.', + ); + return xmlFlyoutCategory(workspace); + } + const blocks = []; + if (Blocks['procedures_defnoreturn']) { + blocks.push({ + 'kind': 'block', + 'type': 'procedures_defnoreturn', + 'gap': 16, + 'fields': { + 'NAME': Msg['PROCEDURES_DEFNORETURN_PROCEDURE'], + }, + }); + } + if (Blocks['procedures_defreturn']) { + blocks.push({ + 'kind': 'block', + 'type': 'procedures_defreturn', + 'gap': 16, + 'fields': { + 'NAME': Msg['PROCEDURES_DEFRETURN_PROCEDURE'], + }, + }); + } + if (Blocks['procedures_ifreturn']) { + blocks.push({ + 'kind': 'block', + 'type': 'procedures_ifreturn', + 'gap': 16, + }); + } + if (blocks.length) { + // Add slightly larger gap between system blocks and user calls. + blocks[blocks.length - 1]['gap'] = 24; + } + + /** + * Creates JSON block definitions for each of the given procedures. + * + * @param procedureList A list of procedures, each of which is defined by a + * three-element list of name, parameter list, and return value boolean. + * @param templateName The type of the block to generate. + */ + function populateProcedures( + procedureList: ProcedureTuple[], + templateName: string, + ) { + for (const [name, args] of procedureList) { + blocks.push({ + 'kind': 'block', + 'type': templateName, + 'gap': 16, + 'extraState': { + 'name': name, + 'params': args, + }, + }); + } + } + + const tuple = allProcedures(workspace); + populateProcedures(tuple[0], 'procedures_callnoreturn'); + populateProcedures(tuple[1], 'procedures_callreturn'); + return blocks; +} + /** * Updates the procedure mutator's flyout so that the arg block is not a * duplicate of another arg. diff --git a/core/variables.ts b/core/variables.ts index 5d60b475b1..c896efd0f1 100644 --- a/core/variables.ts +++ b/core/variables.ts @@ -13,6 +13,8 @@ import {isLegacyProcedureDefBlock} from './interfaces/i_legacy_procedure_blocks. import {isVariableBackedParameterModel} from './interfaces/i_variable_backed_parameter_model.js'; import {IVariableModel, IVariableState} from './interfaces/i_variable_model.js'; import {Msg} from './msg.js'; +import * as deprecation from './utils/deprecation.js'; +import type {BlockInfo, FlyoutItemInfo} from './utils/toolbox.js'; import * as utilsXml from './utils/xml.js'; import type {Workspace} from './workspace.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -84,19 +86,165 @@ export function allDeveloperVariables(workspace: Workspace): string[] { return Array.from(variables.values()); } +/** + * Internal wrapper that returns the contents of the variables category. + * + * @internal + * @param workspace The workspace to populate variable blocks for. + */ +export function internalFlyoutCategory( + workspace: WorkspaceSvg, +): FlyoutItemInfo[] { + return flyoutCategory(workspace, false); +} + +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml: true, +): Element[]; +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml: false, +): FlyoutItemInfo[]; /** * Construct the elements (blocks and button) required by the flyout for the * variable category. * * @param workspace The workspace containing variables. - * @returns Array of XML elements. + * @param useXml True to return the contents as XML, false to use JSON. + * @returns List of flyout contents as either XML or JSON. */ -export function flyoutCategory(workspace: WorkspaceSvg): Element[] { +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml = true, +): Element[] | FlyoutItemInfo[] { if (!Blocks['variables_set'] && !Blocks['variables_get']) { console.warn( 'There are no variable blocks, but there is a variable category.', ); } + + if (useXml) { + deprecation.warn( + 'The XML return value of Blockly.Variables.flyoutCategory()', + 'v12', + 'v13', + 'the same method, but handle a return type of FlyoutItemInfo[] (JSON) instead.', + ); + return xmlFlyoutCategory(workspace); + } + + workspace.registerButtonCallback('CREATE_VARIABLE', function (button) { + createVariableButtonHandler(button.getTargetWorkspace()); + }); + + return [ + { + 'kind': 'button', + 'text': '%{BKY_NEW_VARIABLE}', + 'callbackkey': 'CREATE_VARIABLE', + }, + ...jsonFlyoutCategoryBlocks( + workspace, + workspace.getVariablesOfType(''), + true, + ), + ]; +} + +/** + * Returns the JSON definition for a variable field. + * + * @param variable The variable the field should reference. + * @returns JSON for a variable field. + */ +function generateVariableFieldJson(variable: IVariableModel) { + return { + 'VAR': { + 'name': variable.getName(), + 'type': variable.getType(), + }, + }; +} + +/** + * Construct the blocks required by the flyout for the variable category. + * + * @internal + * @param workspace The workspace containing variables. + * @param variables List of variables to create blocks for. + * @param includeChangeBlocks True to include `change x by _` blocks. + * @param getterType The type of the variable getter block to generate. + * @param setterType The type of the variable setter block to generate. + * @returns JSON list of blocks. + */ +export function jsonFlyoutCategoryBlocks( + workspace: Workspace, + variables: IVariableModel[], + includeChangeBlocks: boolean, + getterType = 'variables_get', + setterType = 'variables_set', +): BlockInfo[] { + includeChangeBlocks &&= Blocks['math_change']; + + const blocks = []; + const mostRecentVariable = variables.slice(-1)[0]; + if (mostRecentVariable) { + // Show one setter block, with the name of the most recently created variable. + if (Blocks[setterType]) { + blocks.push({ + kind: 'block', + type: setterType, + gap: includeChangeBlocks ? 8 : 24, + fields: generateVariableFieldJson(mostRecentVariable), + }); + } + + if (includeChangeBlocks) { + blocks.push({ + 'kind': 'block', + 'type': 'math_change', + 'gap': Blocks[getterType] ? 20 : 8, + 'fields': generateVariableFieldJson(mostRecentVariable), + 'inputs': { + 'DELTA': { + 'shadow': { + 'type': 'math_number', + 'fields': { + 'NUM': 1, + }, + }, + }, + }, + }); + } + } + + if (Blocks[getterType]) { + // Show one getter block for each variable, sorted in alphabetical order. + blocks.push( + ...variables.sort(compareByName).map((variable) => { + return { + 'kind': 'block', + 'type': getterType, + 'gap': 8, + 'fields': generateVariableFieldJson(variable), + }; + }), + ); + } + + return blocks; +} + +/** + * Construct the elements (blocks and button) required by the flyout for the + * variable category. + * + * @param workspace The workspace containing variables. + * @returns Array of XML elements. + */ +function xmlFlyoutCategory(workspace: WorkspaceSvg): Element[] { let xmlList = new Array(); const button = document.createElement('button'); button.setAttribute('text', '%{BKY_NEW_VARIABLE}'); diff --git a/core/variables_dynamic.ts b/core/variables_dynamic.ts index 6722f8b49f..4e2682ce8e 100644 --- a/core/variables_dynamic.ts +++ b/core/variables_dynamic.ts @@ -9,6 +9,8 @@ import {Blocks} from './blocks.js'; import type {FlyoutButton} from './flyout_button.js'; import {Msg} from './msg.js'; +import * as deprecation from './utils/deprecation.js'; +import type {FlyoutItemInfo} from './utils/toolbox.js'; import * as xml from './utils/xml.js'; import * as Variables from './variables.js'; import type {Workspace} from './workspace.js'; @@ -68,19 +70,100 @@ function colourButtonClickHandler(button: FlyoutButton) { // eslint-disable-next-line camelcase export const onCreateVariableButtonClick_Colour = colourButtonClickHandler; +/** + * Internal wrapper that returns the contents of the dynamic variables category. + * + * @internal + * @param workspace The workspace to populate variable blocks for. + */ +export function internalFlyoutCategory( + workspace: WorkspaceSvg, +): FlyoutItemInfo[] { + return flyoutCategory(workspace, false); +} + +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml: true, +): Element[]; +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml: false, +): FlyoutItemInfo[]; /** * Construct the elements (blocks and button) required by the flyout for the - * variable category. + * dynamic variables category. * - * @param workspace The workspace containing variables. - * @returns Array of XML elements. + * @param useXml True to return the contents as XML, false to use JSON. + * @returns List of flyout contents as either XML or JSON. */ -export function flyoutCategory(workspace: WorkspaceSvg): Element[] { +export function flyoutCategory( + workspace: WorkspaceSvg, + useXml = true, +): Element[] | FlyoutItemInfo[] { if (!Blocks['variables_set_dynamic'] && !Blocks['variables_get_dynamic']) { console.warn( 'There are no dynamic variable blocks, but there is a dynamic variable category.', ); } + + if (useXml) { + deprecation.warn( + 'The XML return value of Blockly.VariablesDynamic.flyoutCategory()', + 'v12', + 'v13', + 'the same method, but handle a return type of FlyoutItemInfo[] (JSON) instead.', + ); + return xmlFlyoutCategory(workspace); + } + + workspace.registerButtonCallback( + 'CREATE_VARIABLE_STRING', + stringButtonClickHandler, + ); + workspace.registerButtonCallback( + 'CREATE_VARIABLE_NUMBER', + numberButtonClickHandler, + ); + workspace.registerButtonCallback( + 'CREATE_VARIABLE_COLOUR', + colourButtonClickHandler, + ); + + return [ + { + 'kind': 'button', + 'text': Msg['NEW_STRING_VARIABLE'], + 'callbackkey': 'CREATE_VARIABLE_STRING', + }, + { + 'kind': 'button', + 'text': Msg['NEW_NUMBER_VARIABLE'], + 'callbackkey': 'CREATE_VARIABLE_NUMBER', + }, + { + 'kind': 'button', + 'text': Msg['NEW_COLOUR_VARIABLE'], + 'callbackkey': 'CREATE_VARIABLE_COLOUR', + }, + ...Variables.jsonFlyoutCategoryBlocks( + workspace, + workspace.getAllVariables(), + false, + 'variables_get_dynamic', + 'variables_set_dynamic', + ), + ]; +} + +/** + * Construct the elements (blocks and button) required by the flyout for the + * variable category. + * + * @param workspace The workspace containing variables. + * @returns Array of XML elements. + */ +function xmlFlyoutCategory(workspace: WorkspaceSvg): Element[] { let xmlList = new Array(); let button = document.createElement('button'); button.setAttribute('text', Msg['NEW_STRING_VARIABLE']); diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index 60288573ce..e9aac5b9de 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -365,24 +365,24 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { /** Manager in charge of markers and cursors. */ this.markerManager = new MarkerManager(this); - if (Variables && Variables.flyoutCategory) { + if (Variables && Variables.internalFlyoutCategory) { this.registerToolboxCategoryCallback( Variables.CATEGORY_NAME, - Variables.flyoutCategory, + Variables.internalFlyoutCategory, ); } - if (VariablesDynamic && VariablesDynamic.flyoutCategory) { + if (VariablesDynamic && VariablesDynamic.internalFlyoutCategory) { this.registerToolboxCategoryCallback( VariablesDynamic.CATEGORY_NAME, - VariablesDynamic.flyoutCategory, + VariablesDynamic.internalFlyoutCategory, ); } - if (Procedures && Procedures.flyoutCategory) { + if (Procedures && Procedures.internalFlyoutCategory) { this.registerToolboxCategoryCallback( Procedures.CATEGORY_NAME, - Procedures.flyoutCategory, + Procedures.internalFlyoutCategory, ); this.addChangeListener(Procedures.mutatorOpenListener); } diff --git a/tests/mocha/contextmenu_test.js b/tests/mocha/contextmenu_test.js index fe6d4be997..65896112bb 100644 --- a/tests/mocha/contextmenu_test.js +++ b/tests/mocha/contextmenu_test.js @@ -6,7 +6,6 @@ import {callbackFactory} from '../../build/src/core/contextmenu.js'; import * as xmlUtils from '../../build/src/core/utils/xml.js'; -import * as Variables from '../../build/src/core/variables.js'; import {assert} from '../../node_modules/chai/chai.js'; import { sharedTestSetup, @@ -32,9 +31,13 @@ suite('Context Menu', function () { }); test('callback with xml state creates block', function () { - const xmlField = Variables.generateVariableFieldDom( - this.forLoopBlock.getField('VAR').getVariable(), - ); + const variable = this.forLoopBlock.getField('VAR').getVariable(); + const xmlField = document.createElement('field'); + xmlField.setAttribute('name', 'VAR'); + xmlField.setAttribute('id', variable.getId()); + xmlField.setAttribute('variabletype', variable.getType()); + xmlField.textContent = variable.getName(); + const xmlBlock = xmlUtils.createElement('block'); xmlBlock.setAttribute('type', 'variables_get'); xmlBlock.appendChild(xmlField); diff --git a/tests/mocha/xml_test.js b/tests/mocha/xml_test.js index d30716edb4..218324197b 100644 --- a/tests/mocha/xml_test.js +++ b/tests/mocha/xml_test.js @@ -531,28 +531,6 @@ suite('XML', function () { teardown(function () { workspaceTeardown.call(this, this.workspace); }); - suite('Dynamic Category Blocks', function () { - test('Untyped Variables', function () { - this.workspace.createVariable('name1', '', 'id1'); - const blocksArray = Blockly.Variables.flyoutCategoryBlocks( - this.workspace, - ); - for (let i = 0, xml; (xml = blocksArray[i]); i++) { - Blockly.Xml.domToBlock(xml, this.workspace); - } - }); - test('Typed Variables', function () { - this.workspace.createVariable('name1', 'String', 'id1'); - this.workspace.createVariable('name2', 'Number', 'id2'); - this.workspace.createVariable('name3', 'Colour', 'id3'); - const blocksArray = Blockly.VariablesDynamic.flyoutCategoryBlocks( - this.workspace, - ); - for (let i = 0, xml; (xml = blocksArray[i]); i++) { - Blockly.Xml.domToBlock(xml, this.workspace); - } - }); - }); suite('Comments', function () { suite('Headless', function () { test('Text', function () { @@ -910,42 +888,4 @@ suite('XML', function () { }); }); }); - suite('generateVariableFieldDom', function () { - test('Case Sensitive', function () { - const varId = 'testId'; - const type = 'testType'; - const name = 'testName'; - - const mockVariableModel = { - type: type, - name: name, - getId: function () { - return varId; - }, - getName: function () { - return name; - }, - getType: function () { - return type; - }, - }; - - const generatedXml = Blockly.Xml.domToText( - Blockly.Variables.generateVariableFieldDom(mockVariableModel), - ); - const expectedXml = - '' + - name + - ''; - assert.equal(generatedXml, expectedXml); - }); - }); });