From fa2228d1f5a675fdd7c198d14875786561c70402 Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Fri, 18 Sep 2026 15:04:00 -0400 Subject: [PATCH 1/7] feat: toolbox-search updates --- packages/blockly/msg/json/en.json | 7 +- packages/blockly/msg/json/qqq.json | 7 +- packages/blockly/msg/messages.js | 18 +- .../toolbox-search/src/block_searcher.ts | 188 +++++++++++++++-- .../toolbox-search/src/toolbox_search.ts | 193 ++++++++++++++---- packages/plugins/toolbox-search/test/index.ts | 185 ++++++++++++++++- .../toolbox-search/test/tests.mocha.js | 32 ++- 7 files changed, 550 insertions(+), 80 deletions(-) diff --git a/packages/blockly/msg/json/en.json b/packages/blockly/msg/json/en.json index 1fe29609ba6..e778a82b5a8 100644 --- a/packages/blockly/msg/json/en.json +++ b/packages/blockly/msg/json/en.json @@ -671,5 +671,10 @@ "WORKSPACE_SEARCH_FIND_PREVIOUS": "Find previous", "WORKSPACE_SEARCH_CLOSE": "Close search bar", "WORKSPACE_SEARCH_NO_MATCHES": "No matching blocks", - "WORKSPACE_SEARCH_MATCH": "Match %1 of %2: %3" + "WORKSPACE_SEARCH_MATCH": "Match %1 of %2: %3", + "TOOLBOX_SEARCH_PLACEHOLDER": "Search for blocks", + "TOOLBOX_SEARCH_PROMPT": "Type to search for blocks", + "TOOLBOX_SEARCH_NO_RESULTS": "No matching blocks found", + "TOOLBOX_SEARCH_RESULT_COUNT_ONE": "1 matching block", + "TOOLBOX_SEARCH_RESULT_COUNT": "%1 matching blocks" } diff --git a/packages/blockly/msg/json/qqq.json b/packages/blockly/msg/json/qqq.json index 320dba23d35..7a9770ce39d 100644 --- a/packages/blockly/msg/json/qqq.json +++ b/packages/blockly/msg/json/qqq.json @@ -676,5 +676,10 @@ "WORKSPACE_SEARCH_FIND_PREVIOUS": "ARIA label for the workspace search button that selects the previous matching block.", "WORKSPACE_SEARCH_CLOSE": "ARIA label for the button that closes the workspace search bar.", "WORKSPACE_SEARCH_NO_MATCHES": "ARIA live region message announced when workspace search finds no matching blocks.", - "WORKSPACE_SEARCH_MATCH": "ARIA live region message announcing the currently highlighted workspace search match. \n\nParameters:\n* %1 - 1-based index of the current match\n* %2 - total number of matches\n* %3 - accessible label of the current block \n\nExamples:\n* 'Match 1 of 3: print, hello'" + "WORKSPACE_SEARCH_MATCH": "ARIA live region message announcing the currently highlighted workspace search match. \n\nParameters:\n* %1 - 1-based index of the current match\n* %2 - total number of matches\n* %3 - accessible label of the current block \n\nExamples:\n* 'Match 1 of 3: print, hello'", + "TOOLBOX_SEARCH_PLACEHOLDER": "Placeholder text in the toolbox search category's input field.", + "TOOLBOX_SEARCH_PROMPT": "Message shown in the toolbox search flyout before a query has been entered.", + "TOOLBOX_SEARCH_NO_RESULTS": "Message shown in the toolbox search flyout when a query matches no blocks.", + "TOOLBOX_SEARCH_RESULT_COUNT_ONE": "Screen reader announcement when a toolbox search matches exactly one block.", + "TOOLBOX_SEARCH_RESULT_COUNT": "Screen reader announcement of how many blocks a toolbox search matched. \n\nParameters:\n* %1 - a number of blocks, which may be zero or greater than one." } diff --git a/packages/blockly/msg/messages.js b/packages/blockly/msg/messages.js index 068eda42a32..2529f5def0c 100644 --- a/packages/blockly/msg/messages.js +++ b/packages/blockly/msg/messages.js @@ -2533,4 +2533,20 @@ Blockly.Msg.WORKSPACE_SEARCH_NO_MATCHES = 'No matching blocks'; /// ARIA live region message announcing the currently highlighted workspace search match. /// \n\nParameters:\n* %1 - 1-based index of the current match\n* %2 - total number of matches\n* %3 - accessible label of the current block /// \n\nExamples:\n* "Match 1 of 3: print, hello" -Blockly.Msg.WORKSPACE_SEARCH_MATCH = 'Match %1 of %2: %3'; \ No newline at end of file +Blockly.Msg.WORKSPACE_SEARCH_MATCH = 'Match %1 of %2: %3'; +/** @type {string} */ +/// Placeholder text in the toolbox search category's input field. +Blockly.Msg.TOOLBOX_SEARCH_PLACEHOLDER = 'Search for blocks'; +/** @type {string} */ +/// Message shown in the toolbox search flyout before a query has been entered. +Blockly.Msg.TOOLBOX_SEARCH_PROMPT = 'Type to search for blocks'; +/** @type {string} */ +/// Message shown in the toolbox search flyout when a query matches no blocks. +Blockly.Msg.TOOLBOX_SEARCH_NO_RESULTS = 'No matching blocks found'; +/** @type {string} */ +/// Screen reader announcement when a toolbox search matches exactly one block. +Blockly.Msg.TOOLBOX_SEARCH_RESULT_COUNT_ONE = '1 matching block'; +/** @type {string} */ +/// Screen reader announcement of how many blocks a toolbox search matched. +/// \n\nParameters:\n* %1 - a number of blocks, which may be zero or greater than one. +Blockly.Msg.TOOLBOX_SEARCH_RESULT_COUNT = '%1 matching blocks'; \ No newline at end of file diff --git a/packages/plugins/toolbox-search/src/block_searcher.ts b/packages/plugins/toolbox-search/src/block_searcher.ts index 005974b730b..b1cf9301078 100644 --- a/packages/plugins/toolbox-search/src/block_searcher.ts +++ b/packages/plugins/toolbox-search/src/block_searcher.ts @@ -6,6 +6,13 @@ import * as Blockly from 'blockly/core'; +interface DropdownOption { + fieldName: string; + label: string; + value: string; + selected: boolean; +} + /** * A class that provides methods for indexing and searching blocks. */ @@ -15,6 +22,23 @@ export class BlockSearcher { Set >(); + // A map of blocks to the text that was indexed for them, used to filter + // the results of a search to only those blocks that contain the search term. + private blockText = new Map(); + // A map of blocks to the options of their dropdown fields, used to generate + // variants of blocks with different dropdown values. + private dropdownOptions = new Map< + Blockly.utils.toolbox.BlockInfo, + DropdownOption[] + >(); + // A map of blocks to the names of their variable fields, used to generate + // variants of blocks with different variable values. + private variableFields = new Map(); + // All workspace variables, sorted by name, updated when blocks are indexed. + private workspaceVariables: Array< + Blockly.IVariableModel + > = []; + /** * Populates the cached map of trigrams to the blocks they correspond to. * @@ -24,21 +48,76 @@ export class BlockSearcher { * itself. * * @param blockInfos A list of blocks to index. + * @param workspace The workspace source of truth for variables. This is + * used to index variable names and update variable fields. */ - indexBlocks(blockInfos: Blockly.utils.toolbox.BlockInfo[]) { + indexBlocks( + blockInfos: Blockly.utils.toolbox.BlockInfo[], + workspace: Blockly.Workspace, + ) { + this.blockText.clear(); + this.dropdownOptions.clear(); + this.trigramsToBlocks.clear(); + this.variableFields.clear(); + this.workspaceVariables = workspace + .getVariableMap() + .getAllVariables() + .sort(Blockly.Variables.compareByName); + const blockCreationWorkspace = new Blockly.Workspace(); blockInfos.forEach((blockInfo) => { const type = blockInfo.type; if (!type || type === '') return; - const block = blockCreationWorkspace.newBlock(type); + blockCreationWorkspace.clear(); + this.workspaceVariables.forEach((variable) => + blockCreationWorkspace + .getVariableMap() + .createVariable(variable.getName(), variable.getType()), + ); + const block = Blockly.serialization.blocks.append( + blockInfo as Blockly.serialization.blocks.State, + blockCreationWorkspace, + ); this.indexBlockText(type.replaceAll('_', ' '), blockInfo); - block.inputList.forEach((input) => { - input.fieldRow.forEach((field) => { - this.indexDropdownOption(field, blockInfo); - this.indexBlockText(field.getText(), blockInfo); + const variableFieldNames: string[] = []; + + // Index the text of every field on the block and its descendants, and record + // the names of any variable fields for later use in generating variants. + block.getDescendants(false).forEach((descendantBlock) => { + descendantBlock.inputList.forEach((input) => { + input.fieldRow.forEach((field) => { + if (field instanceof Blockly.FieldVariable) { + this.indexBlockText(field.getText(), blockInfo); + // If the current variable is one of the workspace variables, record + // the field name for later use in generating variants. + if ( + descendantBlock === block && + field.name && + this.workspaceVariables.some( + (v) => v.getName() === field.getText(), + ) + ) { + variableFieldNames.push(field.name); + } + } else { + // Index the text of the dropdown option and the block. + this.indexDropdownOption(field, blockInfo); + this.indexBlockText(field.getText(), blockInfo); + } + }); }); }); + if (variableFieldNames.length) { + // Index all workspace variable names for the block, so that a search for any of them + // will return the block, and record the names of the variable fields for later use + // in generating variants. + this.variableFields.set(blockInfo, variableFieldNames); + this.workspaceVariables.forEach((variable) => { + this.indexBlockText(variable.getName(), blockInfo); + }); + } }); + blockCreationWorkspace.dispose(); } /** @@ -51,15 +130,49 @@ export class BlockSearcher { field: Blockly.Field, block: Blockly.utils.toolbox.BlockInfo, ) { - if (field instanceof Blockly.FieldDropdown) { - field.getOptions(true).forEach((option) => { - if (typeof option[0] === 'string') { - this.indexBlockText(option[0], block); - } else if ('alt' in option[0]) { - this.indexBlockText(option[0].alt, block); - } - }); + if (!(field instanceof Blockly.FieldDropdown)) { + return; } + field.getOptions(true).forEach(([label, value]) => { + const text = + typeof label === 'string' + ? label + : label && 'alt' in label + ? label.alt + : ''; + if (!text) return; + this.indexBlockText(text, block); + if (!field.name || typeof value !== 'string') return; + const options = this.dropdownOptions.get(block) ?? []; + options.push({ + fieldName: field.name, + label: text.toLowerCase(), + value, + selected: value === field.getValue(), + }); + this.dropdownOptions.set(block, options); + }); + } + + /** + * Returns a list of variants of the given block with different dropdown values + * + * @param info The block to vary. + * @param options The options whose labels matched the query. + * @returns One block per matching option. + */ + private createMatchingBlockVariants( + info: Blockly.utils.toolbox.BlockInfo, + options: Array<{fieldName: string; value: string; selected: boolean}>, + ): Blockly.utils.toolbox.BlockInfo[] { + if (!options.length) return [info]; + // One variant per matching option, each differing in a single field, so + // two matching dropdowns give two results rather than four. + return options.map((option) => + option.selected + ? info + : {...info, fields: {...info.fields, [option.fieldName]: option.value}}, + ); } /** @@ -69,7 +182,7 @@ export class BlockSearcher { * @returns A list of blocks matching the query. */ blockTypesMatching(query: string): Blockly.utils.toolbox.BlockInfo[] { - return [ + const candidates = [ ...this.generateTrigrams(query) .map((trigram) => { return ( @@ -82,6 +195,47 @@ export class BlockSearcher { }) .values(), ]; + + const searchTerm = query.toLowerCase(); + const matches = candidates.filter((block) => + this.blockText.get(block)?.some((text) => text.includes(searchTerm)), + ); + + const matchedVariables = this.workspaceVariables.filter((v) => + v.getName().toLowerCase().includes(searchTerm), + ); + // The flyout creates one getter per variable, and they all collapse onto + // the same block once bound, so results are keyed by content. + const results = new Map(); + for (const match of matches) { + const variableFieldNames = this.variableFields.get(match); + const bound = + variableFieldNames && matchedVariables.length + ? matchedVariables.map((variable) => ({ + ...match, + fields: { + ...match.fields, + ...Object.fromEntries( + variableFieldNames.map((name) => [ + name, + {name: variable.getName(), type: variable.getType()}, + ]), + ), + }, + })) + : [match]; + + const options = (this.dropdownOptions.get(match) ?? []).filter((option) => + option.label.includes(searchTerm), + ); + + for (const info of bound) { + for (const variant of this.createMatchingBlockVariants(info, options)) { + results.set(JSON.stringify(variant), variant); + } + } + } + return [...results.values()]; } /** @@ -92,6 +246,9 @@ export class BlockSearcher { * @param block The block to associate the trigrams with. */ private indexBlockText(text: string, block: Blockly.utils.toolbox.BlockInfo) { + const texts = this.blockText.get(block) ?? []; + texts.push(text.toLowerCase()); + this.blockText.set(block, texts); this.generateTrigrams(text).forEach((trigram) => { const blockSet = this.trigramsToBlocks.get(trigram) ?? @@ -116,7 +273,6 @@ export class BlockSearcher { for (let start = 0; start <= normalizedInput.length - 3; start++) { trigrams.push(normalizedInput.substring(start, start + 3)); } - return trigrams; } diff --git a/packages/plugins/toolbox-search/src/toolbox_search.ts b/packages/plugins/toolbox-search/src/toolbox_search.ts index 25e4ba43fe0..c64dd8a8e71 100644 --- a/packages/plugins/toolbox-search/src/toolbox_search.ts +++ b/packages/plugins/toolbox-search/src/toolbox_search.ts @@ -23,6 +23,9 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { private readonly SEARCH_INPUT_ID = 'toolbox-search-input'; private searchField?: HTMLInputElement; private blockSearcher = new BlockSearcher(); + private onChangeWrapper?: (event: Blockly.Events.Abstract) => void; + private indexedBlocks = ''; + private boundEvents: Blockly.browserEvents.Data[] = []; /** * Initializes a ToolboxSearchCategory. @@ -41,6 +44,8 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { super(categoryDef, parentToolbox, opt_parent); this.initBlockSearcher(); this.registerShortcut(); + this.onChangeWrapper = this.handleWorkspaceChange.bind(this); + this.workspace_.addChangeListener(this.onChangeWrapper); } /** @@ -53,44 +58,63 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { this.searchField = document.createElement('input'); this.searchField.id = this.SEARCH_INPUT_ID; this.searchField.type = 'search'; - this.searchField.placeholder = 'Search for blocks'; + this.searchField.placeholder = Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER']; this.workspace_.RTL ? (this.searchField.style.marginRight = '8px') : (this.searchField.style.marginLeft = '8px'); - this.searchField.addEventListener('keydown', (event) => { - if (event.key === 'ArrowUp' && this.searchField?.selectionStart === 0) { - const previous = this.parentToolbox_.getNavigator().getPreviousNode(); - if (previous) { - Blockly.getFocusManager().focusNode(previous); - } - return; - } else if ( - event.key === 'ArrowRight' && - this.searchField?.selectionStart === this.searchField?.value.length - ) { - const previous = this.parentToolbox_.getNavigator().getInNode(); - if (previous) { - Blockly.getFocusManager().focusNode(previous); - } - return; - } else if ( - event.key === 'ArrowDown' && - this.searchField?.selectionStart === this.searchField?.value.length - ) { - const next = this.parentToolbox_.getNavigator().getNextNode(); - if (next) { - Blockly.getFocusManager().focusNode(next); - } - return; - } else if (event.key === 'Escape' && this.searchField) { - if (this.searchField.value !== '') { - this.searchField.value = ''; - event.stopPropagation(); - } - } - - this.matchBlocks(); - }); + this.boundEvents.push( + Blockly.browserEvents.conditionalBind( + this.searchField, + 'keydown', + this, + (event: KeyboardEvent) => { + if ( + event.key === 'ArrowUp' && + this.searchField?.selectionStart === 0 + ) { + const previous = this.parentToolbox_ + .getNavigator() + .getPreviousNode(); + if (previous) { + Blockly.getFocusManager().focusNode(previous); + } + return; + } else if ( + event.key === 'ArrowRight' && + this.searchField?.selectionStart === this.searchField?.value.length + ) { + const previous = this.parentToolbox_.getNavigator().getInNode(); + if (previous) { + Blockly.getFocusManager().focusNode(previous); + } + return; + } else if ( + event.key === 'ArrowDown' && + this.searchField?.selectionStart === this.searchField?.value.length + ) { + const next = this.parentToolbox_.getNavigator().getNextNode(); + if (next) { + Blockly.getFocusManager().focusNode(next); + } + return; + } else if (event.key === 'Escape' && this.searchField) { + if (this.searchField.value !== '') { + this.searchField.value = ''; + event.stopPropagation(); + // Removes matches from the flyout after programmatically clearing the search field. + this.matchBlocks(); + } + } + }, + ), + // When the user types in the search field, update the flyout to show matching blocks. + Blockly.browserEvents.conditionalBind( + this.searchField, + 'input', + this, + () => this.matchBlocks(), + ), + ); this.rowContents_?.replaceChildren(this.searchField); return dom; } @@ -106,7 +130,7 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { private registerShortcut() { const shortcut = Blockly.ShortcutRegistry.registry.createSerializedKey( Blockly.utils.KeyCodes.B, - [Blockly.utils.KeyCodes.CTRL], + [Blockly.utils.KeyCodes.CTRL_CMD], ); Blockly.ShortcutRegistry.registry.register({ name: ToolboxSearchCategory.START_SEARCH_SHORTCUT, @@ -129,7 +153,20 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { schema: Blockly.utils.toolbox.ToolboxItemInfo, allBlocks: Set, ) { - if ('contents' in schema) { + if ('custom' in schema && schema.custom) { + const flyoutCallback = this.workspace_.getToolboxCategoryCallback( + schema.custom, + ); + if (!flyoutCallback) { + return; + } + const flyoutDef = flyoutCallback(this.workspace_); + Blockly.utils.toolbox + .convertFlyoutDefToJsonArray(flyoutDef) + .forEach((item) => { + this.getAvailableBlocks(item, allBlocks); + }); + } else if ('contents' in schema) { schema.contents.forEach((contents) => { this.getAvailableBlocks(contents, allBlocks); }); @@ -148,7 +185,7 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { this.workspace_.options.languageTree?.contents?.forEach((item) => this.getAvailableBlocks(item, availableBlocks), ); - this.blockSearcher.indexBlocks([...availableBlocks]); + this.blockSearcher.indexBlocks([...availableBlocks], this.workspace_); } /** See IFocusableNode.getFocusableElement. */ @@ -181,21 +218,32 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { : []; const newCount = this.flyoutItems_.length; - if (oldCount !== newCount) { - Blockly.utils.aria.announceDynamicAriaState( - `${newCount} matching blocks`, - ); - } if (!this.flyoutItems_.length) { this.flyoutItems_.push({ kind: 'label', text: query.length < 3 - ? 'Type to search for blocks' - : 'No matching blocks found', + ? Blockly.Msg['TOOLBOX_SEARCH_PROMPT'] + : Blockly.Msg['TOOLBOX_SEARCH_NO_RESULTS'], }); } + + if (this.parentToolbox_.getSelectedItem() !== this) { + return; + } + + if (oldCount !== newCount) { + Blockly.utils.aria.announceDynamicAriaState( + newCount === 1 + ? Blockly.Msg['TOOLBOX_SEARCH_RESULT_COUNT_ONE'] + : Blockly.Msg['TOOLBOX_SEARCH_RESULT_COUNT'].replace( + '%1', + String(newCount), + ), + ); + } + this.parentToolbox_.refreshSelection(); } @@ -204,10 +252,65 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { */ override dispose() { super.dispose(); + for (const event of this.boundEvents) { + Blockly.browserEvents.unbind(event); + } + this.boundEvents.length = 0; + if (this.onChangeWrapper) { + this.workspace_.removeChangeListener(this.onChangeWrapper); + this.onChangeWrapper = undefined; + } Blockly.ShortcutRegistry.registry.unregister( ToolboxSearchCategory.START_SEARCH_SHORTCUT, ); } + + /** + * Rebuilds the search index when the workspace changes in a way that alters + * what a dynamic toolbox category offers, such as creating or renaming a + * variable or a procedure. + * + * @param event The change that occurred on the workspace. + */ + private handleWorkspaceChange(event: Blockly.Events.Abstract) { + if (event.isUiEvent) return; + // This works off of the assumption that these events don't typically change + // what a dynamic category contains. Apps that need to rebuild the index + // manually can do so by firing a different workspace event. + if ( + event.type === Blockly.Events.BLOCK_MOVE || + event.type === Blockly.Events.BLOCK_FIELD_INTERMEDIATE_CHANGE || + event.type.startsWith('comment_') + ) { + return; + } + if (this.refreshBlockSearcher()) this.matchBlocks(); + } + + /** + * Rebuilds the BlockSearcher index if the available blocks have changed. + * + * @returns True if the index was rebuilt. + */ + private refreshBlockSearcher(): boolean { + const availableBlocks = new Set(); + this.workspace_.options.languageTree?.contents?.forEach((item) => + this.getAvailableBlocks(item, availableBlocks), + ); + + const blocks = [...availableBlocks]; + const snapshot = JSON.stringify([ + blocks, + this.workspace_ + .getVariableMap() + .getAllVariables() + .map((v) => v.getName()), + ]); + if (snapshot === this.indexedBlocks) return false; + this.indexedBlocks = snapshot; + this.blockSearcher.indexBlocks(blocks, this.workspace_); + return true; + } } // Make the clear button clickable in Safari. diff --git a/packages/plugins/toolbox-search/test/index.ts b/packages/plugins/toolbox-search/test/index.ts index 2d3068ad514..9519ddeb145 100644 --- a/packages/plugins/toolbox-search/test/index.ts +++ b/packages/plugins/toolbox-search/test/index.ts @@ -9,7 +9,7 @@ */ import * as Blockly from 'blockly'; -import {toolboxCategories, createPlayground} from '@blockly/dev-tools'; +import {createPlayground} from '@blockly/dev-tools'; import '../src/toolbox_search'; /** @@ -23,16 +23,185 @@ function createWorkspace( blocklyDiv: HTMLElement, options: Blockly.BlocklyOptions, ): Blockly.WorkspaceSvg { - return Blockly.inject(blocklyDiv, options); + const workspace = Blockly.inject(blocklyDiv, options); + workspace.getVariableMap().createVariable('alpha'); + workspace.getVariableMap().createVariable('beta'); + return workspace; } +const toolbox = { + kind: 'categoryToolbox', + contents: [ + { + kind: 'category', + name: 'Logic', + categorystyle: 'logic_category', + contents: [ + {kind: 'block', type: 'controls_if'}, + {kind: 'block', type: 'logic_compare', fields: {OP: 'EQ'}}, + {kind: 'block', type: 'logic_operation', fields: {OP: 'AND'}}, + {kind: 'block', type: 'logic_negate'}, + {kind: 'block', type: 'logic_boolean', fields: {BOOL: 'TRUE'}}, + ], + }, + { + kind: 'category', + name: 'Loops', + categorystyle: 'loop_category', + contents: [ + { + kind: 'block', + type: 'controls_repeat_ext', + inputs: {TIMES: {shadow: {type: 'math_number', fields: {NUM: 250}}}}, + }, + {kind: 'block', type: 'controls_whileUntil', fields: {MODE: 'WHILE'}}, + {kind: 'block', type: 'controls_forEach'}, + { + kind: 'block', + type: 'controls_flow_statements', + fields: {FLOW: 'BREAK'}, + }, + ], + }, + { + kind: 'category', + name: 'Math', + categorystyle: 'math_category', + contents: [ + {kind: 'block', type: 'math_number', fields: {NUM: 42}}, + { + kind: 'block', + type: 'math_arithmetic', + fields: {OP: 'ADD'}, + inputs: { + A: {shadow: {type: 'math_number', fields: {NUM: 1}}}, + B: {shadow: {type: 'math_number', fields: {NUM: 1}}}, + }, + }, + { + kind: 'block', + type: 'math_round', + fields: {OP: 'ROUND'}, + inputs: {NUM: {shadow: {type: 'math_number', fields: {NUM: 3.1}}}}, + }, + { + kind: 'block', + type: 'math_modulo', + inputs: { + DIVIDEND: {shadow: {type: 'math_number', fields: {NUM: 64}}}, + DIVISOR: {shadow: {type: 'math_number', fields: {NUM: 10}}}, + }, + }, + ], + }, + { + kind: 'category', + name: 'Text', + categorystyle: 'text_category', + contents: [ + {kind: 'block', type: 'text', fields: {TEXT: 'abracadabra'}}, + { + kind: 'block', + type: 'text_print', + inputs: { + TEXT: {shadow: {type: 'text', fields: {TEXT: 'hello world'}}}, + }, + }, + {kind: 'block', type: 'text_join'}, + { + kind: 'block', + type: 'text_length', + inputs: {VALUE: {shadow: {type: 'text', fields: {TEXT: 'abc'}}}}, + }, + { + kind: 'block', + type: 'text_changeCase', + fields: {CASE: 'UPPERCASE'}, + inputs: {TEXT: {shadow: {type: 'text', fields: {TEXT: 'abc'}}}}, + }, + { + kind: 'block', + type: 'text_append', + inputs: {TEXT: {shadow: {type: 'text', fields: {TEXT: '!'}}}}, + }, + ], + }, + { + kind: 'category', + name: 'Lists', + categorystyle: 'list_category', + contents: [ + {kind: 'block', type: 'lists_create_with'}, + { + kind: 'block', + type: 'lists_sort', + fields: {TYPE: 'NUMERIC', DIRECTION: '1'}, + }, + { + kind: 'block', + type: 'lists_split', + fields: {MODE: 'SPLIT'}, + inputs: {DELIM: {shadow: {type: 'text', fields: {TEXT: ','}}}}, + }, + { + kind: 'block', + type: 'lists_getIndex', + fields: {MODE: 'GET', WHERE: 'FROM_START'}, + }, + { + kind: 'block', + type: 'lists_getSublist', + fields: {WHERE1: 'FROM_START', WHERE2: 'FROM_START'}, + }, + ], + }, + {kind: 'sep'}, + { + kind: 'category', + name: 'Variables', + categorystyle: 'variable_category', + custom: 'VARIABLE', + }, + { + kind: 'category', + name: 'Functions', + categorystyle: 'procedure_category', + custom: 'PROCEDURE', + }, + {kind: 'sep'}, + { + kind: 'category', + name: 'Snippets', + categorystyle: 'logic_category', + contents: [ + { + kind: 'block', + type: 'text_print', + inputs: {TEXT: {block: {type: 'variables_get'}}}, + }, + { + kind: 'block', + type: 'controls_if', + inputs: { + IF0: { + block: { + type: 'logic_compare', + fields: {OP: 'GT'}, + inputs: { + A: {block: {type: 'variables_get'}}, + B: {shadow: {type: 'math_number', fields: {NUM: 100}}}, + }, + }, + }, + }, + }, + ], + }, + {kind: 'search', name: 'Search', contents: []}, + ], +}; + document.addEventListener('DOMContentLoaded', function () { - const toolbox = {...toolboxCategories}; - toolbox['contents'].push({ - kind: 'search', - name: 'Search', - contents: [], - }); const defaultOptions: Blockly.BlocklyOptions = { toolbox, }; diff --git a/packages/plugins/toolbox-search/test/tests.mocha.js b/packages/plugins/toolbox-search/test/tests.mocha.js index 950eaa12fba..90044c681d3 100644 --- a/packages/plugins/toolbox-search/test/tests.mocha.js +++ b/packages/plugins/toolbox-search/test/tests.mocha.js @@ -15,6 +15,16 @@ suite('Toolbox search', () => { }); suite('BlockSearcher', () => { + let workspace; + + setup(() => { + workspace = new Blockly.Workspace(); + }); + + teardown(() => { + workspace.dispose(); + }); + test('generateTrigrams handles empty and short input', () => { const searcher = new BlockSearcher(); const generateTrigrams = searcher.generateTrigrams.bind(searcher); @@ -39,7 +49,7 @@ suite('BlockSearcher', () => { // Text on these: // lists_sort: sort // lists_split: make with delimiter , - searcher.indexBlocks(blocks); + searcher.indexBlocks(blocks, workspace); const numericMatches = searcher.blockTypesMatching('numeric'); assert.sameMembers(numericMatches, [blocks[0]]); @@ -54,7 +64,7 @@ suite('BlockSearcher', () => { kind: 'block', type: 'lists_create_with', }; - searcher.indexBlocks([listCreateWithBlock]); + searcher.indexBlocks([listCreateWithBlock], workspace); const lowercaseMatches = searcher.blockTypesMatching('create list'); assert.sameMembers(lowercaseMatches, [listCreateWithBlock]); @@ -72,7 +82,7 @@ suite('BlockSearcher', () => { kind: 'block', type: 'math_constrain', }; - searcher.indexBlocks([mathConstrainBlock]); + searcher.indexBlocks([mathConstrainBlock], workspace); const matches = searcher.blockTypesMatching('conso'); @@ -98,7 +108,7 @@ suite('BlockSearcher', () => { kind: 'block', type: 'searcher_underscore_block', }; - searcher.indexBlocks([blockInfo]); + searcher.indexBlocks([blockInfo], workspace); assert.sameMembers( searcher.blockTypesMatching('searcher underscore block'), @@ -125,7 +135,7 @@ suite('BlockSearcher', () => { const blockA = {kind: 'block', type: 'searcher_charlie'}; const blockB = {kind: 'block', type: 'searcher_delta'}; - searcher.indexBlocks([blockA, blockB]); + searcher.indexBlocks([blockA, blockB], workspace); const broadQueryMatches = searcher.blockTypesMatching('alpha bravo'); assert.sameMembers(broadQueryMatches, [blockA, blockB]); @@ -174,10 +184,16 @@ suite('BlockSearcher', () => { const searcher = new BlockSearcher(); const blockInfo = {kind: 'block', type: 'searcher_dropdown_alt'}; - searcher.indexBlocks([blockInfo]); + searcher.indexBlocks([blockInfo], workspace); assert.sameMembers(searcher.blockTypesMatching('sunny'), [blockInfo]); - assert.sameMembers(searcher.blockTypesMatching('cloudy'), [blockInfo]); + // 'cloudy' wasn't the selected option, but it should be set with the matching option if found. + const cloudyMatches = searcher.blockTypesMatching('cloudy'); + assert.lengthOf(cloudyMatches, 1); + assert.deepEqual(cloudyMatches[0], { + ...blockInfo, + fields: {WEATHER: 'CLOUD'}, + }); }); test('returns an empty list when no matches are found', () => { @@ -211,7 +227,7 @@ suite('BlockSearcher', () => { }, ]; - searcher.indexBlocks(blocks); + searcher.indexBlocks(blocks, workspace); const matches = searcher.blockTypesMatching('replace'); assert.sameMembers(matches, [blocks[0]]); From 6081dcf1542bdb55b897dcc34b5f0f493161c5dc Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:46:40 -0400 Subject: [PATCH 2/7] chore: add tests --- packages/plugins/toolbox-search/package.json | 3 + .../toolbox-search/test/tests.mocha.js | 238 ++++++++++++++++++ 2 files changed, 241 insertions(+) diff --git a/packages/plugins/toolbox-search/package.json b/packages/plugins/toolbox-search/package.json index 88e551896d3..3d703e92c05 100644 --- a/packages/plugins/toolbox-search/package.json +++ b/packages/plugins/toolbox-search/package.json @@ -43,6 +43,9 @@ "@blockly/dev-scripts": "^13.1.0", "@blockly/dev-tools": "^13.1.0", "chai": "^6.2.2", + "jsdom": "^30.0.1", + "jsdom-global": "^3.0.2", + "sinon": "^22.1.0", "typescript": "^6.0.3" }, "peerDependencies": { diff --git a/packages/plugins/toolbox-search/test/tests.mocha.js b/packages/plugins/toolbox-search/test/tests.mocha.js index 90044c681d3..1f69b73b35a 100644 --- a/packages/plugins/toolbox-search/test/tests.mocha.js +++ b/packages/plugins/toolbox-search/test/tests.mocha.js @@ -1,5 +1,6 @@ import {assert} from 'chai'; import * as Blockly from 'blockly'; +import * as sinon from 'sinon'; import {ToolboxSearchCategory} from '../src/toolbox_search'; import {BlockSearcher} from '../src/block_searcher'; @@ -14,6 +15,106 @@ suite('Toolbox search', () => { }); }); +suite('ToolboxSearchCategory', () => { + /** + * @param {!Blockly.WorkspaceSvg} workspace The workspace to inspect. + * @returns {!Array} The types of the blocks now in the flyout. + */ + function flyoutBlockTypes(workspace) { + return workspace + .getFlyout() + .getWorkspace() + .getTopBlocks(false) + .map((block) => block.type); + } + + setup(function () { + this.jsdomCleanup = require('jsdom-global')( + '
', + ); + this.clock = sinon.useFakeTimers(); + this.workspace = Blockly.inject('blocklyDiv', { + media: 'media/', + toolbox: { + kind: 'categoryToolbox', + contents: [ + { + kind: 'category', + name: 'Logic', + contents: [{kind: 'block', type: 'controls_if'}], + }, + {kind: 'category', name: 'Variables', custom: 'VARIABLE'}, + {kind: 'search', name: 'Search', contents: []}, + ], + }, + }); + // See https://github.com/RaspberryPiFoundation/blockly-samples/issues/2528. + global.SVGElement = window.SVGElement; + global.requestAnimationFrame = (callback) => setTimeout(callback, 0); + this.searchCategory = this.workspace + .getToolbox() + .getToolboxItems() + .find((item) => item instanceof ToolboxSearchCategory); + + /** + * Types into the search field the way a user would, so the category's own + * input listener drives the flyout. + * @param {string} query The text to search for. + */ + this.search = (query) => { + const field = this.searchCategory.searchField; + this.searchCategory + .getParentToolbox() + .setSelectedItem(this.searchCategory); + field.value = query; + field.dispatchEvent(new window.Event('input')); + this.clock.runAll(); + }; + }); + + teardown(function () { + this.workspace.dispose(); + this.clock.runAll(); + this.clock.restore(); + this.jsdomCleanup(); + }); + + test('shows matching blocks in the flyout', function () { + this.search('controls if'); + + assert.deepEqual(flyoutBlockTypes(this.workspace), ['controls_if']); + }); + + test('updates flyout blocks to the variable that was searched for', function () { + this.workspace.getVariableMap().createVariable('score'); + this.clock.runAll(); + + this.search('score'); + + const blocks = this.workspace + .getFlyout() + .getWorkspace() + .getTopBlocks(false); + assert.deepEqual( + blocks.map((block) => block.type), + ['variables_set', 'math_change', 'variables_get'], + ); + blocks.forEach((block) => { + assert.equal(block.getVarModels()[0].getName(), 'score'); + }); + }); + + test('keeps the flyout current as variables are created', function () { + this.search('score'); + assert.isEmpty(flyoutBlockTypes(this.workspace)); + + this.workspace.getVariableMap().createVariable('score'); + this.clock.runAll(); + + assert.include(flyoutBlockTypes(this.workspace), 'variables_get'); + }); +}); + suite('BlockSearcher', () => { let workspace; @@ -25,6 +126,21 @@ suite('BlockSearcher', () => { workspace.dispose(); }); + /** + * Creates the named variables on the workspace and returns the block + * definitions the VARIABLE category would create for them. + * @param {!Array} names The variables to create. + * @returns {!Array} A setter, a change block and one getter each. + */ + function createVariableBlocks(names) { + names.forEach((name) => workspace.getVariableMap().createVariable(name)); + return Blockly.Variables.jsonFlyoutCategoryBlocks( + workspace, + workspace.getVariableMap().getVariablesOfType(''), + true, + ); + } + test('generateTrigrams handles empty and short input', () => { const searcher = new BlockSearcher(); const generateTrigrams = searcher.generateTrigrams.bind(searcher); @@ -232,4 +348,126 @@ suite('BlockSearcher', () => { const matches = searcher.blockTypesMatching('replace'); assert.sameMembers(matches, [blocks[0]]); }); + + test('indexes field values from the block definition', () => { + const searcher = new BlockSearcher(); + const numberBlock = { + kind: 'block', + type: 'math_number', + fields: {NUM: 250}, + }; + const printBlock = { + kind: 'block', + type: 'text_print', + inputs: {TEXT: {shadow: {type: 'text', fields: {TEXT: 'abc'}}}}, + }; + searcher.indexBlocks([numberBlock, printBlock], workspace); + + assert.sameMembers(searcher.blockTypesMatching('250'), [numberBlock]); + // The value lives on a shadow block, not on the block itself. + assert.sameMembers(searcher.blockTypesMatching('abc'), [printBlock]); + }); + + test('binds variable blocks to every matching variable', () => { + const searcher = new BlockSearcher(); + searcher.indexBlocks( + createVariableBlocks(['alpha', 'alphabet', 'beta']), + workspace, + ); + + const matches = searcher.blockTypesMatching('alpha'); + assert.sameMembers( + matches.map((match) => `${match.type}(${match.fields.VAR.name})`), + [ + 'variables_set(alpha)', + 'variables_set(alphabet)', + 'math_change(alpha)', + 'math_change(alphabet)', + 'variables_get(alpha)', + 'variables_get(alphabet)', + ], + ); + }); + + test('leaves matches unchanged when the query doesn\'t name a variable', () => { + const searcher = new BlockSearcher(); + const blocks = createVariableBlocks(['alpha']); + searcher.indexBlocks(blocks, workspace); + + const setter = blocks.find((block) => block.type === 'variables_set'); + assert.include(searcher.blockTypesMatching('set'), setter); + }); + + test('does not index variable rename and delete options', () => { + const searcher = new BlockSearcher(); + searcher.indexBlocks(createVariableBlocks(['alpha']), workspace); + + assert.isEmpty(searcher.blockTypesMatching('rename')); + assert.isEmpty(searcher.blockTypesMatching('delete the')); + }); + + test('sets dropdowns to the option that matched', () => { + const searcher = new BlockSearcher(); + const sortBlock = {kind: 'block', type: 'lists_sort'}; + searcher.indexBlocks([sortBlock], workspace); + + // 'numeric' is already selected, so the indexed block is returned as-is. + assert.sameMembers(searcher.blockTypesMatching('numeric'), [sortBlock]); + assert.sameDeepMembers( + searcher.blockTypesMatching('alphabetic').map((match) => match.fields), + [{TYPE: 'TEXT'}, {TYPE: 'IGNORE_CASE'}], + ); + }); + + test('varies one dropdown at a time', () => { + const searcher = new BlockSearcher(); + // Both WHERE1 and WHERE2 offer '# from end'. + searcher.indexBlocks( + [{kind: 'block', type: 'lists_getSublist'}], + workspace, + ); + + assert.sameDeepMembers( + searcher.blockTypesMatching('from end').map((match) => match.fields), + [{WHERE1: 'FROM_END'}, {WHERE2: 'FROM_END'}], + ); + }); + + test('indexes procedure names from extra state', () => { + const searcher = new BlockSearcher(); + const callBlock = { + kind: 'block', + type: 'procedures_callnoreturn', + extraState: {name: 'draw sprites', params: []}, + }; + searcher.indexBlocks([callBlock], workspace); + + assert.sameMembers(searcher.blockTypesMatching('draw sprites'), [ + callBlock, + ]); + }); + + test('does not match trigrams pooled from different strings', () => { + const searcher = new BlockSearcher(); + // 'controls flow statements' supplies 'tem' and 'next iteration' supplies + // 'ite', but the block never says 'item'. + searcher.indexBlocks( + [{kind: 'block', type: 'controls_flow_statements'}], + workspace, + ); + + assert.isEmpty(searcher.blockTypesMatching('item')); + }); + + test('replaces the previous index when reindexing', () => { + const searcher = new BlockSearcher(); + searcher.indexBlocks([{kind: 'block', type: 'text_print'}], workspace); + + // Reindexing should forget the previous pass entirely, not add to it. + const negate = {kind: 'block', type: 'logic_negate'}; + searcher.indexBlocks([negate], workspace); + + assert.isEmpty(searcher.blockTypesMatching('print')); + assert.sameMembers(searcher.blockTypesMatching('not'), [negate]); + }); }); From fa37e765f1ba7eb71946c1d35eca8d5a43d55270 Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:53:53 -0400 Subject: [PATCH 3/7] chore: formatting --- packages/plugins/toolbox-search/test/tests.mocha.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/toolbox-search/test/tests.mocha.js b/packages/plugins/toolbox-search/test/tests.mocha.js index 1f69b73b35a..1a6a0d2cf76 100644 --- a/packages/plugins/toolbox-search/test/tests.mocha.js +++ b/packages/plugins/toolbox-search/test/tests.mocha.js @@ -389,7 +389,7 @@ suite('BlockSearcher', () => { ); }); - test('leaves matches unchanged when the query doesn\'t name a variable', () => { + test("leaves matches unchanged when the query doesn't name a variable", () => { const searcher = new BlockSearcher(); const blocks = createVariableBlocks(['alpha']); searcher.indexBlocks(blocks, workspace); From 185b2a8ddaaaaf1325f950869a4e62cf102f60c5 Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Wed, 23 Sep 2026 14:49:13 -0400 Subject: [PATCH 4/7] chore: code review --- .../toolbox-search/src/block_searcher.ts | 265 +++++++++++------- .../toolbox-search/src/toolbox_search.ts | 7 +- packages/plugins/toolbox-search/test/index.ts | 42 +++ .../toolbox-search/test/tests.mocha.js | 162 +++++++---- 4 files changed, 318 insertions(+), 158 deletions(-) diff --git a/packages/plugins/toolbox-search/src/block_searcher.ts b/packages/plugins/toolbox-search/src/block_searcher.ts index b1cf9301078..e7448c2ef91 100644 --- a/packages/plugins/toolbox-search/src/block_searcher.ts +++ b/packages/plugins/toolbox-search/src/block_searcher.ts @@ -6,13 +6,24 @@ import * as Blockly from 'blockly/core'; -interface DropdownOption { +/** A value a field could be rewritten to, in order to make a query visible. */ +interface FieldCandidate { + // The id, in the saved state, of the block owning the field. + blockId: string; fieldName: string; + // Lowercase text that would be visible if this value were set. label: string; - value: string; + // The value to write into the block's serialized fields, either an option value + // for dropdown options, or an object with name and type for variable fields. + value: string | {name: string; type: string}; + // Whether the field already holds this value. selected: boolean; } +/** A block in a toolbox definition, or in a serialized block state. */ +type BlockNode = + Blockly.utils.toolbox.BlockInfo | Blockly.serialization.blocks.State; + /** * A class that provides methods for indexing and searching blocks. */ @@ -25,20 +36,25 @@ export class BlockSearcher { // A map of blocks to the text that was indexed for them, used to filter // the results of a search to only those blocks that contain the search term. private blockText = new Map(); - // A map of blocks to the options of their dropdown fields, used to generate - // variants of blocks with different dropdown values. - private dropdownOptions = new Map< + // A map of blocks to the field values they could be rewritten to, used to + // show a block with the searched text visible on it. + private fieldCandidates = new Map< + Blockly.utils.toolbox.BlockInfo, + FieldCandidate[] + >(); + // The workspace whose variables searches are matched against. + private workspace: Blockly.Workspace; + // A map of blocks to the serialized state of the block built while indexing. + // Rewrites go through this rather than the toolbox definition, because it + // carries an id for every block in the tree and the definition does not. + private blockStates = new Map< Blockly.utils.toolbox.BlockInfo, - DropdownOption[] + Blockly.utils.toolbox.BlockInfo >(); - // A map of blocks to the names of their variable fields, used to generate - // variants of blocks with different variable values. - private variableFields = new Map(); - // All workspace variables, sorted by name, updated when blocks are indexed. - private workspaceVariables: Array< - Blockly.IVariableModel - > = []; + constructor(workspace: Blockly.Workspace) { + this.workspace = workspace; + } /** * Populates the cached map of trigrams to the blocks they correspond to. * @@ -48,28 +64,23 @@ export class BlockSearcher { * itself. * * @param blockInfos A list of blocks to index. - * @param workspace The workspace source of truth for variables. This is - * used to index variable names and update variable fields. */ - indexBlocks( - blockInfos: Blockly.utils.toolbox.BlockInfo[], - workspace: Blockly.Workspace, - ) { + indexBlocks(blockInfos: Blockly.utils.toolbox.BlockInfo[]) { this.blockText.clear(); - this.dropdownOptions.clear(); + this.fieldCandidates.clear(); this.trigramsToBlocks.clear(); - this.variableFields.clear(); - this.workspaceVariables = workspace + this.blockStates.clear(); + const workspaceVariables = this.workspace .getVariableMap() .getAllVariables() .sort(Blockly.Variables.compareByName); - + Blockly.Events.disable(); const blockCreationWorkspace = new Blockly.Workspace(); blockInfos.forEach((blockInfo) => { const type = blockInfo.type; if (!type || type === '') return; blockCreationWorkspace.clear(); - this.workspaceVariables.forEach((variable) => + workspaceVariables.forEach((variable) => blockCreationWorkspace .getVariableMap() .createVariable(variable.getName(), variable.getType()), @@ -78,56 +89,50 @@ export class BlockSearcher { blockInfo as Blockly.serialization.blocks.State, blockCreationWorkspace, ); + const state = Blockly.serialization.blocks.save(block); + if (state) + // Merged with the original block info to preserve the kind and other props. + this.blockStates.set(blockInfo, {...blockInfo, ...state}); this.indexBlockText(type.replaceAll('_', ' '), blockInfo); - const variableFieldNames: string[] = []; - - // Index the text of every field on the block and its descendants, and record - // the names of any variable fields for later use in generating variants. + // Index the text of every field on the block and its descendants, and + // record the values each field could be rewritten to. block.getDescendants(false).forEach((descendantBlock) => { descendantBlock.inputList.forEach((input) => { input.fieldRow.forEach((field) => { + this.indexBlockText(field.getText(), blockInfo); if (field instanceof Blockly.FieldVariable) { - this.indexBlockText(field.getText(), blockInfo); - // If the current variable is one of the workspace variables, record - // the field name for later use in generating variants. - if ( - descendantBlock === block && - field.name && - this.workspaceVariables.some( - (v) => v.getName() === field.getText(), - ) - ) { - variableFieldNames.push(field.name); - } + this.indexVariableCandidates( + field, + descendantBlock.id, + blockInfo, + workspaceVariables, + ); } else { - // Index the text of the dropdown option and the block. - this.indexDropdownOption(field, blockInfo); - this.indexBlockText(field.getText(), blockInfo); + this.indexDropdownCandidates( + field, + descendantBlock.id, + blockInfo, + ); } }); }); }); - if (variableFieldNames.length) { - // Index all workspace variable names for the block, so that a search for any of them - // will return the block, and record the names of the variable fields for later use - // in generating variants. - this.variableFields.set(blockInfo, variableFieldNames); - this.workspaceVariables.forEach((variable) => { - this.indexBlockText(variable.getName(), blockInfo); - }); - } }); blockCreationWorkspace.dispose(); + Blockly.Events.enable(); } /** - * Check if the field is a dropdown, and index every text in the option + * Indexes a dropdown field's option labels and records each option + * as a value the field could be rewritten to. * - * @param field We need to check the type of field + * @param field The field to index. + * @param blockId The id of the block containing the field. * @param block The block to associate the trigrams with. */ - private indexDropdownOption( + private indexDropdownCandidates( field: Blockly.Field, + blockId: string, block: Blockly.utils.toolbox.BlockInfo, ) { if (!(field instanceof Blockly.FieldDropdown)) { @@ -143,36 +148,60 @@ export class BlockSearcher { if (!text) return; this.indexBlockText(text, block); if (!field.name || typeof value !== 'string') return; - const options = this.dropdownOptions.get(block) ?? []; - options.push({ + this.addCandidate(block, { + blockId, fieldName: field.name, label: text.toLowerCase(), value, selected: value === field.getValue(), }); - this.dropdownOptions.set(block, options); }); } /** - * Returns a list of variants of the given block with different dropdown values + * Indexes every workspace variable name against the block and records each + * as a value the field could be rewritten to. * - * @param info The block to vary. - * @param options The options whose labels matched the query. - * @returns One block per matching option. + * A field set to a variable that exists only in the flyout is left alone. + * + * @param field The variable field to index. + * @param blockId The id of the block containing the field. + * @param block The block to associate the trigrams with. + * @param variables The workspace's variables, sorted by name. */ - private createMatchingBlockVariants( - info: Blockly.utils.toolbox.BlockInfo, - options: Array<{fieldName: string; value: string; selected: boolean}>, - ): Blockly.utils.toolbox.BlockInfo[] { - if (!options.length) return [info]; - // One variant per matching option, each differing in a single field, so - // two matching dropdowns give two results rather than four. - return options.map((option) => - option.selected - ? info - : {...info, fields: {...info.fields, [option.fieldName]: option.value}}, - ); + private indexVariableCandidates( + field: Blockly.FieldVariable, + blockId: string, + block: Blockly.utils.toolbox.BlockInfo, + variables: Array>, + ) { + const current = field.getText(); + if (!field.name || !variables.some((v) => v.getName() === current)) return; + variables.forEach((variable) => { + this.indexBlockText(variable.getName(), block); + this.addCandidate(block, { + blockId, + fieldName: field.name as string, + label: variable.getName().toLowerCase(), + value: {name: variable.getName(), type: variable.getType()}, + selected: variable.getName() === current, + }); + }); + } + + /** + * Records a value a field could be rewritten to. + * + * @param block The block the field belongs to. + * @param candidate The value and the text it would make visible. + */ + private addCandidate( + block: Blockly.utils.toolbox.BlockInfo, + candidate: FieldCandidate, + ) { + const candidates = this.fieldCandidates.get(block) ?? []; + candidates.push(candidate); + this.fieldCandidates.set(block, candidates); } /** @@ -200,39 +229,32 @@ export class BlockSearcher { const matches = candidates.filter((block) => this.blockText.get(block)?.some((text) => text.includes(searchTerm)), ); - - const matchedVariables = this.workspaceVariables.filter((v) => - v.getName().toLowerCase().includes(searchTerm), - ); // The flyout creates one getter per variable, and they all collapse onto // the same block once bound, so results are keyed by content. const results = new Map(); for (const match of matches) { - const variableFieldNames = this.variableFields.get(match); - const bound = - variableFieldNames && matchedVariables.length - ? matchedVariables.map((variable) => ({ - ...match, - fields: { - ...match.fields, - ...Object.fromEntries( - variableFieldNames.map((name) => [ - name, - {name: variable.getName(), type: variable.getType()}, - ]), - ), - }, - })) - : [match]; - - const options = (this.dropdownOptions.get(match) ?? []).filter((option) => - option.label.includes(searchTerm), + const candidates = (this.fieldCandidates.get(match) ?? []).filter( + (candidate) => candidate.label.includes(searchTerm), ); - - for (const info of bound) { - for (const variant of this.createMatchingBlockVariants(info, options)) { - results.set(JSON.stringify(variant), variant); - } + // If the value is already showing, the block doesn't need to be updated. + const hasUnselectedCandidates = + candidates.length > 0 && + !candidates.some((candidate) => candidate.selected); + const variants = hasUnselectedCandidates + ? candidates.map((candidate) => + this.updateBlockField( + this.blockStates.get(match) ?? match, + candidate, + ), + ) + : [match]; + for (const variant of variants) { + // Ignore the id when comparing, so that we don't end up with two + // copies of the same block. + const key = JSON.stringify(variant, (property, value) => + property === 'id' ? undefined : value, + ); + results.set(key, hasUnselectedCandidates ? JSON.parse(key) : variant); } } return [...results.values()]; @@ -289,4 +311,43 @@ export class BlockSearcher { ): Set { return new Set([...a].filter((value) => b.has(value))); } + + /** + * Returns the block with the given id, searched depth-first. + * + * @param node The block to search, along with its descendants. + * @param id The block id to find. + * @returns The matching block, or null if it isn't there. + */ + private getNodeById(node: BlockNode, id: string): BlockNode | null { + if (node.id === id) return node; + for (const connection of [...Object.values(node.inputs ?? {}), node.next]) { + for (const childBlock of [connection?.block, connection?.shadow]) { + const found = childBlock && this.getNodeById(childBlock, id); + if (found) return found; + } + } + return null; + } + + /** + * Returns a copy of a block's state with one of its fields set. + * + * @param info The state to copy. + * @param candidate The field to set, which may be on any block in the tree, + * and the value to set it to. + * @returns A copy with the field set, or info itself if the block is gone. + */ + private updateBlockField( + info: Blockly.utils.toolbox.BlockInfo, + candidate: FieldCandidate, + ): Blockly.utils.toolbox.BlockInfo { + const copy = JSON.parse( + JSON.stringify(info), + ) as Blockly.utils.toolbox.BlockInfo; + const target = this.getNodeById(copy, candidate.blockId); + if (!target) return info; + target.fields = {...target.fields, [candidate.fieldName]: candidate.value}; + return copy; + } } diff --git a/packages/plugins/toolbox-search/src/toolbox_search.ts b/packages/plugins/toolbox-search/src/toolbox_search.ts index c64dd8a8e71..9b699041e57 100644 --- a/packages/plugins/toolbox-search/src/toolbox_search.ts +++ b/packages/plugins/toolbox-search/src/toolbox_search.ts @@ -22,7 +22,7 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { static readonly SEARCH_CATEGORY_KIND = 'search'; private readonly SEARCH_INPUT_ID = 'toolbox-search-input'; private searchField?: HTMLInputElement; - private blockSearcher = new BlockSearcher(); + private blockSearcher: BlockSearcher; private onChangeWrapper?: (event: Blockly.Events.Abstract) => void; private indexedBlocks = ''; private boundEvents: Blockly.browserEvents.Data[] = []; @@ -42,6 +42,7 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { opt_parent?: Blockly.ICollapsibleToolboxItem, ) { super(categoryDef, parentToolbox, opt_parent); + this.blockSearcher = new BlockSearcher(this.workspace_); this.initBlockSearcher(); this.registerShortcut(); this.onChangeWrapper = this.handleWorkspaceChange.bind(this); @@ -185,7 +186,7 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { this.workspace_.options.languageTree?.contents?.forEach((item) => this.getAvailableBlocks(item, availableBlocks), ); - this.blockSearcher.indexBlocks([...availableBlocks], this.workspace_); + this.blockSearcher.indexBlocks([...availableBlocks]); } /** See IFocusableNode.getFocusableElement. */ @@ -308,7 +309,7 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { ]); if (snapshot === this.indexedBlocks) return false; this.indexedBlocks = snapshot; - this.blockSearcher.indexBlocks(blocks, this.workspace_); + this.blockSearcher.indexBlocks(blocks); return true; } } diff --git a/packages/plugins/toolbox-search/test/index.ts b/packages/plugins/toolbox-search/test/index.ts index 9519ddeb145..fcff7dfa590 100644 --- a/packages/plugins/toolbox-search/test/index.ts +++ b/packages/plugins/toolbox-search/test/index.ts @@ -195,6 +195,48 @@ const toolbox = { }, }, }, + { + kind: 'block', + type: 'text_print', + inputs: { + TEXT: { + block: { + type: 'lists_getSublist', + fields: {WHERE1: 'FROM_END'}, + inputs: { + LIST: { + block: { + type: 'variables_get', + fields: {VAR: {name: 'beta'}}, + }, + }, + AT1: {shadow: {type: 'math_number'}}, + AT2: {shadow: {type: 'math_number', fields: {NUM: 3}}}, + }, + }, + }, + }, + }, + { + kind: 'block', + type: 'text_print', + inputs: { + TEXT: { + block: { + type: 'lists_sort', + fields: {TYPE: 'NUMERIC', DIRECTION: '1'}, + inputs: { + LIST: { + block: { + type: 'variables_get', + fields: {VAR: {name: 'beta'}}, + }, + }, + }, + }, + }, + }, + }, ], }, {kind: 'search', name: 'Search', contents: []}, diff --git a/packages/plugins/toolbox-search/test/tests.mocha.js b/packages/plugins/toolbox-search/test/tests.mocha.js index 1a6a0d2cf76..5dfd8f224de 100644 --- a/packages/plugins/toolbox-search/test/tests.mocha.js +++ b/packages/plugins/toolbox-search/test/tests.mocha.js @@ -142,7 +142,7 @@ suite('BlockSearcher', () => { } test('generateTrigrams handles empty and short input', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const generateTrigrams = searcher.generateTrigrams.bind(searcher); assert.deepEqual(generateTrigrams(''), []); @@ -151,7 +151,7 @@ suite('BlockSearcher', () => { }); test('indexes the default value of dropdown fields', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const blocks = [ { kind: 'block', @@ -165,7 +165,7 @@ suite('BlockSearcher', () => { // Text on these: // lists_sort: sort // lists_split: make with delimiter , - searcher.indexBlocks(blocks, workspace); + searcher.indexBlocks(blocks); const numericMatches = searcher.blockTypesMatching('numeric'); assert.sameMembers(numericMatches, [blocks[0]]); @@ -175,12 +175,12 @@ suite('BlockSearcher', () => { }); test('is not case-sensitive', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const listCreateWithBlock = { kind: 'block', type: 'lists_create_with', }; - searcher.indexBlocks([listCreateWithBlock], workspace); + searcher.indexBlocks([listCreateWithBlock]); const lowercaseMatches = searcher.blockTypesMatching('create list'); assert.sameMembers(lowercaseMatches, [listCreateWithBlock]); @@ -193,12 +193,12 @@ suite('BlockSearcher', () => { }); test('requires the final trigram when matching longer queries', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const mathConstrainBlock = { kind: 'block', type: 'math_constrain', }; - searcher.indexBlocks([mathConstrainBlock], workspace); + searcher.indexBlocks([mathConstrainBlock]); const matches = searcher.blockTypesMatching('conso'); @@ -219,12 +219,12 @@ suite('BlockSearcher', () => { ]); } - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const blockInfo = { kind: 'block', type: 'searcher_underscore_block', }; - searcher.indexBlocks([blockInfo], workspace); + searcher.indexBlocks([blockInfo]); assert.sameMembers( searcher.blockTypesMatching('searcher underscore block'), @@ -247,11 +247,11 @@ suite('BlockSearcher', () => { ]); } - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const blockA = {kind: 'block', type: 'searcher_charlie'}; const blockB = {kind: 'block', type: 'searcher_delta'}; - searcher.indexBlocks([blockA, blockB], workspace); + searcher.indexBlocks([blockA, blockB]); const broadQueryMatches = searcher.blockTypesMatching('alpha bravo'); assert.sameMembers(broadQueryMatches, [blockA, blockB]); @@ -298,9 +298,9 @@ suite('BlockSearcher', () => { ]); } - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const blockInfo = {kind: 'block', type: 'searcher_dropdown_alt'}; - searcher.indexBlocks([blockInfo], workspace); + searcher.indexBlocks([blockInfo]); assert.sameMembers(searcher.blockTypesMatching('sunny'), [blockInfo]); // 'cloudy' wasn't the selected option, but it should be set with the matching option if found. @@ -313,12 +313,12 @@ suite('BlockSearcher', () => { }); test('returns an empty list when no matches are found', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); assert.isEmpty(searcher.blockTypesMatching('abc123')); }); test('returns preset blocks', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const blocks = [ { kind: 'block', @@ -343,14 +343,14 @@ suite('BlockSearcher', () => { }, ]; - searcher.indexBlocks(blocks, workspace); + searcher.indexBlocks(blocks); const matches = searcher.blockTypesMatching('replace'); assert.sameMembers(matches, [blocks[0]]); }); test('indexes field values from the block definition', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const numberBlock = { kind: 'block', type: 'math_number', @@ -361,7 +361,7 @@ suite('BlockSearcher', () => { type: 'text_print', inputs: {TEXT: {shadow: {type: 'text', fields: {TEXT: 'abc'}}}}, }; - searcher.indexBlocks([numberBlock, printBlock], workspace); + searcher.indexBlocks([numberBlock, printBlock]); assert.sameMembers(searcher.blockTypesMatching('250'), [numberBlock]); // The value lives on a shadow block, not on the block itself. @@ -369,11 +369,8 @@ suite('BlockSearcher', () => { }); test('binds variable blocks to every matching variable', () => { - const searcher = new BlockSearcher(); - searcher.indexBlocks( - createVariableBlocks(['alpha', 'alphabet', 'beta']), - workspace, - ); + const searcher = new BlockSearcher(workspace); + searcher.indexBlocks(createVariableBlocks(['alpha', 'alphabet', 'beta'])); const matches = searcher.blockTypesMatching('alpha'); assert.sameMembers( @@ -389,58 +386,52 @@ suite('BlockSearcher', () => { ); }); - test("leaves matches unchanged when the query doesn't name a variable", () => { - const searcher = new BlockSearcher(); - const blocks = createVariableBlocks(['alpha']); - searcher.indexBlocks(blocks, workspace); - - const setter = blocks.find((block) => block.type === 'variables_set'); - assert.include(searcher.blockTypesMatching('set'), setter); - }); - test('does not index variable rename and delete options', () => { - const searcher = new BlockSearcher(); - searcher.indexBlocks(createVariableBlocks(['alpha']), workspace); + const searcher = new BlockSearcher(workspace); + searcher.indexBlocks(createVariableBlocks(['alpha'])); assert.isEmpty(searcher.blockTypesMatching('rename')); assert.isEmpty(searcher.blockTypesMatching('delete the')); }); test('sets dropdowns to the option that matched', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const sortBlock = {kind: 'block', type: 'lists_sort'}; - searcher.indexBlocks([sortBlock], workspace); + searcher.indexBlocks([sortBlock]); // 'numeric' is already selected, so the indexed block is returned as-is. assert.sameMembers(searcher.blockTypesMatching('numeric'), [sortBlock]); assert.sameDeepMembers( searcher.blockTypesMatching('alphabetic').map((match) => match.fields), - [{TYPE: 'TEXT'}, {TYPE: 'IGNORE_CASE'}], + [ + {TYPE: 'TEXT', DIRECTION: '1'}, + {TYPE: 'IGNORE_CASE', DIRECTION: '1'}, + ], ); }); test('varies one dropdown at a time', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); // Both WHERE1 and WHERE2 offer '# from end'. - searcher.indexBlocks( - [{kind: 'block', type: 'lists_getSublist'}], - workspace, - ); + searcher.indexBlocks([{kind: 'block', type: 'lists_getSublist'}]); assert.sameDeepMembers( searcher.blockTypesMatching('from end').map((match) => match.fields), - [{WHERE1: 'FROM_END'}, {WHERE2: 'FROM_END'}], + [ + {WHERE1: 'FROM_END', WHERE2: 'FROM_START'}, + {WHERE1: 'FROM_START', WHERE2: 'FROM_END'}, + ], ); }); test('indexes procedure names from extra state', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); const callBlock = { kind: 'block', type: 'procedures_callnoreturn', extraState: {name: 'draw sprites', params: []}, }; - searcher.indexBlocks([callBlock], workspace); + searcher.indexBlocks([callBlock]); assert.sameMembers(searcher.blockTypesMatching('draw sprites'), [ callBlock, @@ -448,26 +439,91 @@ suite('BlockSearcher', () => { }); test('does not match trigrams pooled from different strings', () => { - const searcher = new BlockSearcher(); + const searcher = new BlockSearcher(workspace); // 'controls flow statements' supplies 'tem' and 'next iteration' supplies // 'ite', but the block never says 'item'. - searcher.indexBlocks( - [{kind: 'block', type: 'controls_flow_statements'}], - workspace, - ); + searcher.indexBlocks([{kind: 'block', type: 'controls_flow_statements'}]); assert.isEmpty(searcher.blockTypesMatching('item')); }); test('replaces the previous index when reindexing', () => { - const searcher = new BlockSearcher(); - searcher.indexBlocks([{kind: 'block', type: 'text_print'}], workspace); + const searcher = new BlockSearcher(workspace); + searcher.indexBlocks([{kind: 'block', type: 'text_print'}]); // Reindexing should forget the previous pass entirely, not add to it. const negate = {kind: 'block', type: 'logic_negate'}; - searcher.indexBlocks([negate], workspace); + searcher.indexBlocks([negate]); assert.isEmpty(searcher.blockTypesMatching('print')); assert.sameMembers(searcher.blockTypesMatching('not'), [negate]); }); + + test('sets a dropdown on a child block without touching the root', () => { + const searcher = new BlockSearcher(workspace); + searcher.indexBlocks([ + { + kind: 'block', + type: 'controls_if', + inputs: {IF0: {block: {type: 'logic_compare', fields: {OP: 'GT'}}}}, + }, + ]); + + const matches = searcher.blockTypesMatching('≠'); + assert.lengthOf(matches, 1); + assert.equal(matches[0].inputs.IF0.block.fields.OP, 'NEQ'); + }); + + test('updates a variable field on a child block', () => { + ['item', 'score'].forEach((name) => + workspace.getVariableMap().createVariable(name), + ); + const searcher = new BlockSearcher(workspace); + searcher.indexBlocks([ + { + kind: 'block', + type: 'text_print', + inputs: {TEXT: {block: {type: 'variables_get'}}}, + }, + ]); + + const matches = searcher.blockTypesMatching('score'); + assert.deepEqual( + matches.map((match) => match.inputs.TEXT.block.fields.VAR.name), + ['score'], + ); + }); + + test('does not rewrite a sibling dropdown to text already showing', () => { + const searcher = new BlockSearcher(workspace); + const sublist = { + kind: 'block', + type: 'lists_getSublist', + fields: {WHERE1: 'FROM_END', WHERE2: 'LAST'}, + }; + searcher.indexBlocks([sublist]); + + // WHERE1 already reads '# from end', so WHERE2 is left alone. + assert.sameMembers(searcher.blockTypesMatching('from end'), [sublist]); + }); + + test('leaves a block alone when the query is already visible', () => { + ['alpha', 'beta'].forEach((name) => + workspace.getVariableMap().createVariable(name), + ); + const searcher = new BlockSearcher(workspace); + const forEach = { + kind: 'block', + type: 'controls_forEach', + fields: {VAR: {name: 'alpha'}}, + inputs: { + LIST: {block: {type: 'variables_get', fields: {VAR: {name: 'beta'}}}}, + }, + }; + searcher.indexBlocks([forEach]); + + // 'alpha' is already on one of the two variable fields, so neither is + // updated. + assert.sameMembers(searcher.blockTypesMatching('alpha'), [forEach]); + }); }); From 4990b09ac1749c2a9f340c6db7fe20a7d8a28058 Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:03:22 -0400 Subject: [PATCH 5/7] feat: move the toolbox search field into the flyout --- packages/plugins/toolbox-search/README.md | 26 +- .../toolbox-search/src/search_field_node.ts | 51 +++ .../src/search_flyout_navigator.ts | 80 ++++ .../toolbox-search/src/toolbox_search.ts | 414 +++++++++++++----- packages/plugins/toolbox-search/test/index.ts | 2 +- .../toolbox-search/test/tests.mocha.js | 73 +++ 6 files changed, 544 insertions(+), 102 deletions(-) create mode 100644 packages/plugins/toolbox-search/src/search_field_node.ts create mode 100644 packages/plugins/toolbox-search/src/search_flyout_navigator.ts diff --git a/packages/plugins/toolbox-search/README.md b/packages/plugins/toolbox-search/README.md index 0033309f126..1003a3c7cad 100644 --- a/packages/plugins/toolbox-search/README.md +++ b/packages/plugins/toolbox-search/README.md @@ -1,8 +1,30 @@ # @blockly/toolbox-search [![Built on Blockly](https://tinyurl.com/built-on-blockly)](https://github.com/google/blockly) A [Blockly](https://www.npmjs.com/package/blockly) plugin that adds a toolbox category for searching -blocks. The category displays a search field in the toolbox, and filters the available blocks as you -type. The Blockly docs have [more information about toolbox definitions and categories](https://docs.blockly.com/guides/configure/toolboxes/category/). +blocks. Selecting the category opens its flyout with a search field pinned to the top, and the +flyout fills with matching blocks as you type. The Blockly docs have +[more information about toolbox definitions and categories](https://docs.blockly.com/guides/configure/toolboxes/category/). + +## Keyboard + +The category behaves like any other, except that moving into its flyout lands in the search field +rather than on the first block. `Ctrl`/`Cmd`+`B` also opens the category and puts the cursor there +from anywhere. + +From inside the search field, in any toolbox: + +- `Enter`: go to the first result +- `Escape`: go to the workspace +- `Left`/`Right`: move the cursor, and never leave the field + +In a vertical toolbox: + +- `Up`/`Down`: move between the search field and the results, wrapping at either end + +In a horizontal toolbox: + +- `Up`: go back to the category, if the toolbox is at the top +- `Down`: go back to the category, if the toolbox is at the bottom ## Installation diff --git a/packages/plugins/toolbox-search/src/search_field_node.ts b/packages/plugins/toolbox-search/src/search_field_node.ts new file mode 100644 index 00000000000..8a803207326 --- /dev/null +++ b/packages/plugins/toolbox-search/src/search_field_node.ts @@ -0,0 +1,51 @@ +/** + * @license + * Copyright 2026 Raspberry Pi Foundation + * SPDX-License-Identifier: Apache-2.0 + */ + +import type * as Blockly from 'blockly/core'; + +/** + * Makes the search field a focusable node of the flyout's workspace. + */ +export class SearchFieldNode + implements Blockly.IFocusableNode, Blockly.IComponent +{ + /** + * @param id The DOM ID of the input, which the flyout workspace matches + * against to resolve focus back to this node. IComponent requires + * this exact name. + * @param input The search input. + * @param flyout The flyout the field sits above. + */ + constructor( + readonly id: string, + private readonly input: HTMLInputElement, + private readonly flyout: Blockly.IFlyout, + ) {} + + /** @returns The search input. */ + getFocusableElement(): HTMLElement | SVGElement { + return this.input; + } + + /** @returns The flyout's workspace. */ + getFocusableTree(): Blockly.IFocusableTree { + return this.flyout.getWorkspace(); + } + + /** + * @returns Whether the field is on screen. A hidden field can't be focused, + * and this is also what keeps it out of other categories' flyouts. + */ + canBeFocused(): boolean { + return this.input.parentElement?.style.display !== 'none'; + } + + /** See IFocusableNode.onNodeFocus. */ + onNodeFocus(): void {} + + /** See IFocusableNode.onNodeBlur. */ + onNodeBlur(): void {} +} diff --git a/packages/plugins/toolbox-search/src/search_flyout_navigator.ts b/packages/plugins/toolbox-search/src/search_flyout_navigator.ts new file mode 100644 index 00000000000..4bb3d60d1f7 --- /dev/null +++ b/packages/plugins/toolbox-search/src/search_flyout_navigator.ts @@ -0,0 +1,80 @@ +/** + * @license + * Copyright 2026 Raspberry Pi Foundation + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as Blockly from 'blockly/core'; +import type {SearchFieldNode} from './search_field_node'; + +/** + * Makes the search field the flyout's entry point, so that moving in from + * the category reaches the field rather than the first block. + * + * In a vertical flyout, the user can navigate from the field to the blocks + * using the up and down arrows. + * In a horizontal flyout, the left and right arrow keys are still only used + * for the field's cursor. The match results can be reached with enter instead. + */ +export class SearchFlyoutNavigator extends Blockly.FlyoutNavigator { + /** + * @param flyout The flyout being navigated. + * @param searchNode The search field's focusable node. + */ + constructor( + flyout: Blockly.IFlyout, + private readonly searchNode: SearchFieldNode, + ) { + super(flyout); + } + + /** + * @returns Whether the search field takes part in arrow-key navigation. + */ + private cyclesThroughSearchField(): boolean { + return !this.flyout.horizontalLayout && this.searchNode.canBeFocused(); + } + + /** @returns The flyout's items, not counting the search field. */ + private getFlyoutItems(): Blockly.IFocusableNode[] { + return super.getTopLevelItems(); + } + + /** @returns The first result, or null if the search matched nothing. */ + getFirstItem(): Blockly.IFocusableNode | null { + return this.getFlyoutItems()[0] ?? null; + } + + /** @returns The last result, or null if the search matched nothing. */ + getLastItem(): Blockly.IFocusableNode | null { + const items = this.getFlyoutItems(); + return items[items.length - 1] ?? null; + } + + /** @returns The items to cycle through with the arrow keys. */ + protected override getTopLevelItems(): Blockly.IFocusableNode[] { + const items = this.getFlyoutItems(); + return this.cyclesThroughSearchField() + ? [this.searchNode, ...items] + : items; + } + + /** + * @param node The node to check. + * @returns Whether the node can be navigated to. The base implementation + * only admits the flyout's own contents. + */ + protected override isNavigable(node: Blockly.IFocusableNode) { + if (node === this.searchNode) return this.cyclesThroughSearchField(); + return super.isNavigable(node); + } + + /** + * @returns The search field, so that arrowing in from the toolbox lands + * there whichever way the flyout is laid out. + */ + override getFirstNode(): Blockly.IFocusableNode | null { + if (this.searchNode.canBeFocused()) return this.searchNode; + return super.getFirstNode(); + } +} diff --git a/packages/plugins/toolbox-search/src/toolbox_search.ts b/packages/plugins/toolbox-search/src/toolbox_search.ts index 9b699041e57..90a468750ad 100644 --- a/packages/plugins/toolbox-search/src/toolbox_search.ts +++ b/packages/plugins/toolbox-search/src/toolbox_search.ts @@ -10,22 +10,50 @@ */ import * as Blockly from 'blockly/core'; import {BlockSearcher} from './block_searcher'; +import {SearchFieldNode} from './search_field_node'; +import {SearchFlyoutNavigator} from './search_flyout_navigator'; /* eslint-disable @typescript-eslint/naming-convention */ +/** + * Padding between a flyout's scrollbar and the edge of the flyout. Mirrors + * Flyout.SCROLLBAR_MARGIN, which is not part of the IFlyout interface. + */ +const SCROLLBAR_MARGIN = 2.5; + /** * A toolbox category that provides a search field and displays matching blocks * in its flyout. + * + * The search field is deliberately not a flyout item. Typing into it rebuilds + * the flyout's contents, which would dispose the field along with everything + * else. Instead the category registers itself as an IPositionable and pins the + * field to the flyout's top edge, reserving room for it with a separator at + * the head of the flyout. */ -export class ToolboxSearchCategory extends Blockly.ToolboxCategory { +export class ToolboxSearchCategory + extends Blockly.ToolboxCategory + implements Blockly.IPositionable +{ private static readonly START_SEARCH_SHORTCUT = 'startSearch'; static readonly SEARCH_CATEGORY_KIND = 'search'; - private readonly SEARCH_INPUT_ID = 'toolbox-search-input'; + /** + * Identifies the search field to the workspace's ComponentManager. + * IComponent requires this exact name; it is not the toolbox item's ID, + * which is getId(). + */ + readonly id: string; + private searchFieldDiv?: HTMLDivElement; private searchField?: HTMLInputElement; + private searchNode?: SearchFieldNode; + private flyoutNavigator?: SearchFlyoutNavigator; + /** The flyout this category searches into. */ + private readonly flyout: Blockly.IFlyout; private blockSearcher: BlockSearcher; private onChangeWrapper?: (event: Blockly.Events.Abstract) => void; private indexedBlocks = ''; private boundEvents: Blockly.browserEvents.Data[] = []; + private resultCount = 0; /** * Initializes a ToolboxSearchCategory. @@ -42,6 +70,14 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { opt_parent?: Blockly.ICollapsibleToolboxItem, ) { super(categoryDef, parentToolbox, opt_parent); + const flyout = parentToolbox.getFlyout(); + if (!flyout) { + throw Error('A search category needs a toolbox that has a flyout.'); + } + this.flyout = flyout; + // Suffixed with the toolbox item's ID so that two workspaces on a page + // don't share the same ID. + this.id = `toolbox-search-input-${this.getId()}`; this.blockSearcher = new BlockSearcher(this.workspace_); this.initBlockSearcher(); this.registerShortcut(); @@ -50,79 +86,171 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { } /** - * Initializes the search field toolbox category. - * - * @returns The
that will be displayed in the toolbox. + * Initializes the category and creates its search field. */ - protected override createDom_(): HTMLDivElement { - const dom = super.createDom_(); - this.searchField = document.createElement('input'); - this.searchField.id = this.SEARCH_INPUT_ID; - this.searchField.type = 'search'; - this.searchField.placeholder = Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER']; - this.workspace_.RTL - ? (this.searchField.style.marginRight = '8px') - : (this.searchField.style.marginLeft = '8px'); + override init() { + super.init(); + const searchField = this.createSearchField(); this.boundEvents.push( Blockly.browserEvents.conditionalBind( - this.searchField, + searchField, 'keydown', this, + // Blockly suppresses its own keyboard shortcuts while a text input + // has focus, so every key that should move focus is handled here. + // Left and right are left alone in both layouts: inside the field + // they belong to the cursor. (event: KeyboardEvent) => { - if ( - event.key === 'ArrowUp' && - this.searchField?.selectionStart === 0 - ) { - const previous = this.parentToolbox_ - .getNavigator() - .getPreviousNode(); - if (previous) { - Blockly.getFocusManager().focusNode(previous); - } - return; - } else if ( - event.key === 'ArrowRight' && - this.searchField?.selectionStart === this.searchField?.value.length - ) { - const previous = this.parentToolbox_.getNavigator().getInNode(); - if (previous) { - Blockly.getFocusManager().focusNode(previous); - } - return; - } else if ( - event.key === 'ArrowDown' && - this.searchField?.selectionStart === this.searchField?.value.length - ) { - const next = this.parentToolbox_.getNavigator().getNextNode(); - if (next) { - Blockly.getFocusManager().focusNode(next); - } - return; - } else if (event.key === 'Escape' && this.searchField) { - if (this.searchField.value !== '') { - this.searchField.value = ''; - event.stopPropagation(); - // Removes matches from the flyout after programmatically clearing the search field. - this.matchBlocks(); + const isHorizontal = this.flyout.horizontalLayout; + let handled = true; + if (event.key === 'Escape') { + Blockly.getFocusManager().focusTree(this.workspace_); + } else if (event.key === 'Enter') { + this.focusResult(); + } else if (!isHorizontal && event.key === 'ArrowDown') { + this.focusResult(); + } else if (!isHorizontal && event.key === 'ArrowUp') { + this.focusResult(true); + } else if (isHorizontal) { + const isBottom = + this.workspace_.options.toolboxPosition === + Blockly.utils.toolbox.Position.BOTTOM; + const leaveKey = isBottom ? 'ArrowDown' : 'ArrowUp'; + if (event.key === leaveKey) { + Blockly.getFocusManager().focusNode(this); } + } else { + handled = false; + } + + if (handled) { + event.preventDefault(); } }, ), // When the user types in the search field, update the flyout to show matching blocks. - Blockly.browserEvents.conditionalBind( - this.searchField, - 'input', - this, - () => this.matchBlocks(), + Blockly.browserEvents.conditionalBind(searchField, 'input', this, () => + this.matchBlocks(), ), ); - this.rowContents_?.replaceChildren(this.searchField); - return dom; } - /** The ID of the toolbox item must match the ID of the focusable node. */ - override getId(): string { - return this.SEARCH_INPUT_ID; + /** + * Builds the search field and pins it over the flyout. + * + * @returns The search input. + */ + private createSearchField(): HTMLInputElement { + const input = document.createElement('input'); + input.id = this.id; + input.type = 'search'; + input.placeholder = Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER']; + Blockly.utils.aria.setState( + input, + Blockly.utils.aria.State.LABEL, + Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER'], + ); + this.searchField = input; + + const div = document.createElement('div'); + div.classList.add('blocklyToolboxSearchField'); + div.appendChild(input); + this.searchFieldDiv = div; + this.setFieldVisible(false); + + // Match the flyout's colour, so blocks scrolling past go behind it. + // Themes without a flyout colour fall back to the one in the CSS below. + this.workspace_ + .getThemeManager() + .subscribe(div, 'flyoutBackgroundColour', 'background-color'); + this.workspace_ + .getInjectionDiv() + .insertBefore(div, this.workspace_.getParentSvg()); + this.workspace_.getComponentManager().addComponent({ + component: this, + weight: 0, + capabilities: [Blockly.ComponentManager.Capability.POSITIONABLE], + }); + + // Make the field a focusable node and first in the flyout's navigation order. + const flyout = this.flyout; + this.searchNode = new SearchFieldNode(this.id, input, flyout); + this.flyoutNavigator = new SearchFlyoutNavigator(flyout, this.searchNode); + flyout + .getWorkspace() + .getComponentManager() + .addComponent({ + component: this.searchNode, + weight: 0, + capabilities: [Blockly.ComponentManager.Capability.FOCUSABLE], + }); + flyout.getWorkspace().setNavigator(this.flyoutNavigator); + return input; + } + + /** + * Moves focus to one end of the results. + * + * @param getLast Whether to focus the last item instead of the first. + */ + private focusResult(getLast = false) { + if (!this.flyoutNavigator) return; + const result = getLast + ? this.flyoutNavigator.getLastItem() + : this.flyoutNavigator.getFirstItem(); + if (result) { + Blockly.getFocusManager().focusNode(result); + } + } + + /** + * Shows or hides the search field. + * + * @param visible Whether the field should be shown. + */ + private setFieldVisible(visible: boolean) { + if (this.searchFieldDiv) { + this.searchFieldDiv.style.display = visible ? 'block' : 'none'; + } + } + + /** + * Returns the bounding rectangle of the UI element in pixel units relative to + * the Blockly injection div. + * + * @returns The component’s bounding box. Null in this case since we don't need + * other elements to avoid the toolbox search field. + */ + getBoundingRectangle(): Blockly.utils.Rect | null { + return null; + } + + /** + * Aligns the search field with the top edge of the flyout. + */ + position() { + const div = this.searchFieldDiv; + const flyout = this.flyout; + if (!div || !flyout.isVisible()) return; + + const margin = flyout.MARGIN; + let left = flyout.getX() + margin; + + if (flyout.horizontalLayout) { + // The flyout spans the workspace and its scrollbar runs along the + // bottom, so there is nothing to size the field against. + div.style.width = ''; + } else { + // Fit the search field within the flyout, accounting for the scrollbar. + const scrollbarInset = + Blockly.Scrollbar.scrollbarThickness + SCROLLBAR_MARGIN; + if (this.workspace_.RTL) left += scrollbarInset; + const width = flyout.getWidth() - 2 * margin - scrollbarInset; + div.style.width = `${Math.max(width, 0)}px`; + } + + div.style.left = `${left}px`; + div.style.top = `${flyout.getY() + margin}px`; } /** @@ -136,7 +264,14 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { Blockly.ShortcutRegistry.registry.register({ name: ToolboxSearchCategory.START_SEARCH_SHORTCUT, callback: () => { - Blockly.getFocusManager().focusNode(this); + if (this.parentToolbox_.getSelectedItem() !== this) { + // Shows the flyout and the field, so that the field can take focus. + this.parentToolbox_.setSelectedItem(this); + } + this.matchBlocks(); + if (this.searchNode?.canBeFocused()) { + Blockly.getFocusManager().focusNode(this.searchNode); + } return true; }, keyCodes: [shortcut], @@ -189,21 +324,26 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { this.blockSearcher.indexBlocks([...availableBlocks]); } - /** See IFocusableNode.getFocusableElement. */ - override getFocusableElement(): HTMLElement | SVGElement { - if (!this.searchField) { - throw Error('This field currently has no representative DOM element.'); - } - return this.searchField; - } - /** See IFocusableNode.onNodeFocus. */ override onNodeFocus(): void { super.onNodeFocus(); - if (!this.searchField?.value) { - Blockly.renderManagement.finishQueuedRenders().then(() => { - this.matchBlocks(); - }); + Blockly.renderManagement.finishQueuedRenders().then(() => { + this.matchBlocks(); + }); + } + + /** + * Shows the search field alongside this category's flyout. + * + * @param isSelected Whether this category is now the selected one. + */ + override setSelected(isSelected: boolean) { + super.setSelected(isSelected); + this.setFieldVisible(isSelected); + if (!isSelected) { + // The focus manager only cleans up focus indicators it can find inside + // the toolbox, and the search field is not in there. + this.searchField?.classList.remove('blocklyPassiveFocus'); } } @@ -212,40 +352,99 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { */ private matchBlocks() { const query = this.searchField?.value || ''; + const results = query ? this.blockSearcher.blockTypesMatching(query) : []; - const oldCount = this.flyoutItems_.length; - this.flyoutItems_ = query - ? this.blockSearcher.blockTypesMatching(query) - : []; - - const newCount = this.flyoutItems_.length; - - if (!this.flyoutItems_.length) { - this.flyoutItems_.push({ - kind: 'label', - text: - query.length < 3 - ? Blockly.Msg['TOOLBOX_SEARCH_PROMPT'] - : Blockly.Msg['TOOLBOX_SEARCH_NO_RESULTS'], - }); - } + const oldCount = this.resultCount; + this.resultCount = results.length; + this.flyoutItems_ = this.generateFlyoutContent(query, results); if (this.parentToolbox_.getSelectedItem() !== this) { return; } - if (oldCount !== newCount) { + if (oldCount !== results.length) { Blockly.utils.aria.announceDynamicAriaState( - newCount === 1 + results.length === 1 ? Blockly.Msg['TOOLBOX_SEARCH_RESULT_COUNT_ONE'] : Blockly.Msg['TOOLBOX_SEARCH_RESULT_COUNT'].replace( '%1', - String(newCount), + String(results.length), ), ); } + // Prevent losing the cursor position in the search field when the flyout is rebuilt. + const cursorToRestore = + this.searchField && + Blockly.getFocusManager().getFocusedNode() === this.searchNode + ? { + start: this.searchField.selectionStart, + end: this.searchField.selectionEnd, + } + : null; + this.parentToolbox_.refreshSelection(); + + if (cursorToRestore && this.searchField && this.searchNode) { + Blockly.getFocusManager().focusNode(this.searchNode); + this.searchField.setSelectionRange( + cursorToRestore.start, + cursorToRestore.end, + ); + } + // Showing the flyout may have changed its width, and that path doesn't + // run positionable components. + this.position(); + } + + /** + * Returns the room to reserve at the head of the flyout for the search field + * + * @returns The height of the search field, in workspace units. + */ + private spacerGap(): number { + const flyout = this.flyout; + const scale = flyout.getWorkspace().scale || 1; + // Leave the flyout's own margin below the field as well as room for the + // field itself. + const clearance = flyout.MARGIN; + const size = + (flyout.horizontalLayout + ? this.searchFieldDiv?.offsetWidth + : this.searchFieldDiv?.offsetHeight) ?? 0; + return (size + clearance) / scale; + } + + /** + * Builds the flyout contents for a query. + * + * The list always starts with a separator tall enough to clear the search + * field, so that the first result appears below it rather than behind it. + * + * @param query The current search query. + * @param results The blocks matching the query. + * @returns The contents to show in the flyout. + */ + private generateFlyoutContent( + query: string, + results: Blockly.utils.toolbox.BlockInfo[], + ): Blockly.utils.toolbox.FlyoutItemInfoArray { + const items: Blockly.utils.toolbox.FlyoutItemInfoArray = [ + {kind: 'sep', gap: this.spacerGap()}, + ]; + + if (results.length) { + items.push(...results); + } else { + items.push({ + kind: 'label', + text: + query.length < 3 + ? Blockly.Msg['TOOLBOX_SEARCH_PROMPT'] + : Blockly.Msg['TOOLBOX_SEARCH_NO_RESULTS'], + }); + } + return items; } /** @@ -257,6 +456,17 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { Blockly.browserEvents.unbind(event); } this.boundEvents.length = 0; + if (this.searchFieldDiv) { + this.workspace_.getThemeManager().unsubscribe(this.searchFieldDiv); + this.workspace_.getComponentManager().removeComponent(this.id); + this.searchFieldDiv.remove(); + this.searchFieldDiv = undefined; + } + if (this.searchNode) { + this.flyout.getWorkspace().getComponentManager().removeComponent(this.id); + this.searchNode = undefined; + } + this.searchField = undefined; if (this.onChangeWrapper) { this.workspace_.removeChangeListener(this.onChangeWrapper); this.onChangeWrapper = undefined; @@ -314,14 +524,20 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { } } -// Make the clear button clickable in Safari. Blockly.Css.register(` -input[type="search"]::-webkit-search-cancel-button { - -webkit-appearance: searchfield-cancel-button; - pointer-events: auto !important; - position: relative; - z-index: 10; -}`); +.blocklyToolboxSearchField { + /* Matches the default fill of .blocklyFlyoutBackground. + A theme that sets flyoutBackgroundColour overrides this. */ + background-color: #ddd; + position: absolute; + /* Matches the z-index of .blocklyFlyout. */ + z-index: 20; +} + +.blocklyToolboxSearchField>input { + width: 100%; +} +`); Blockly.registry.register( Blockly.registry.Type.TOOLBOX_ITEM, diff --git a/packages/plugins/toolbox-search/test/index.ts b/packages/plugins/toolbox-search/test/index.ts index fcff7dfa590..737d5e35837 100644 --- a/packages/plugins/toolbox-search/test/index.ts +++ b/packages/plugins/toolbox-search/test/index.ts @@ -239,7 +239,7 @@ const toolbox = { }, ], }, - {kind: 'search', name: 'Search', contents: []}, + {kind: 'search', name: 'Search', colour: '#000', contents: []}, ], }; diff --git a/packages/plugins/toolbox-search/test/tests.mocha.js b/packages/plugins/toolbox-search/test/tests.mocha.js index 5dfd8f224de..773f038ccde 100644 --- a/packages/plugins/toolbox-search/test/tests.mocha.js +++ b/packages/plugins/toolbox-search/test/tests.mocha.js @@ -113,6 +113,79 @@ suite('ToolboxSearchCategory', () => { assert.include(flyoutBlockTypes(this.workspace), 'variables_get'); }); + + test('reserves room for the search field above the results', function () { + this.search('controls if'); + + const contents = this.searchCategory.getContents(); + assert.equal(contents[0].kind, 'sep'); + assert.deepEqual( + contents.slice(1).map((item) => item.type), + ['controls_if'], + ); + }); + + test('explains itself in the flyout when nothing matches', function () { + const label = () => this.searchCategory.getContents().slice(1); + + // Too short to have matched anything yet, so it still asks for a query. + this.search('ab'); + assert.deepEqual(label(), [ + {kind: 'label', text: Blockly.Msg['TOOLBOX_SEARCH_PROMPT']}, + ]); + + this.search('zzz'); + + assert.deepEqual(label(), [ + {kind: 'label', text: Blockly.Msg['TOOLBOX_SEARCH_NO_RESULTS']}, + ]); + }); + + test('shows and hides the search field with the flyout', function () { + const isSearchFieldVisible = () => + this.searchCategory.searchField.parentElement.style.display !== 'none'; + const toolbox = this.workspace.getToolbox(); + + assert.isFalse(isSearchFieldVisible()); + + toolbox.setSelectedItem(this.searchCategory); + assert.isTrue(isSearchFieldVisible()); + + toolbox.clearSelection(); + assert.isFalse(isSearchFieldVisible()); + + const firstCategory = toolbox.contents.get( + toolbox.contents.keys().next().value, + ); + toolbox.setSelectedItem(firstCategory); + assert.isFalse(isSearchFieldVisible()); + }); + + test('builds the same row as an ordinary category', function () { + const [logic] = this.workspace.getToolbox().getToolboxItems(); + const rowClasses = (category) => + [...category.getDiv().querySelectorAll('*')].map((el) => el.className); + + assert.deepEqual(rowClasses(this.searchCategory), rowClasses(logic)); + assert.equal( + this.searchCategory.getDiv().querySelector('.blocklyToolboxCategoryLabel') + .textContent, + 'Search', + ); + }); + + test('remembers its query while the flyout is hidden', function () { + const toolbox = this.workspace.getToolbox(); + this.search('controls if'); + + toolbox.clearSelection(); + this.clock.runAll(); + toolbox.setSelectedItem(this.searchCategory); + this.clock.runAll(); + + assert.equal(this.searchCategory.searchField.value, 'controls if'); + assert.deepEqual(flyoutBlockTypes(this.workspace), ['controls_if']); + }); }); suite('BlockSearcher', () => { From e5164d4475b82d365a0e2bfac81aa5530dc1a7d3 Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:07:18 -0400 Subject: [PATCH 6/7] chore: tweak comments --- .../plugins/toolbox-search/src/toolbox_search.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/plugins/toolbox-search/src/toolbox_search.ts b/packages/plugins/toolbox-search/src/toolbox_search.ts index 90a468750ad..64c84adf10c 100644 --- a/packages/plugins/toolbox-search/src/toolbox_search.ts +++ b/packages/plugins/toolbox-search/src/toolbox_search.ts @@ -172,7 +172,8 @@ export class ToolboxSearchCategory capabilities: [Blockly.ComponentManager.Capability.POSITIONABLE], }); - // Make the field a focusable node and first in the flyout's navigation order. + // Make the field a focusable node of the flyout's workspace, and the node + // that arrowing into the flyout lands on. const flyout = this.flyout; this.searchNode = new SearchFieldNode(this.id, input, flyout); this.flyoutNavigator = new SearchFlyoutNavigator(flyout, this.searchNode); @@ -218,7 +219,7 @@ export class ToolboxSearchCategory * Returns the bounding rectangle of the UI element in pixel units relative to * the Blockly injection div. * - * @returns The component’s bounding box. Null in this case since we don't need + * @returns The component's bounding box. Null in this case since we don't need * other elements to avoid the toolbox search field. */ getBoundingRectangle(): Blockly.utils.Rect | null { @@ -341,8 +342,8 @@ export class ToolboxSearchCategory super.setSelected(isSelected); this.setFieldVisible(isSelected); if (!isSelected) { - // The focus manager only cleans up focus indicators it can find inside - // the toolbox, and the search field is not in there. + // The field belongs to the flyout's workspace but sits outside its SVG, + // so core's own cleanup can't find the indicator to remove. this.searchField?.classList.remove('blocklyPassiveFocus'); } } @@ -398,7 +399,7 @@ export class ToolboxSearchCategory } /** - * Returns the room to reserve at the head of the flyout for the search field + * Returns the room to reserve at the head of the flyout for the search field. * * @returns The height of the search field, in workspace units. */ @@ -530,7 +531,7 @@ Blockly.Css.register(` A theme that sets flyoutBackgroundColour overrides this. */ background-color: #ddd; position: absolute; - /* Matches the z-index of .blocklyFlyout. */ + /* Matches the z-index of .blocklyFlyout. */ z-index: 20; } From 6d92327de08fcae1a3fe21a5870326709cf5343d Mon Sep 17 00:00:00 2001 From: Mike Harvey <43474485+mikeharv@users.noreply.github.com> Date: Fri, 16 Oct 2026 15:07:31 -0400 Subject: [PATCH 7/7] fix: code review refactors --- .../src/search_field_navigation_policy.ts | 91 +++++++ .../toolbox-search/src/search_field_node.ts | 241 +++++++++++++++-- .../src/search_flyout_navigator.ts | 96 ++++--- .../toolbox-search/src/toolbox_search.ts | 249 ++++-------------- .../toolbox-search/test/tests.mocha.js | 20 +- 5 files changed, 428 insertions(+), 269 deletions(-) create mode 100644 packages/plugins/toolbox-search/src/search_field_navigation_policy.ts diff --git a/packages/plugins/toolbox-search/src/search_field_navigation_policy.ts b/packages/plugins/toolbox-search/src/search_field_navigation_policy.ts new file mode 100644 index 00000000000..88f2b772b7d --- /dev/null +++ b/packages/plugins/toolbox-search/src/search_field_navigation_policy.ts @@ -0,0 +1,91 @@ +/** + * @license + * Copyright 2026 Raspberry Pi Foundation + * SPDX-License-Identifier: Apache-2.0 + */ + +import type * as Blockly from 'blockly/core'; +import {SearchFieldNode} from './search_field_node'; + +/** + * Set of rules controlling keyboard navigation from the search field. + * + * In a vertical flyout the field is part of the arrow-key cycle, which also + * makes it the flyout's entry point. In a horizontal flyout it is kept out of + * that cycle so that left and right stay with the text cursor. It remains the + * entry point via SearchFlyoutNavigator.isHorizontalEntryPoint(). + */ +export class SearchFieldNavigationPolicy implements Blockly.INavigationPolicy { + /** + * @param flyout The flyout the field belongs to. + */ + constructor(private readonly flyout: Blockly.IFlyout) {} + + /** + * Returns the first child of the given search field. + * + * @returns Null. + */ + getFirstChild(): Blockly.IFocusableNode | null { + return null; + } + + /** + * Returns the parent of the given search field. + * + * @returns Null; SearchFlyoutNavigator places the field itself. + */ + getParent(): Blockly.IFocusableNode | null { + return null; + } + + /** + * Returns the next sibling of the given search field. + * + * @returns Null; its neighbours come from getTopLevelItems(). + */ + getNextSibling(): Blockly.IFocusableNode | null { + return null; + } + + /** + * Returns the previous sibling of the given search field. + * + * @returns Null; its neighbours come from getTopLevelItems(). + */ + getPreviousSibling(): Blockly.IFocusableNode | null { + return null; + } + + /** + * Returns the row ID of the given search field. + * + * @param current The search field to retrieve the row ID of. + * @returns The row ID of the given search field. + */ + getRowId(current: SearchFieldNode): string { + return current.id; + } + + /** + * Returns whether or not the given search field can be navigated to. + * + * @param current The instance to check for navigability. + * @returns True in a vertical flyout, where the field is part of the + * arrow-key cycle; false in a horizontal one, where it is reached + * explicitly instead. + */ + isNavigable(current: SearchFieldNode): boolean { + return current.canBeFocused() && !this.flyout.horizontalLayout; + } + + /** + * Returns whether the given object can be navigated from by this policy. + * + * @param current The object to check if this policy applies to. + * @returns True if the object is a SearchFieldNode. + */ + isApplicable(current: Blockly.IFocusableNode): current is SearchFieldNode { + return current instanceof SearchFieldNode; + } +} diff --git a/packages/plugins/toolbox-search/src/search_field_node.ts b/packages/plugins/toolbox-search/src/search_field_node.ts index 8a803207326..30b925ce6ee 100644 --- a/packages/plugins/toolbox-search/src/search_field_node.ts +++ b/packages/plugins/toolbox-search/src/search_field_node.ts @@ -4,43 +4,172 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type * as Blockly from 'blockly/core'; +import * as Blockly from 'blockly/core'; /** - * Makes the search field a focusable node of the flyout's workspace. + * The field's box size, in CSS pixels. + */ +const FIELD_HEIGHT = 24; +const FIELD_WIDTH = 150; + +/** + * The search field that a search category pins to the top of its flyout. + * + * The field is deliberately not a flyout item. Typing into it rebuilds the + * flyout's contents, which would dispose the field along with everything else. + * It is positioned over the flyout instead, and reserves room for itself with + * a separator at the head of the flyout's contents. + * + * It is still a focusable node of the flyout's workspace, so focusing it does + * not read as leaving the toolbox and close the flyout. */ export class SearchFieldNode - implements Blockly.IFocusableNode, Blockly.IComponent + implements Blockly.IFocusableNode, Blockly.IPositionable { + private readonly input: HTMLInputElement; + private readonly foreignObject: SVGForeignObjectElement; + private readonly boundEvents: Blockly.browserEvents.Data[] = []; + /** - * @param id The DOM ID of the input, which the flyout workspace matches - * against to resolve focus back to this node. IComponent requires - * this exact name. - * @param input The search input. + * @param id The DOM ID of the input, which the flyout's workspace matches + * against to resolve focus back to this node. IComponent requires this + * exact name. + * @param workspace The workspace whose injection div the field is added to. * @param flyout The flyout the field sits above. + * @param category The category the field belongs to. */ constructor( readonly id: string, - private readonly input: HTMLInputElement, - private readonly flyout: Blockly.IFlyout, - ) {} + private readonly workspace: Blockly.WorkspaceSvg, + private readonly flyout: Blockly.Flyout, + private readonly category: Blockly.ISelectableToolboxItem, + ) { + const html = Blockly.utils.dom.HTML_NS; + this.foreignObject = Blockly.utils.dom.createSvgElement( + Blockly.utils.Svg.FOREIGNOBJECT, + {}, + ); + const body = document.createElementNS(html, 'body'); + body.setAttribute('xmlns', html); + body.className = 'blocklyMinimalBody blocklyToolboxSearchField'; + this.input = document.createElementNS(html, 'input') as HTMLInputElement; + this.input.id = id; + this.input.type = 'search'; + this.input.placeholder = Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER']; + Blockly.utils.aria.setState( + this.input, + Blockly.utils.aria.State.LABEL, + Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER'], + ); + + body.appendChild(this.input); + this.foreignObject.appendChild(body); + this.setVisible(false); + + flyout.getWorkspace().getSvgGroup().appendChild(this.foreignObject); + + workspace.getComponentManager().addComponent({ + component: this, + weight: 0, + capabilities: [Blockly.ComponentManager.Capability.POSITIONABLE], + }); + flyout + .getWorkspace() + .getComponentManager() + .addComponent({ + component: this, + weight: 0, + capabilities: [Blockly.ComponentManager.Capability.FOCUSABLE], + }); + + this.boundEvents.push( + Blockly.browserEvents.conditionalBind( + flyout.getWorkspace().getParentSvg(), + 'focusin', + this, + (event: FocusEvent) => this.keepClearOfField(event.target), + ), + ); + } + + /** + * Scrolls the flyout on if a newly focused item would sit under the field. + * + * Blockly scrolls a focused item just far enough to reach the edge of the + * viewport, which in a horizontal flyout is the edge the field covers. + * + * @param target The element that just took focus. + */ + private keepClearOfField(target: EventTarget | null) { + if ( + !this.flyout.horizontalLayout || + !this.canBeFocused() || + !(target instanceof Element) + ) { + return; + } - /** @returns The search input. */ + const flyoutWorkspace = this.flyout.getWorkspace(); + + // Focus lands on an element inside the item, so climb to the item itself. + const canvas = flyoutWorkspace.getCanvas(); + let root: Element | null = target; + while (root && root.parentNode !== canvas) root = root.parentElement; + if (!root) return; + + const item = root.getBoundingClientRect(); + if (!item.width) return; + const field = this.foreignObject.getBoundingClientRect(); + // Leave the same gap the flyout leaves before its first item, so that an + // item reached by wrapping lands where scrolling to the start would put it. + // The rects are CSS pixels and these two are workspace units. + const clearance = + (this.flyout.MARGIN + + flyoutWorkspace.getRenderer().getConstants().TAB_WIDTH) * + flyoutWorkspace.scale; + const overlap = this.flyout.RTL + ? item.right - (field.left - clearance) + : field.right + clearance - item.left; + if (overlap <= 0) return; + + const offset = this.flyout.RTL ? overlap : -overlap; + flyoutWorkspace.scrollbar?.setX( + flyoutWorkspace.getMetricsManager().getViewMetrics().left + offset, + ); + } + + /** @returns The search input, which the category binds its listeners to. */ + getInput(): HTMLInputElement { + return this.input; + } + + /** See IFocusableNode.getFocusableElement. */ getFocusableElement(): HTMLElement | SVGElement { return this.input; } - /** @returns The flyout's workspace. */ + /** + * See IFocusableNode.getFocusableTree. + * + * @returns The flyout's workspace, not the main one: that is what stops + * focusing the field reading as leaving the toolbox. + */ getFocusableTree(): Blockly.IFocusableTree { return this.flyout.getWorkspace(); } /** - * @returns Whether the field is on screen. A hidden field can't be focused, - * and this is also what keeps it out of other categories' flyouts. + * See IFocusableNode.canBeFocused. + * + * @returns Whether its own category's flyout is the one on screen. This is + * what keeps the field out of other categories' flyouts, since the + * navigator it installs serves all of them. */ canBeFocused(): boolean { - return this.input.parentElement?.style.display !== 'none'; + return ( + this.flyout.isVisible() && + this.workspace.getToolbox()?.getSelectedItem() === this.category + ); } /** See IFocusableNode.onNodeFocus. */ @@ -48,4 +177,86 @@ export class SearchFieldNode /** See IFocusableNode.onNodeBlur. */ onNodeBlur(): void {} + + /** + * Shows or hides the field, which also governs whether it can be focused. + * + * @param visible Whether the field should be shown. + */ + setVisible(visible: boolean) { + this.foreignObject.style.display = visible ? 'block' : 'none'; + } + + /** + * Returns the bounding rectangle of the UI element in pixel units relative to + * the Blockly injection div. + * + * @returns The component's bounding box. Null in this case since we don't need + * other elements to avoid the toolbox search field. + */ + getBoundingRectangle(): Blockly.utils.Rect | null { + return null; + } + + /** + * Aligns the field with the top edge of the flyout. + */ + position() { + const flyout = this.flyout; + if (!flyout.isVisible()) return; + + const margin = flyout.MARGIN; + // The flyout's own group is the origin, so getX()/getY() drop out. + let left = margin; + let width = FIELD_WIDTH; + + if (flyout.horizontalLayout) { + if (this.workspace.RTL) left = flyout.getWidth() - margin - width; + } else { + // Fit the search field within the flyout, accounting for the scrollbar. + const scrollbarInset = + Blockly.Scrollbar.scrollbarThickness + flyout.SCROLLBAR_MARGIN; + if (this.workspace.RTL) left += scrollbarInset; + width = Math.max(flyout.getWidth() - 2 * margin - scrollbarInset, 0); + } + + this.foreignObject.setAttribute('x', `${left}`); + this.foreignObject.setAttribute('y', `${margin}`); + this.foreignObject.setAttribute('width', `${width}`); + this.foreignObject.setAttribute('height', `${FIELD_HEIGHT}`); + } + + /** + * Returns the room the flyout should reserve at its head for this field. + * + * @returns The size of the field along the flyout's axis, in workspace units. + */ + getRequiredGap(): number { + const flyout = this.flyout; + const scale = flyout.getWorkspace().scale || 1; + // Leave the flyout's own margin below the field as well as room for the + // field itself. + const clearance = flyout.MARGIN; + const size = flyout.horizontalLayout ? FIELD_WIDTH : FIELD_HEIGHT; + return (size + clearance) / scale; + } + + /** + * Removes the field and unregisters it from both component managers. + */ + dispose() { + for (const event of this.boundEvents) { + Blockly.browserEvents.unbind(event); + } + this.boundEvents.length = 0; + this.workspace.getComponentManager().removeComponent(this.id); + this.flyout.getWorkspace().getComponentManager().removeComponent(this.id); + this.foreignObject.remove(); + } +} + +Blockly.Css.register(` +.blocklyToolboxSearchField>input { + width: 100%; } +`); diff --git a/packages/plugins/toolbox-search/src/search_flyout_navigator.ts b/packages/plugins/toolbox-search/src/search_flyout_navigator.ts index 4bb3d60d1f7..0237614e78a 100644 --- a/packages/plugins/toolbox-search/src/search_flyout_navigator.ts +++ b/packages/plugins/toolbox-search/src/search_flyout_navigator.ts @@ -6,6 +6,7 @@ import * as Blockly from 'blockly/core'; import type {SearchFieldNode} from './search_field_node'; +import {SearchFieldNavigationPolicy} from './search_field_navigation_policy'; /** * Makes the search field the flyout's entry point, so that moving in from @@ -17,6 +18,8 @@ import type {SearchFieldNode} from './search_field_node'; * for the field's cursor. The match results can be reached with enter instead. */ export class SearchFlyoutNavigator extends Blockly.FlyoutNavigator { + private readonly policy: SearchFieldNavigationPolicy; + /** * @param flyout The flyout being navigated. * @param searchNode The search field's focusable node. @@ -26,55 +29,80 @@ export class SearchFlyoutNavigator extends Blockly.FlyoutNavigator { private readonly searchNode: SearchFieldNode, ) { super(flyout); + this.policy = new SearchFieldNavigationPolicy(flyout); + this.addNavigationPolicy(this.policy); } - /** - * @returns Whether the search field takes part in arrow-key navigation. - */ - private cyclesThroughSearchField(): boolean { - return !this.flyout.horizontalLayout && this.searchNode.canBeFocused(); - } - - /** @returns The flyout's items, not counting the search field. */ - private getFlyoutItems(): Blockly.IFocusableNode[] { - return super.getTopLevelItems(); - } - - /** @returns The first result, or null if the search matched nothing. */ - getFirstItem(): Blockly.IFocusableNode | null { - return this.getFlyoutItems()[0] ?? null; - } - - /** @returns The last result, or null if the search matched nothing. */ - getLastItem(): Blockly.IFocusableNode | null { - const items = this.getFlyoutItems(); - return items[items.length - 1] ?? null; - } - - /** @returns The items to cycle through with the arrow keys. */ + /** Returns a list of top-level navigable flyout items, plus the search field. */ protected override getTopLevelItems(): Blockly.IFocusableNode[] { - const items = this.getFlyoutItems(); - return this.cyclesThroughSearchField() + const items = super.getTopLevelItems(); + return this.policy.isNavigable(this.searchNode) ? [this.searchNode, ...items] : items; } /** - * @param node The node to check. - * @returns Whether the node can be navigated to. The base implementation - * only admits the flyout's own contents. + * Returns whether or not the given node is navigable. + * + * @param node A focusable node to check the navigability of. + * @returns True if the node is navigable, otherwise false. */ protected override isNavigable(node: Blockly.IFocusableNode) { - if (node === this.searchNode) return this.cyclesThroughSearchField(); + // The field is never a flyout item, so we check its own policy. + if (node === this.searchNode) { + return this.policy.isNavigable(this.searchNode); + } + // The placeholder text label is not navigable. + if (node instanceof Blockly.FlyoutButton && node.isLabel()) return false; return super.isNavigable(node); } /** - * @returns The search field, so that arrowing in from the toolbox lands - * there whichever way the flyout is laid out. + * Returns whether the search field is where arrowing in from the toolbox + * should land, for whichever of the two methods below is asking. + * + * @param requestLastEntry Whether the caller is asking for the last entry + * point (true) or the first (false). + * @returns Whether that caller should return the search field. False + * whenever the field is hidden, since this navigator serves every + * category's flyout, not just the search category's. + */ + private isHorizontalEntryPoint(requestLastEntry: boolean): boolean { + // A vertical flyout puts the field first in getTopLevelItems(), so the + // inherited getFirstNode() returns it already, and getLastNode() keeps + // its ordinary meaning, which is what vertical ArrowUp relies on to reach + // the last result. + if (!this.flyout.horizontalLayout) return false; + if (!this.searchNode.canBeFocused()) return false; + + // getInNode() takes the flyout's last node in an RTL horizontal toolbox + // and its first node otherwise. + const entryPointIsLast = !!this.flyout.targetWorkspace?.RTL; + return requestLastEntry === entryPointIsLast; + } + + /** + * Gets the first navigable node on the flyout, or null if none exist. + * + * @returns The search field in an LTR horizontal flyout, where it is the + * entry point but not part of the arrow-key cycle; otherwise the first + * item, which in a vertical flyout is the field itself. */ override getFirstNode(): Blockly.IFocusableNode | null { - if (this.searchNode.canBeFocused()) return this.searchNode; - return super.getFirstNode(); + return this.isHorizontalEntryPoint(false) + ? this.searchNode + : super.getFirstNode(); + } + + /** + * Gets the last navigable node on the flyout, or null if none exist. + * + * @returns The search field in an RTL horizontal flyout, which is the entry + * point getInNode() asks for there; otherwise the last item. + */ + override getLastNode(): Blockly.IFocusableNode | null { + return this.isHorizontalEntryPoint(true) + ? this.searchNode + : super.getLastNode(); } } diff --git a/packages/plugins/toolbox-search/src/toolbox_search.ts b/packages/plugins/toolbox-search/src/toolbox_search.ts index 64c84adf10c..594f7d1e557 100644 --- a/packages/plugins/toolbox-search/src/toolbox_search.ts +++ b/packages/plugins/toolbox-search/src/toolbox_search.ts @@ -15,40 +15,17 @@ import {SearchFlyoutNavigator} from './search_flyout_navigator'; /* eslint-disable @typescript-eslint/naming-convention */ -/** - * Padding between a flyout's scrollbar and the edge of the flyout. Mirrors - * Flyout.SCROLLBAR_MARGIN, which is not part of the IFlyout interface. - */ -const SCROLLBAR_MARGIN = 2.5; - /** * A toolbox category that provides a search field and displays matching blocks * in its flyout. - * - * The search field is deliberately not a flyout item. Typing into it rebuilds - * the flyout's contents, which would dispose the field along with everything - * else. Instead the category registers itself as an IPositionable and pins the - * field to the flyout's top edge, reserving room for it with a separator at - * the head of the flyout. */ -export class ToolboxSearchCategory - extends Blockly.ToolboxCategory - implements Blockly.IPositionable -{ +export class ToolboxSearchCategory extends Blockly.ToolboxCategory { private static readonly START_SEARCH_SHORTCUT = 'startSearch'; static readonly SEARCH_CATEGORY_KIND = 'search'; - /** - * Identifies the search field to the workspace's ComponentManager. - * IComponent requires this exact name; it is not the toolbox item's ID, - * which is getId(). - */ - readonly id: string; - private searchFieldDiv?: HTMLDivElement; - private searchField?: HTMLInputElement; private searchNode?: SearchFieldNode; private flyoutNavigator?: SearchFlyoutNavigator; /** The flyout this category searches into. */ - private readonly flyout: Blockly.IFlyout; + private readonly flyout: Blockly.Flyout; private blockSearcher: BlockSearcher; private onChangeWrapper?: (event: Blockly.Events.Abstract) => void; private indexedBlocks = ''; @@ -71,13 +48,10 @@ export class ToolboxSearchCategory ) { super(categoryDef, parentToolbox, opt_parent); const flyout = parentToolbox.getFlyout(); - if (!flyout) { + if (!(flyout instanceof Blockly.Flyout)) { throw Error('A search category needs a toolbox that has a flyout.'); } this.flyout = flyout; - // Suffixed with the toolbox item's ID so that two workspaces on a page - // don't share the same ID. - this.id = `toolbox-search-input-${this.getId()}`; this.blockSearcher = new BlockSearcher(this.workspace_); this.initBlockSearcher(); this.registerShortcut(); @@ -90,7 +64,18 @@ export class ToolboxSearchCategory */ override init() { super.init(); - const searchField = this.createSearchField(); + const node = new SearchFieldNode( + `toolbox-search-input-${this.getId()}`, + this.workspace_, + this.flyout, + this, + ); + this.searchNode = node; + // Make the field the node that arrowing into the flyout lands on. + this.flyoutNavigator = new SearchFlyoutNavigator(this.flyout, node); + this.flyout.getWorkspace().setNavigator(this.flyoutNavigator); + + const searchField = node.getInput(); this.boundEvents.push( Blockly.browserEvents.conditionalBind( searchField, @@ -102,6 +87,11 @@ export class ToolboxSearchCategory // they belong to the cursor. (event: KeyboardEvent) => { const isHorizontal = this.flyout.horizontalLayout; + const leaveKey = + this.workspace_.options.toolboxPosition === + Blockly.utils.toolbox.Position.BOTTOM + ? 'ArrowDown' + : 'ArrowUp'; let handled = true; if (event.key === 'Escape') { Blockly.getFocusManager().focusTree(this.workspace_); @@ -111,14 +101,8 @@ export class ToolboxSearchCategory this.focusResult(); } else if (!isHorizontal && event.key === 'ArrowUp') { this.focusResult(true); - } else if (isHorizontal) { - const isBottom = - this.workspace_.options.toolboxPosition === - Blockly.utils.toolbox.Position.BOTTOM; - const leaveKey = isBottom ? 'ArrowDown' : 'ArrowUp'; - if (event.key === leaveKey) { - Blockly.getFocusManager().focusNode(this); - } + } else if (isHorizontal && event.key === leaveKey) { + Blockly.getFocusManager().focusNode(this); } else { handled = false; } @@ -135,60 +119,6 @@ export class ToolboxSearchCategory ); } - /** - * Builds the search field and pins it over the flyout. - * - * @returns The search input. - */ - private createSearchField(): HTMLInputElement { - const input = document.createElement('input'); - input.id = this.id; - input.type = 'search'; - input.placeholder = Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER']; - Blockly.utils.aria.setState( - input, - Blockly.utils.aria.State.LABEL, - Blockly.Msg['TOOLBOX_SEARCH_PLACEHOLDER'], - ); - this.searchField = input; - - const div = document.createElement('div'); - div.classList.add('blocklyToolboxSearchField'); - div.appendChild(input); - this.searchFieldDiv = div; - this.setFieldVisible(false); - - // Match the flyout's colour, so blocks scrolling past go behind it. - // Themes without a flyout colour fall back to the one in the CSS below. - this.workspace_ - .getThemeManager() - .subscribe(div, 'flyoutBackgroundColour', 'background-color'); - this.workspace_ - .getInjectionDiv() - .insertBefore(div, this.workspace_.getParentSvg()); - this.workspace_.getComponentManager().addComponent({ - component: this, - weight: 0, - capabilities: [Blockly.ComponentManager.Capability.POSITIONABLE], - }); - - // Make the field a focusable node of the flyout's workspace, and the node - // that arrowing into the flyout lands on. - const flyout = this.flyout; - this.searchNode = new SearchFieldNode(this.id, input, flyout); - this.flyoutNavigator = new SearchFlyoutNavigator(flyout, this.searchNode); - flyout - .getWorkspace() - .getComponentManager() - .addComponent({ - component: this.searchNode, - weight: 0, - capabilities: [Blockly.ComponentManager.Capability.FOCUSABLE], - }); - flyout.getWorkspace().setNavigator(this.flyoutNavigator); - return input; - } - /** * Moves focus to one end of the results. * @@ -197,63 +127,15 @@ export class ToolboxSearchCategory private focusResult(getLast = false) { if (!this.flyoutNavigator) return; const result = getLast - ? this.flyoutNavigator.getLastItem() - : this.flyoutNavigator.getFirstItem(); + ? this.flyoutNavigator.getLastNode() + : this.flyoutNavigator.getFirstChild( + this.flyout.getWorkspace().getRootFocusableNode(), + ); if (result) { Blockly.getFocusManager().focusNode(result); } } - /** - * Shows or hides the search field. - * - * @param visible Whether the field should be shown. - */ - private setFieldVisible(visible: boolean) { - if (this.searchFieldDiv) { - this.searchFieldDiv.style.display = visible ? 'block' : 'none'; - } - } - - /** - * Returns the bounding rectangle of the UI element in pixel units relative to - * the Blockly injection div. - * - * @returns The component's bounding box. Null in this case since we don't need - * other elements to avoid the toolbox search field. - */ - getBoundingRectangle(): Blockly.utils.Rect | null { - return null; - } - - /** - * Aligns the search field with the top edge of the flyout. - */ - position() { - const div = this.searchFieldDiv; - const flyout = this.flyout; - if (!div || !flyout.isVisible()) return; - - const margin = flyout.MARGIN; - let left = flyout.getX() + margin; - - if (flyout.horizontalLayout) { - // The flyout spans the workspace and its scrollbar runs along the - // bottom, so there is nothing to size the field against. - div.style.width = ''; - } else { - // Fit the search field within the flyout, accounting for the scrollbar. - const scrollbarInset = - Blockly.Scrollbar.scrollbarThickness + SCROLLBAR_MARGIN; - if (this.workspace_.RTL) left += scrollbarInset; - const width = flyout.getWidth() - 2 * margin - scrollbarInset; - div.style.width = `${Math.max(width, 0)}px`; - } - - div.style.left = `${left}px`; - div.style.top = `${flyout.getY() + margin}px`; - } - /** * Registers a shortcut for displaying the toolbox search category. */ @@ -340,19 +222,16 @@ export class ToolboxSearchCategory */ override setSelected(isSelected: boolean) { super.setSelected(isSelected); - this.setFieldVisible(isSelected); - if (!isSelected) { - // The field belongs to the flyout's workspace but sits outside its SVG, - // so core's own cleanup can't find the indicator to remove. - this.searchField?.classList.remove('blocklyPassiveFocus'); - } + this.searchNode?.setVisible(isSelected); } /** * Filters the available blocks based on the current query string. */ private matchBlocks() { - const query = this.searchField?.value || ''; + const node = this.searchNode; + const field = node?.getInput(); + const query = field?.value || ''; const results = query ? this.blockSearcher.blockTypesMatching(query) : []; const oldCount = this.resultCount; @@ -376,44 +255,30 @@ export class ToolboxSearchCategory // Prevent losing the cursor position in the search field when the flyout is rebuilt. const cursorToRestore = - this.searchField && - Blockly.getFocusManager().getFocusedNode() === this.searchNode + field && node && Blockly.getFocusManager().getFocusedNode() === node ? { - start: this.searchField.selectionStart, - end: this.searchField.selectionEnd, + field, + node, + start: field.selectionStart, + end: field.selectionEnd, } : null; this.parentToolbox_.refreshSelection(); + // Start each new set of results from the best match rather than from + // wherever the previous query left the flyout scrolled. + this.flyout.scrollToStart(); - if (cursorToRestore && this.searchField && this.searchNode) { - Blockly.getFocusManager().focusNode(this.searchNode); - this.searchField.setSelectionRange( + if (cursorToRestore) { + Blockly.getFocusManager().focusNode(cursorToRestore.node); + cursorToRestore.field.setSelectionRange( cursorToRestore.start, cursorToRestore.end, ); } // Showing the flyout may have changed its width, and that path doesn't // run positionable components. - this.position(); - } - - /** - * Returns the room to reserve at the head of the flyout for the search field. - * - * @returns The height of the search field, in workspace units. - */ - private spacerGap(): number { - const flyout = this.flyout; - const scale = flyout.getWorkspace().scale || 1; - // Leave the flyout's own margin below the field as well as room for the - // field itself. - const clearance = flyout.MARGIN; - const size = - (flyout.horizontalLayout - ? this.searchFieldDiv?.offsetWidth - : this.searchFieldDiv?.offsetHeight) ?? 0; - return (size + clearance) / scale; + this.searchNode?.position(); } /** @@ -431,7 +296,7 @@ export class ToolboxSearchCategory results: Blockly.utils.toolbox.BlockInfo[], ): Blockly.utils.toolbox.FlyoutItemInfoArray { const items: Blockly.utils.toolbox.FlyoutItemInfoArray = [ - {kind: 'sep', gap: this.spacerGap()}, + {kind: 'sep', gap: this.searchNode?.getRequiredGap() ?? 0}, ]; if (results.length) { @@ -457,17 +322,8 @@ export class ToolboxSearchCategory Blockly.browserEvents.unbind(event); } this.boundEvents.length = 0; - if (this.searchFieldDiv) { - this.workspace_.getThemeManager().unsubscribe(this.searchFieldDiv); - this.workspace_.getComponentManager().removeComponent(this.id); - this.searchFieldDiv.remove(); - this.searchFieldDiv = undefined; - } - if (this.searchNode) { - this.flyout.getWorkspace().getComponentManager().removeComponent(this.id); - this.searchNode = undefined; - } - this.searchField = undefined; + this.searchNode?.dispose(); + this.searchNode = undefined; if (this.onChangeWrapper) { this.workspace_.removeChangeListener(this.onChangeWrapper); this.onChangeWrapper = undefined; @@ -525,21 +381,6 @@ export class ToolboxSearchCategory } } -Blockly.Css.register(` -.blocklyToolboxSearchField { - /* Matches the default fill of .blocklyFlyoutBackground. - A theme that sets flyoutBackgroundColour overrides this. */ - background-color: #ddd; - position: absolute; - /* Matches the z-index of .blocklyFlyout. */ - z-index: 20; -} - -.blocklyToolboxSearchField>input { - width: 100%; -} -`); - Blockly.registry.register( Blockly.registry.Type.TOOLBOX_ITEM, ToolboxSearchCategory.SEARCH_CATEGORY_KIND, diff --git a/packages/plugins/toolbox-search/test/tests.mocha.js b/packages/plugins/toolbox-search/test/tests.mocha.js index 773f038ccde..95bec6e128e 100644 --- a/packages/plugins/toolbox-search/test/tests.mocha.js +++ b/packages/plugins/toolbox-search/test/tests.mocha.js @@ -55,6 +55,7 @@ suite('ToolboxSearchCategory', () => { .getToolbox() .getToolboxItems() .find((item) => item instanceof ToolboxSearchCategory); + this.searchField = this.searchCategory.searchNode.getInput(); /** * Types into the search field the way a user would, so the category's own @@ -62,7 +63,7 @@ suite('ToolboxSearchCategory', () => { * @param {string} query The text to search for. */ this.search = (query) => { - const field = this.searchCategory.searchField; + const field = this.searchField; this.searchCategory .getParentToolbox() .setSelectedItem(this.searchCategory); @@ -143,7 +144,7 @@ suite('ToolboxSearchCategory', () => { test('shows and hides the search field with the flyout', function () { const isSearchFieldVisible = () => - this.searchCategory.searchField.parentElement.style.display !== 'none'; + this.searchField.parentElement.parentElement.style.display !== 'none'; const toolbox = this.workspace.getToolbox(); assert.isFalse(isSearchFieldVisible()); @@ -161,19 +162,6 @@ suite('ToolboxSearchCategory', () => { assert.isFalse(isSearchFieldVisible()); }); - test('builds the same row as an ordinary category', function () { - const [logic] = this.workspace.getToolbox().getToolboxItems(); - const rowClasses = (category) => - [...category.getDiv().querySelectorAll('*')].map((el) => el.className); - - assert.deepEqual(rowClasses(this.searchCategory), rowClasses(logic)); - assert.equal( - this.searchCategory.getDiv().querySelector('.blocklyToolboxCategoryLabel') - .textContent, - 'Search', - ); - }); - test('remembers its query while the flyout is hidden', function () { const toolbox = this.workspace.getToolbox(); this.search('controls if'); @@ -183,7 +171,7 @@ suite('ToolboxSearchCategory', () => { toolbox.setSelectedItem(this.searchCategory); this.clock.runAll(); - assert.equal(this.searchCategory.searchField.value, 'controls if'); + assert.equal(this.searchField.value, 'controls if'); assert.deepEqual(flyoutBlockTypes(this.workspace), ['controls_if']); }); });