From 9b81317d326ba63f063d333aa9f9d135620fad03 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Thu, 13 Oct 2022 01:30:17 +0200 Subject: [PATCH] chore: Remove blockly factory use of utils.dom.add/removeClass (#6534) * Remove usages of utils.dom.add/removeClass from Blockly Factory * Use template strings for error messages. (Random stuff found while working on something larger.) --- blocks/procedures.js | 3 +- core/block_svg.ts | 4 +- core/shortcut_registry.ts | 21 +- demos/blockfactory/factory.js | 33 +- demos/blockfactory/standard_categories.js | 556 +++++++++--------- .../workspacefactory/wfactory_controller.js | 2 +- .../workspacefactory/wfactory_view.js | 18 +- tests/mocha/blocks/variables_test.js | 4 +- tests/mocha/shortcut_registry_test.js | 10 +- 9 files changed, 325 insertions(+), 326 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index cdb27203121..d6fd9ddcff5 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -150,8 +150,7 @@ const PROCEDURE_DEF_COMMON = { this.argumentVarModels_.push(variable); } else { console.log( - 'Failed to create a variable with name ' + varName + - ', ignoring.'); + `Failed to create a variable named "${varName}", ignoring.`); } } } diff --git a/core/block_svg.ts b/core/block_svg.ts index b4bdd5dfaa6..f0054e8af13 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -980,8 +980,8 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, } /** - * Updates the color of the block (and children) to match the current disabled - * state. + * Updates the colour of the block (and children) to match the current + * disabled state. * * @internal */ diff --git a/core/shortcut_registry.ts b/core/shortcut_registry.ts index 7bb7f035ec0..aba3678874f 100644 --- a/core/shortcut_registry.ts +++ b/core/shortcut_registry.ts @@ -56,8 +56,7 @@ export class ShortcutRegistry { register(shortcut: KeyboardShortcut, opt_allowOverrides?: boolean) { const registeredShortcut = this.shortcuts.get(shortcut.name); if (registeredShortcut && !opt_allowOverrides) { - throw new Error( - 'Shortcut with name "' + shortcut.name + '" already exists.'); + throw new Error(`Shortcut named "${shortcut.name}" already exists.`); } this.shortcuts.set(shortcut.name, shortcut); @@ -81,8 +80,7 @@ export class ShortcutRegistry { const shortcut = this.shortcuts.get(shortcutName); if (!shortcut) { - console.warn( - 'Keyboard shortcut with name "' + shortcutName + '" not found.'); + console.warn(`Keyboard shortcut named "${shortcutName}" not found.`); return false; } @@ -110,9 +108,8 @@ export class ShortcutRegistry { keyCode = String(keyCode); const shortcutNames = this.keyMap.get(keyCode); if (shortcutNames && !opt_allowCollision) { - throw new Error( - 'Shortcut with name "' + shortcutName + '" collides with shortcuts ' + - shortcutNames.toString()); + throw new Error(`Shortcut named "${ + shortcutName}" collides with shortcuts "${shortcutNames}"`); } else if (shortcutNames && opt_allowCollision) { shortcutNames.unshift(shortcutName); } else { @@ -138,9 +135,8 @@ export class ShortcutRegistry { if (!shortcutNames) { if (!opt_quiet) { - console.warn( - 'No keyboard shortcut with name "' + shortcutName + - '" registered with key code "' + keyCode + '"'); + console.warn(`No keyboard shortcut named "${ + shortcutName}" registered with key code "${keyCode}"`); } return false; } @@ -154,9 +150,8 @@ export class ShortcutRegistry { return true; } if (!opt_quiet) { - console.warn( - 'No keyboard shortcut with name "' + shortcutName + - '" registered with key code "' + keyCode + '"'); + console.warn(`No keyboard shortcut named "${ + shortcutName}" registered with key code "${keyCode}"`); } return false; } diff --git a/demos/blockfactory/factory.js b/demos/blockfactory/factory.js index 05832e2f1af..6acdbee82f2 100644 --- a/demos/blockfactory/factory.js +++ b/demos/blockfactory/factory.js @@ -65,18 +65,25 @@ BlockFactory.updateBlocksFlagDelayed = false; */ BlockFactory.STARTER_BLOCK_XML_TEXT = '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '230' + - ''; + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '230' + + '' + + '' + + '' + + ''; /** * Change the language code format. @@ -328,4 +335,4 @@ BlockFactory.manualEdit = function() { BlockFactory.updateBlocksFlag = true; BlockFactory.updateBlocksFlagDelayed = true; BlockFactory.updateLanguage(); -} +}; diff --git a/demos/blockfactory/standard_categories.js b/demos/blockfactory/standard_categories.js index 45ee7e24b26..eefadf2b9a2 100644 --- a/demos/blockfactory/standard_categories.js +++ b/demos/blockfactory/standard_categories.js @@ -29,13 +29,13 @@ StandardCategories.categoryMap['logic'] = StandardCategories.categoryMap['logic'].xml = Blockly.Xml.textToDom( '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + ''); StandardCategories.categoryMap['logic'].hue = 210; @@ -44,33 +44,33 @@ StandardCategories.categoryMap['loops'] = StandardCategories.categoryMap['loops'].xml = Blockly.Xml.textToDom( '' + - '' + - '' + - '' + - '10' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '1' + - '' + - '' + - '' + - '' + - '10' + - '' + - '' + - '' + - '' + - '1' + - '' + - '' + - '' + - '' + - '' + + '' + + '' + + '' + + '10' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '10' + + '' + + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '' + ''); StandardCategories.categoryMap['loops'].hue = 120; @@ -79,91 +79,91 @@ StandardCategories.categoryMap['math'] = StandardCategories.categoryMap['math'].xml = Blockly.Xml.textToDom( '' + - '' + - '' + - '' + - '' + - '1' + - '' + - '' + - '' + - '' + - '1' + - '' + - '' + - '' + - '' + - '' + - '' + - '9' + - '' + - '' + - '' + - '' + - '' + - '' + - '45' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '0' + - '' + - '' + - '' + - '' + - '' + - '' + - '3.1' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '64' + - '' + - '' + - '' + - '' + - '10'+ - '' + - '' + - '' + - '' + - '' + - '' + - '50' + - '' + - '' + - '' + - '' + - '1' + - '' + - '' + - '' + - '' + - '100' + - '' + - '' + - '' + - '' + - '' + - '' + - '1' + - '' + - '' + - '' + - '' + - '100' + - '' + - '' + - '' + - '' + + '' + + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '' + + '' + + '9' + + '' + + '' + + '' + + '' + + '' + + '' + + '45' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '0' + + '' + + '' + + '' + + '' + + '' + + '' + + '3.1' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '64' + + '' + + '' + + '' + + '' + + '10'+ + '' + + '' + + '' + + '' + + '' + + '' + + '50' + + '' + + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '100' + + '' + + '' + + '' + + '' + + '' + + '' + + '1' + + '' + + '' + + '' + + '' + + '100' + + '' + + '' + + '' + + '' + ''); StandardCategories.categoryMap['math'].hue = 230; @@ -172,81 +172,81 @@ StandardCategories.categoryMap['text'] = StandardCategories.categoryMap['text'].xml = Blockly.Xml.textToDom( '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - 'abc' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - 'text' + - '' + - '' + - '' + - '' + - 'abc' + - '' + - '' + - '' + - '' + - '' + - '' + - 'text' + - '' + - '' + - '' + - '' + - '' + - '' + - 'text' + - '' + - '' + - '' + - '' + - '' + - '' + - 'abc' + - '' + - '' + - '' + - '' + - '' + - '' + - 'abc' + - '' + - '' + - '' + - '' + - '' + - '' + - 'abc' + - '' + - '' + - '' + - '' + - '' + - '' + - 'abc' + - '' + - '' + - '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + 'abc' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + 'text' + + '' + + '' + + '' + + '' + + 'abc' + + '' + + '' + + '' + + '' + + '' + + '' + + 'text' + + '' + + '' + + '' + + '' + + '' + + '' + + 'text' + + '' + + '' + + '' + + '' + + '' + + '' + + 'abc' + + '' + + '' + + '' + + '' + + '' + + '' + + 'abc' + + '' + + '' + + '' + + '' + + '' + + '' + + 'abc' + + '' + + '' + + '' + + '' + + '' + + '' + + 'abc' + + '' + + '' + + '' + ''); StandardCategories.categoryMap['text'].hue = 160; @@ -255,55 +255,55 @@ StandardCategories.categoryMap['lists'] = StandardCategories.categoryMap['lists'].xml = Blockly.Xml.textToDom( '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '5' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - '' + - 'list' + - '' + - '' + - '' + - '' + - '' + - '' + - 'list' + - '' + - '' + - '' + - '' + - '' + - '' + - 'list' + - '' + - '' + - '' + - '' + - '' + - '' + - 'list' + - '' + - '' + - '' + - '' + - '' + - '' + - ',' + - '' + - '' + - '' + - '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '5' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + 'list' + + '' + + '' + + '' + + '' + + '' + + '' + + 'list' + + '' + + '' + + '' + + '' + + '' + + '' + + 'list' + + '' + + '' + + '' + + '' + + '' + + '' + + 'list' + + '' + + '' + + '' + + '' + + '' + + '' + + ',' + + '' + + '' + + '' + + '' + ''); StandardCategories.categoryMap['lists'].hue = 260; @@ -312,42 +312,42 @@ StandardCategories.categoryMap['colour'] = StandardCategories.categoryMap['colour'].xml = Blockly.Xml.textToDom( '' + - '' + - '' + - '' + - '' + - '' + - '100' + - '' + - '' + - '' + - '' + - '50' + - '' + - '' + - '' + - '' + - '0' + - '' + - '' + - '' + - '' + - '' + - '' + - '#ff0000' + - '' + - '' + - '' + - '' + - '#3333ff' + + '' + + '' + + '' + + '' + + '' + + '100' + + '' + + '' + + '' + + '' + + '50' + + '' + + '' + + '' + + '' + + '0' + + '' + + '' + + '' + + '' + + '' + + '' + + '#ff0000' + + '' + + '' + + '' + + '' + + '#3333ff' + + '' + + '' + + '' + + '' + + '0.5' + '' + - '' + - '' + - '' + - '0.5' + - '' + - '' + - '' + + '' + + '' + ''); StandardCategories.categoryMap['colour'].hue = 20; diff --git a/demos/blockfactory/workspacefactory/wfactory_controller.js b/demos/blockfactory/workspacefactory/wfactory_controller.js index 149d913528b..73a2a4b417a 100644 --- a/demos/blockfactory/workspacefactory/wfactory_controller.js +++ b/demos/blockfactory/workspacefactory/wfactory_controller.js @@ -606,7 +606,7 @@ WorkspaceFactoryController.prototype.loadCategoryByName = function(name) { // Update the copy in the view. var tab = this.view.addCategoryRow(copy.name, copy.id); this.addClickToSwitch(tab, copy.id); - // Color the category tab in the view. + // Colour the category tab in the view. if (copy.colour) { this.view.setBorderColour(copy.id, copy.colour); } diff --git a/demos/blockfactory/workspacefactory/wfactory_view.js b/demos/blockfactory/workspacefactory/wfactory_view.js index 774bbb8a316..f98b1353049 100644 --- a/demos/blockfactory/workspacefactory/wfactory_view.js +++ b/demos/blockfactory/workspacefactory/wfactory_view.js @@ -10,7 +10,6 @@ * Depends on WorkspaceFactoryController (for adding mouse listeners). Tabs for * each category are stored in tab map, which associates a unique ID for a * category with a particular tab. - * */ @@ -122,7 +121,7 @@ WorkspaceFactoryView.prototype.createCategoryIdName = function(name) { WorkspaceFactoryView.prototype.setCategoryTabSelection = function(id, selected) { if (!this.tabMap[id]) { - return; // Exit if tab does not exist. + return; // Exit if tab does not exist. } this.tabMap[id].className = selected ? 'tabon' : 'taboff'; }; @@ -144,8 +143,8 @@ WorkspaceFactoryView.prototype.bindClick = function(el, func) { /** * Creates a file and downloads it. In some browsers downloads, and in other * browsers, opens new tab with contents. - * @param {string} filename Name of file - * @param {!Blob} data Blob containing contents to download + * @param {string} filename Name of file. + * @param {!Blob} data Blob containing contents to download. */ WorkspaceFactoryView.prototype.createAndDownloadFile = function(filename, data) { @@ -164,8 +163,8 @@ WorkspaceFactoryView.prototype.createAndDownloadFile = /** * Given the ID of a certain category, updates the corresponding tab in * the DOM to show a new name. - * @param {string} newName Name of string to be displayed on tab - * @param {string} id ID of category to be updated + * @param {string} newName Name of string to be displayed on tab. + * @param {string} id ID of category to be updated. */ WorkspaceFactoryView.prototype.updateCategoryName = function(newName, id) { this.tabMap[id].textContent = newName; @@ -311,7 +310,7 @@ WorkspaceFactoryView.prototype.markShadowBlocks = function(blocks) { */ WorkspaceFactoryView.prototype.markShadowBlock = function(block) { // Add Blockly CSS for user-generated shadow blocks. - Blockly.utils.dom.addClass(block.svgGroup_, 'shadowBlock'); + block.getSvgRoot().classList.add('shadowBlock'); // If not a valid shadow block, add a warning message. if (!block.getSurroundParent()) { block.setWarningText('Shadow blocks must be nested inside' + @@ -329,7 +328,7 @@ WorkspaceFactoryView.prototype.markShadowBlock = function(block) { */ WorkspaceFactoryView.prototype.unmarkShadowBlock = function(block) { // Remove Blockly CSS for user-generated shadow blocks. - Blockly.utils.dom.removeClass(block.svgGroup_, 'shadowBlock'); + block.getSvgRoot().classList.remove('shadowBlock'); }; /** @@ -387,8 +386,7 @@ WorkspaceFactoryView.prototype.setBaseOptions = function() { // Check infinite blocks and hide suboption. document.getElementById('option_infiniteBlocks_checkbox').checked = true; - document.getElementById('maxBlockNumber_option').style.display = - 'none'; + document.getElementById('maxBlockNumber_option').style.display = 'none'; // Uncheck grid and zoom options and hide suboptions. document.getElementById('option_grid_checkbox').checked = false; diff --git a/tests/mocha/blocks/variables_test.js b/tests/mocha/blocks/variables_test.js index 73747122086..7de2b73684c 100644 --- a/tests/mocha/blocks/variables_test.js +++ b/tests/mocha/blocks/variables_test.js @@ -92,7 +92,7 @@ suite('Variables', function() { }); suite('getVariable', function() { - test('By id', function() { + test('By ID', function() { const var1 = this.workspace.createVariable('name1', 'type1', 'id1'); const var2 = this.workspace.createVariable('name2', 'type1', 'id2'); const var3 = this.workspace.createVariable('name3', 'type2', 'id3'); @@ -122,7 +122,7 @@ suite('Variables', function() { chai.assert.equal(var3, result3); }); - test('Bad id with name and type fallback', function() { + test('Bad ID with name and type fallback', function() { const var1 = this.workspace.createVariable('name1', 'type1', 'id1'); const var2 = this.workspace.createVariable('name2', 'type1', 'id2'); const var3 = this.workspace.createVariable('name3', 'type2', 'id3'); diff --git a/tests/mocha/shortcut_registry_test.js b/tests/mocha/shortcut_registry_test.js index dea01e22af3..916dc53fd30 100644 --- a/tests/mocha/shortcut_registry_test.js +++ b/tests/mocha/shortcut_registry_test.js @@ -39,7 +39,7 @@ suite('Keyboard Shortcut Registry Test', function() { }; chai.assert.throws( shouldThrow, Error, - 'Shortcut with name "test_shortcut" already exists.'); + 'Shortcut named "test_shortcut" already exists.'); }); test( 'Registers shortcut with same name opt_allowOverrides=true', @@ -104,7 +104,7 @@ suite('Keyboard Shortcut Registry Test', function() { const registry = this.registry; chai.assert.isFalse(registry.unregister('test')); - sinon.assert.calledOnceWithExactly(consoleStub, 'Keyboard shortcut with name "test" not found.'); + sinon.assert.calledOnceWithExactly(consoleStub, 'Keyboard shortcut named "test" not found.'); }); test('Unregistering a shortcut with key mappings', function() { const testShortcut = {'name': 'test_shortcut'}; @@ -173,7 +173,7 @@ suite('Keyboard Shortcut Registry Test', function() { }; chai.assert.throws( shouldThrow, Error, - 'Shortcut with name "test_shortcut" collides with shortcuts test_shortcut_2'); + 'Shortcut named "test_shortcut" collides with shortcuts "test_shortcut_2"'); }); }); @@ -216,7 +216,7 @@ suite('Keyboard Shortcut Registry Test', function() { chai.assert.isFalse(isRemoved); sinon.assert.calledOnceWithExactly( consoleStub, - 'No keyboard shortcut with name "test_shortcut" registered with key code "keyCode"'); + 'No keyboard shortcut named "test_shortcut" registered with key code "keyCode"'); }); test( 'Removes a key map that does not exist from empty key mapping opt_quiet=false', @@ -229,7 +229,7 @@ suite('Keyboard Shortcut Registry Test', function() { chai.assert.isFalse(isRemoved); sinon.assert.calledOnceWithExactly( consoleStub, - 'No keyboard shortcut with name "test_shortcut" registered with key code "keyCode"'); + 'No keyboard shortcut named "test_shortcut" registered with key code "keyCode"'); }); });