From 3cc48222c30afc581c1057e9be0d1717fdd146f9 Mon Sep 17 00:00:00 2001 From: Constantin Iliescu Date: Fri, 23 Feb 2024 16:51:00 +0100 Subject: [PATCH] Fix: When using the toolbox-search, auto select the found dropdown item Fixes: https://github.com/google/blockly-samples/issues/1940 Uses the Block State as suggested here: https://github.com/google/blockly-samples/pull/2004#issuecomment-1832317907 Fixes: bug that didn't indexed the last trigram from a word --- plugins/toolbox-search/CHANGELOG.md | 6 + plugins/toolbox-search/src/block_searcher.ts | 68 ++++-- plugins/toolbox-search/src/toolbox_search.ts | 19 +- plugins/toolbox-search/test/tests.mocha.js | 230 ++++++++++++++++++- 4 files changed, 291 insertions(+), 32 deletions(-) diff --git a/plugins/toolbox-search/CHANGELOG.md b/plugins/toolbox-search/CHANGELOG.md index 035360107a..d6e1640915 100644 --- a/plugins/toolbox-search/CHANGELOG.md +++ b/plugins/toolbox-search/CHANGELOG.md @@ -3,6 +3,12 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [1.2.7](https://github.com/google/blockly-samples/compare/@blockly/toolbox-search@1.2.6...@blockly/toolbox-search@1.2.7) (2024-02-23) +### Bug Fixes + * Improved search for drop down + * Improved unit tests + * Last trigram from a word was never indexed + ## [1.2.6](https://github.com/google/blockly-samples/compare/@blockly/toolbox-search@1.2.5...@blockly/toolbox-search@1.2.6) (2024-02-08) **Note:** Version bump only for package @blockly/toolbox-search diff --git a/plugins/toolbox-search/src/block_searcher.ts b/plugins/toolbox-search/src/block_searcher.ts index d7c5d96ed7..b59f6c74ba 100644 --- a/plugins/toolbox-search/src/block_searcher.ts +++ b/plugins/toolbox-search/src/block_searcher.ts @@ -27,13 +27,16 @@ export class BlockSearcher { const blockCreationWorkspace = new Blockly.Workspace(); blockTypes.forEach((blockType) => { const block = blockCreationWorkspace.newBlock(blockType); - this.indexBlockText(blockType.replaceAll('_', ' '), blockType); - block.inputList.forEach((input) => { - input.fieldRow.forEach((field) => { - this.indexDropdownOption(field, blockType); - this.indexBlockText(field.getText(), blockType); + const blockState = Blockly.serialization.blocks.save(block); + if (blockState != null) { + this.indexBlockText(blockType.replaceAll('_', ' '), blockState); + block.inputList.forEach((input) => { + input.fieldRow.forEach((field) => { + this.indexDropdownOption(field, blockState); + this.indexBlockText(field.getText(), blockState); + }); }); - }); + } }); } @@ -41,15 +44,27 @@ export class BlockSearcher { * Check if the field is a dropdown, and index every text in the option * * @param field We need to check the type of field - * @param blockType The block type to associate the trigrams with. + * @param blockState The block state to associate the trigrams with. */ - private indexDropdownOption(field: Blockly.Field, blockType: string) { + private indexDropdownOption( + field: Blockly.Field, + blockState: Blockly.serialization.blocks.State, + ) { if (field instanceof Blockly.FieldDropdown) { field.getOptions(true).forEach((option) => { + const state = {...blockState}; + state.fields = {...blockState.fields}; if (typeof option[0] === 'string') { - this.indexBlockText(option[0], blockType); + if (state.fields == undefined) { + state.fields = {}; + } + if (field.name) { + state.fields[field.name] = option[1]; + } + this.indexBlockText(option[0], state); + this.indexBlockText(option[1], state); } else if ('alt' in option[0]) { - this.indexBlockText(option[0].alt, blockType); + this.indexBlockText(option[0].alt, state); } }); } @@ -59,10 +74,10 @@ export class BlockSearcher { * Filters the available blocks based on the current query string. * * @param query The text to use to match blocks against. - * @returns A list of block types matching the query. + * @returns A list of block states matching the query. */ - blockTypesMatching(query: string): string[] { - return [ + blockTypesMatching(query: string): Blockly.serialization.blocks.State[] { + const result = [ ...this.generateTrigrams(query) .map((trigram) => { return this.trigramsToBlocks.get(trigram) ?? new Set(); @@ -72,20 +87,33 @@ export class BlockSearcher { }) .values(), ]; + const resultState = result.map((item) => JSON.parse(item)); + return resultState; + } + + private addBlockTrigram(trigram: string, blockState: string) { + const blockSet = this.trigramsToBlocks.get(trigram) ?? new Set(); + blockSet.add(blockState); + this.trigramsToBlocks.set(trigram, blockSet); } /** * Generates trigrams for the given text and associates them with the given - * block type. + * block state. * * @param text The text to generate trigrams of. - * @param blockType The block type to associate the trigrams with. + * @param blockState The block state to associate the trigrams with. */ - private indexBlockText(text: string, blockType: string) { + private indexBlockText( + text: string, + blockState: Blockly.serialization.blocks.State, + ) { + blockState.id = undefined; + blockState.extraState = undefined; + blockState.data = undefined; + const stateString = JSON.stringify(blockState); this.generateTrigrams(text).forEach((trigram) => { - const blockSet = this.trigramsToBlocks.get(trigram) ?? new Set(); - blockSet.add(blockType); - this.trigramsToBlocks.set(trigram, blockSet); + this.addBlockTrigram(trigram, stateString); }); } @@ -101,7 +129,7 @@ export class BlockSearcher { if (normalizedInput.length <= 3) return [normalizedInput]; const trigrams: string[] = []; - for (let start = 0; start < normalizedInput.length - 3; start++) { + for (let start = 0; start <= normalizedInput.length - 3; start++) { trigrams.push(normalizedInput.substring(start, start + 3)); } diff --git a/plugins/toolbox-search/src/toolbox_search.ts b/plugins/toolbox-search/src/toolbox_search.ts index 2c9bc17810..e07b40efbb 100644 --- a/plugins/toolbox-search/src/toolbox_search.ts +++ b/plugins/toolbox-search/src/toolbox_search.ts @@ -9,6 +9,7 @@ * in its flyout. */ import * as Blockly from 'blockly/core'; +import {block} from '../node_modules/blockly/core/tooltip'; import {BlockSearcher} from './block_searcher'; /* eslint-disable @typescript-eslint/naming-convention */ @@ -171,13 +172,19 @@ export class ToolboxSearchCategory extends Blockly.ToolboxCategory { */ private matchBlocks() { const query = this.searchField?.value || ''; - this.flyoutItems_ = query - ? this.blockSearcher.blockTypesMatching(query).map((blockType) => { - return { - kind: 'block', - type: blockType, - }; + ? this.blockSearcher.blockTypesMatching(query).map((blockState) => { + if (blockState.fields) { + return { + kind: 'block', + type: blockState.type, + fields: blockState.fields, + }; + } else + return { + kind: 'block', + type: blockState.type, + }; }) : []; diff --git a/plugins/toolbox-search/test/tests.mocha.js b/plugins/toolbox-search/test/tests.mocha.js index e4248122e3..289696352f 100644 --- a/plugins/toolbox-search/test/tests.mocha.js +++ b/plugins/toolbox-search/test/tests.mocha.js @@ -14,18 +14,129 @@ suite('Toolbox search', () => { }); }); +function allPossibleCombinations(input, length, curstr) { + if (curstr.length == length) return [curstr]; + const ret = []; + for (let i = 0; i < input.length; i++) { + ret.push(...allPossibleCombinations(input, length, curstr + input[i])); + } + return ret; +} + +function allThreeLetterCombinations() { + const input = [ + 'a', + 'b', + 'c', + 'd', + 'e', + 'f', + 'g', + 'h', + 'i', + 'j', + 'k', + 'l', + 'm', + 'n', + 'o', + 'p', + 'r', + 's', + 't', + 'u', + 'v', + 'z', + 'x', + 'y', + 'z', + 'w', + ]; + return allPossibleCombinations(input, 3, ''); +} + +const searchableBlocks = [ + 'controls_if', + 'logic_compare', + 'logic_operation', + 'logic_negate', + 'logic_boolean', + 'logic_null', + 'logic_ternary', + 'controls_repeat_ext', + 'controls_repeat', + 'controls_whileUntil', + 'controls_for', + 'controls_forEach', + 'controls_flow_statements', + 'math_number', + 'math_arithmetic', + 'math_single', + 'math_trig', + 'math_constant', + 'math_number_property', + 'math_round', + 'math_on_list', + 'math_modulo', + 'math_constrain', + 'math_random_int', + 'math_random_float', + 'math_atan2', + 'text', + 'text_multiline', + 'text_join', + 'text_append', + 'text_length', + 'text_isEmpty', + 'text_indexOf', + 'text_charAt', + 'text_getSubstring', + 'text_changeCase', + 'text_trim', + 'text_count', + 'text_replace', + 'text_reverse', + 'text_print', + 'text_prompt_ext', + 'lists_create_with', + 'lists_repeat', + 'lists_length', + 'lists_isEmpty', + 'lists_indexOf', + 'lists_getIndex', + 'lists_setIndex', + 'lists_getSublist', + 'lists_split', + 'lists_sort', + 'lists_reverse', + 'colour_picker', + 'colour_random', + 'colour_rgb', + 'colour_blend', +]; + suite('BlockSearcher', () => { - test('indexes the default value of dropdown fields', () => { + test('indexes the default value of dropdown fields', async () => { const searcher = new BlockSearcher(); + // Text on these: // lists_sort: sort // lists_split: make with delimiter , searcher.indexBlocks(['lists_sort', 'lists_split']); const numericMatches = searcher.blockTypesMatching('numeric'); - assert.sameMembers(['lists_sort'], numericMatches); + assert.sameMembers( + ['lists_sort'], + numericMatches.map((item) => item.type), + ); + assert.sameMembers( + ['NUMERIC'], + numericMatches.map((item) => item.fields['TYPE']), + ); - const listFromTextMatches = searcher.blockTypesMatching('list from text'); + const listFromTextMatches = searcher + .blockTypesMatching('list from text') + .map((item) => item.type); assert.sameMembers(['lists_split'], listFromTextMatches); }); @@ -33,16 +144,123 @@ suite('BlockSearcher', () => { const searcher = new BlockSearcher(); searcher.indexBlocks(['lists_create_with']); - const lowercaseMatches = searcher.blockTypesMatching('create list'); + const lowercaseMatches = searcher + .blockTypesMatching('create list') + .map((item) => item.type); assert.sameMembers(['lists_create_with'], lowercaseMatches); - const uppercaseMatches = searcher.blockTypesMatching('CREATE LIST'); + const uppercaseMatches = searcher + .blockTypesMatching('CREATE LIST') + .map((item) => item.type); assert.sameMembers(['lists_create_with'], uppercaseMatches); - const ransomNoteMatches = searcher.blockTypesMatching('cReATe LiST'); + const ransomNoteMatches = searcher + .blockTypesMatching('cReATe LiST') + .map((item) => item.type); assert.sameMembers(['lists_create_with'], ransomNoteMatches); }); + test('Check for duplicates', () => { + const searcher = new BlockSearcher(); + + searcher.indexBlocks(searchableBlocks); + + const combinations = allThreeLetterCombinations(); + combinations.forEach((element) => { + const matches = searcher + .blockTypesMatching(element) + .map((item) => JSON.stringify(item)); + const matchesSet = new Set(matches); + assert.equal(matches.length, matchesSet.size); + }); + }); + + test('Check for false negative', () => { + const searcher = new BlockSearcher(); + + searcher.indexBlocks(searchableBlocks); + const stringInfo = {}; + searchableBlocks.forEach((element) => { + let itemString = element; + const blockCreationWorkspace = new Blockly.Workspace(); + const block = blockCreationWorkspace.newBlock(element); + block.inputList.forEach((input) => { + input.fieldRow.forEach((field) => { + if (field.getText()) { + itemString += ' ' + field.getText(); + } + if (field.name) { + itemString += ' ' + field.name; + } + if (field instanceof Blockly.FieldDropdown) { + field.getOptions(true).forEach((option) => { + // For VAR fields the option[1] is generated randpmly each time the block is created, + // we don't want to test for it as we don't count it as a false negative + if (field.name != 'VAR' && option[0] != 'i') + itemString += ' ' + option[1]; + itemString += ' ' + option[0]; + }); + } + }); + }); + stringInfo[element] = itemString; + }); + + const combinations = allThreeLetterCombinations(); + combinations.forEach((element) => { + const matches = searcher.blockTypesMatching(element); + for (const key in stringInfo) { + if (stringInfo[key].includes(element)) { + let found = false; + matches.forEach((match) => { + if (match.type == key) { + found = true; + } + }); + if (!found) { + assert.equal(found, true); + } + } + } + }); + }); + + test('Check for false positives', () => { + const searcher = new BlockSearcher(); + + searcher.indexBlocks(searchableBlocks); + const combinations = allThreeLetterCombinations(); + combinations.forEach((element) => { + const matches = searcher.blockTypesMatching(element); + let checkInString = 0; + matches.forEach((item) => { + const blockCreationWorkspace = new Blockly.Workspace(); + const block = blockCreationWorkspace.newBlock(item.type); + const itemString = JSON.stringify(item); + let blockInputList = ''; + block.inputList.forEach((input) => { + input.fieldRow.forEach((field) => { + blockInputList += ' ' + field.getText(); + if (field instanceof Blockly.FieldDropdown) { + field.getOptions(true).forEach((option) => { + if (item.fields[field.name] == option[1]) { + blockInputList += ' ' + option[0]; + } + }); + } + }); + }); + if ( + !itemString.toLowerCase().includes(element) && + !blockInputList.toLowerCase().includes(element) + ) { + checkInString += 1; + } + }); + assert.equal(checkInString, 0); + }); + }); + test('returns an empty list when no matches are found', () => { const searcher = new BlockSearcher(); assert.isEmpty(searcher.blockTypesMatching('abc123'));