From e5dcb766bd0545b05fba2ef06a2260c70d6b3ee1 Mon Sep 17 00:00:00 2001 From: Neil Fraser Date: Thu, 22 Sep 2022 15:59:24 +0200 Subject: [PATCH] chore: Remove radix from parseInt, simplify Blockly.utils.dom methods, use Unicode characters. (#6441) * chore: remove radix from parseInt Previously any number starting with '0' would be parsed as octal if the radix was left blank. But this was changed years ago. It is no longer needed to specify a radix. * chore: 'ID' is identification 'id' is a part of Freud's brain. * Use Unicode characters instead of codes This is in line with the current style guide. * Simplify Blockly.utils.dom methods. classList add/remove/has supports SVG elements in all browsers Blockly supports (i.e. not IE). --- core/events/events_abstract.ts | 2 +- core/events/events_comment_base.ts | 2 +- core/events/events_selected.ts | 4 ++-- core/events/events_var_base.ts | 2 +- core/field.ts | 4 ++-- core/field_angle.ts | 2 +- core/field_checkbox.ts | 2 +- core/field_dropdown.ts | 2 +- core/field_multilineinput.ts | 2 +- core/flyout_base.ts | 6 +++--- core/interfaces/i_component.ts | 2 +- core/procedures.ts | 2 +- core/toolbox/collapsible_category.ts | 2 +- core/toolbox/toolbox.ts | 2 +- core/toolbox/toolbox_item.ts | 2 +- core/utils/dom.ts | 30 +++++----------------------- core/utils/idgenerator.ts | 2 +- core/utils/svg_math.ts | 4 ++-- core/workspace_comment.ts | 8 ++++---- core/workspace_svg.ts | 14 ++++++------- core/xml.ts | 12 ++++------- core/zoom_controls.ts | 2 +- tests/mocha/utils_test.js | 4 ++-- typings/msg/id.d.ts | 3 +-- 24 files changed, 45 insertions(+), 72 deletions(-) diff --git a/core/events/events_abstract.ts b/core/events/events_abstract.ts index 8602b99d7..f7b5731ad 100644 --- a/core/events/events_abstract.ts +++ b/core/events/events_abstract.ts @@ -42,7 +42,7 @@ export abstract class Abstract { /** @alias Blockly.Events.Abstract */ constructor() { /** - * The event group id for the group this event belongs to. Groups define + * The event group ID for the group this event belongs to. Groups define * events that should be treated as an single action from the user's * perspective, and should be undone together. */ diff --git a/core/events/events_comment_base.ts b/core/events/events_comment_base.ts index e65202a22..7d8d39268 100644 --- a/core/events/events_comment_base.ts +++ b/core/events/events_comment_base.ts @@ -48,7 +48,7 @@ export class CommentBase extends AbstractEvent { this.workspaceId = this.isBlank ? '' : opt_comment!.workspace.id; /** - * The event group id for the group this event belongs to. Groups define + * The event group ID for the group this event belongs to. Groups define * events that should be treated as an single action from the user's * perspective, and should be undone together. */ diff --git a/core/events/events_selected.ts b/core/events/events_selected.ts index a6c4b7ad0..5b1ee99bd 100644 --- a/core/events/events_selected.ts +++ b/core/events/events_selected.ts @@ -41,10 +41,10 @@ export class Selected extends UiBase { opt_workspaceId?: string) { super(opt_workspaceId); - /** The id of the last selected element. */ + /** The ID of the last selected element. */ this.oldElementId = opt_oldElementId; - /** The id of the selected element. */ + /** The ID of the selected element. */ this.newElementId = opt_newElementId; /** Type of this event. */ diff --git a/core/events/events_var_base.ts b/core/events/events_var_base.ts index 2463b6bbc..6bcf1d381 100644 --- a/core/events/events_var_base.ts +++ b/core/events/events_var_base.ts @@ -35,7 +35,7 @@ export class VarBase extends AbstractEvent { super(); this.isBlank = typeof opt_variable === 'undefined'; - /** The variable id for the variable this event pertains to. */ + /** The variable ID for the variable this event pertains to. */ this.varId = this.isBlank ? '' : opt_variable!.getId(); /** The workspace identifier for this event. */ diff --git a/core/field.ts b/core/field.ts index 57a821495..de670e236 100644 --- a/core/field.ts +++ b/core/field.ts @@ -845,12 +845,12 @@ export abstract class Field implements IASTNodeLocationSvg, } if (text.length > this.maxDisplayLength) { // Truncate displayed string and add an ellipsis ('...'). - text = text.substring(0, this.maxDisplayLength - 2) + '\u2026'; + text = text.substring(0, this.maxDisplayLength - 2) + '…'; } // Replace whitespace with non-breaking spaces so the text doesn't collapse. text = text.replace(/\s/g, Field.NBSP); if (this.sourceBlock_ && this.sourceBlock_.RTL) { - // The SVG is LTR, force text to be RTL. + // The SVG is LTR, force text to be RTL by adding an RLM. text += '\u200F'; } return text; diff --git a/core/field_angle.ts b/core/field_angle.ts index 26a6a7eb4..7c0ea8cfb 100644 --- a/core/field_angle.ts +++ b/core/field_angle.ts @@ -201,7 +201,7 @@ export class FieldAngle extends FieldTextInput { // Add the degree symbol to the left of the number, even in RTL (issue // #2380) this.symbol_ = dom.createSvgElement(Svg.TSPAN, {}); - this.symbol_.appendChild(document.createTextNode('\u00B0')); + this.symbol_.appendChild(document.createTextNode('°')); this.textElement_.appendChild(this.symbol_); } diff --git a/core/field_checkbox.ts b/core/field_checkbox.ts index 6e3753b38..9cabb3e58 100644 --- a/core/field_checkbox.ts +++ b/core/field_checkbox.ts @@ -27,7 +27,7 @@ import type {Sentinel} from './utils/sentinel.js'; */ export class FieldCheckbox extends Field { /** Default character for the checkmark. */ - static readonly CHECK_CHAR = '\u2713'; + static readonly CHECK_CHAR = '✓'; private checkChar_: string; /** diff --git a/core/field_dropdown.ts b/core/field_dropdown.ts index cff223045..da36fa569 100644 --- a/core/field_dropdown.ts +++ b/core/field_dropdown.ts @@ -741,7 +741,7 @@ const IMAGE_Y_OFFSET = 5; const IMAGE_Y_PADDING: number = IMAGE_Y_OFFSET * 2; /** Android can't (in 2014) display "▾", so use "▼" instead. */ -FieldDropdown.ARROW_CHAR = userAgent.ANDROID ? '\u25BC' : '\u25BE'; +FieldDropdown.ARROW_CHAR = userAgent.ANDROID ? '▼' : '▾'; /** * Validates the data structure to be processed as an options list. diff --git a/core/field_multilineinput.ts b/core/field_multilineinput.ts index 8a5ba7dc6..494c8b542 100644 --- a/core/field_multilineinput.ts +++ b/core/field_multilineinput.ts @@ -190,7 +190,7 @@ export class FieldMultilineInput extends FieldTextInput { } } if (this.sourceBlock_.RTL) { - // The SVG is LTR, force value to be RTL. + // The SVG is LTR, force value to be RTL by adding an RLM. textLines += '\u200F'; } return textLines; diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 150df13cf..762e9078c 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -801,12 +801,12 @@ export abstract class Flyout extends DeleteArea implements IFlyout { blockInfo: toolbox.BlockInfo, gaps: number[], defaultGap: number) { let gap; if (blockInfo['gap']) { - gap = parseInt(blockInfo['gap'].toString(), 10); + gap = parseInt(blockInfo['gap'].toString()); } else if (blockInfo['blockxml']) { const xml = (typeof blockInfo['blockxml'] === 'string' ? Xml.textToDom(blockInfo['blockxml']) : blockInfo['blockxml']) as Element; - gap = parseInt(xml.getAttribute('gap')!, 10); + gap = parseInt(xml.getAttribute('gap')!); } gaps.push(!gap || isNaN(gap) ? defaultGap : gap); } @@ -826,7 +826,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { // // The default gap is 24, can be set larger or smaller. // This overwrites the gap attribute on the previous element. - const newGap = parseInt(sepInfo['gap']!.toString(), 10); + const newGap = parseInt(sepInfo['gap']!.toString()); // Ignore gaps before the first block. if (!isNaN(newGap) && gaps.length > 0) { gaps[gaps.length - 1] = newGap; diff --git a/core/interfaces/i_component.ts b/core/interfaces/i_component.ts index 115f34376..f75a6b014 100644 --- a/core/interfaces/i_component.ts +++ b/core/interfaces/i_component.ts @@ -22,7 +22,7 @@ goog.declareModuleId('Blockly.IComponent'); */ export interface IComponent { /** - * The unique id for this component that is used to register with the + * The unique ID for this component that is used to register with the * ComponentManager. */ id: string; diff --git a/core/procedures.ts b/core/procedures.ts index 608ab24bf..e472fcd8a 100644 --- a/core/procedures.ts +++ b/core/procedures.ts @@ -122,7 +122,7 @@ export function findLegalName(name: string, block: Block): string { if (!r) { name += '2'; } else { - name = r[1] + (parseInt(r[2], 10) + 1); + name = r[1] + (parseInt(r[2]) + 1); } } return name; diff --git a/core/toolbox/collapsible_category.ts b/core/toolbox/collapsible_category.ts index d20dc4269..8f7f036bc 100644 --- a/core/toolbox/collapsible_category.ts +++ b/core/toolbox/collapsible_category.ts @@ -107,7 +107,7 @@ export class CollapsibleToolboxCategory extends ToolboxCategory implements const categoryDef = itemDef as toolbox.CategoryInfo; // Categories that are collapsible are created using a class registered // under a different name. - if (registryName.toUpperCase() == 'CATEGORY' && + if (registryName.toUpperCase() === 'CATEGORY' && toolbox.isCategoryCollapsible(categoryDef)) { registryName = CollapsibleToolboxCategory.registrationName; } diff --git a/core/toolbox/toolbox.ts b/core/toolbox/toolbox.ts index 14ab5c3b1..80d49517b 100644 --- a/core/toolbox/toolbox.ts +++ b/core/toolbox/toolbox.ts @@ -57,7 +57,7 @@ export class Toolbox extends DeleteArea implements IAutoHideable, IKeyboardAccessible, IStyleable, IToolbox { /** - * The unique id for this component that is used to register with the + * The unique ID for this component that is used to register with the * ComponentManager. */ override id = 'toolbox'; diff --git a/core/toolbox/toolbox_item.ts b/core/toolbox/toolbox_item.ts index d3010b32d..36e0e14ac 100644 --- a/core/toolbox/toolbox_item.ts +++ b/core/toolbox/toolbox_item.ts @@ -43,7 +43,7 @@ export class ToolboxItem implements IToolboxItem { constructor( toolboxItemDef: toolbox.ToolboxItemInfo, parentToolbox: IToolbox, opt_parent?: ICollapsibleToolboxItem) { - /** The id for the category. */ + /** The ID for the category. */ this.id_ = (toolboxItemDef as AnyDuringMigration)['toolboxitemid'] || idGenerator.getNextUniqueId(); diff --git a/core/utils/dom.ts b/core/utils/dom.ts index 044f6d6ce..8a49a3f05 100644 --- a/core/utils/dom.ts +++ b/core/utils/dom.ts @@ -84,7 +84,6 @@ export function createSvgElement( /** * Add a CSS class to a element. - * Similar to Closure's goog.dom.classes.add, except it handles SVG elements. * * @param element DOM element to add class to. * @param className Name of class to add. @@ -92,14 +91,10 @@ export function createSvgElement( * @alias Blockly.utils.dom.addClass */ export function addClass(element: Element, className: string): boolean { - let classes = element.getAttribute('class') || ''; - if ((' ' + classes + ' ').indexOf(' ' + className + ' ') !== -1) { + if (element.classList.contains(className)) { return false; } - if (classes) { - classes += ' '; - } - element.setAttribute('class', classes + className); + element.classList.add(className); return true; } @@ -119,7 +114,6 @@ export function removeClasses(element: Element, classNames: string) { /** * Remove a CSS class from a element. - * Similar to Closure's goog.dom.classes.remove, except it handles SVG elements. * * @param element DOM element to remove class from. * @param className Name of class to remove. @@ -127,28 +121,15 @@ export function removeClasses(element: Element, classNames: string) { * @alias Blockly.utils.dom.removeClass */ export function removeClass(element: Element, className: string): boolean { - const classes = element.getAttribute('class'); - if ((' ' + classes + ' ').indexOf(' ' + className + ' ') === -1) { + if (!element.classList.contains(className)) { return false; } - const classList = classes!.split(/\s+/); - for (let i = 0; i < classList.length; i++) { - if (!classList[i] || classList[i] === className) { - classList.splice(i, 1); - i--; - } - } - if (classList.length) { - element.setAttribute('class', classList.join(' ')); - } else { - element.removeAttribute('class'); - } + element.classList.remove(className); return true; } /** * Checks if an element has the specified CSS class. - * Similar to Closure's goog.dom.classes.has, except it handles SVG elements. * * @param element DOM element to check. * @param className Name of class to check. @@ -156,8 +137,7 @@ export function removeClass(element: Element, className: string): boolean { * @alias Blockly.utils.dom.hasClass */ export function hasClass(element: Element, className: string): boolean { - const classes = element.getAttribute('class'); - return (' ' + classes + ' ').indexOf(' ' + className + ' ') !== -1; + return element.classList.contains(className); } /** diff --git a/core/utils/idgenerator.ts b/core/utils/idgenerator.ts index 4f4735608..e0b647df1 100644 --- a/core/utils/idgenerator.ts +++ b/core/utils/idgenerator.ts @@ -52,7 +52,7 @@ let nextId = 0; /** * Generate the next unique element IDs. - * IDs are compatible with the HTML4 id attribute restrictions: + * IDs are compatible with the HTML4 'id' attribute restrictions: * Use only ASCII letters, digits, '_', '-' and '.' * * For UUIDs use genUid (below) instead; this ID generator should diff --git a/core/utils/svg_math.ts b/core/utils/svg_math.ts index 5fbdd0483..4f16332d7 100644 --- a/core/utils/svg_math.ts +++ b/core/utils/svg_math.ts @@ -55,10 +55,10 @@ export function getRelativeXY(element: Element): Coordinate { const x = (element as any).x && element.getAttribute('x'); const y = (element as any).y && element.getAttribute('y'); if (x) { - xy.x = parseInt(x, 10); + xy.x = parseInt(x); } if (y) { - xy.y = parseInt(y, 10); + xy.y = parseInt(y); } // Second, check for transform="translate(...)" attribute. const transform = element.getAttribute('transform'); diff --git a/core/workspace_comment.ts b/core/workspace_comment.ts index d99defd26..6e218c858 100644 --- a/core/workspace_comment.ts +++ b/core/workspace_comment.ts @@ -370,15 +370,15 @@ export class WorkspaceComment { return { id: xmlId, // The height of the comment in workspace units, or 100 if not specified. - h: xmlH ? parseInt(xmlH, 10) : 100, + h: xmlH ? parseInt(xmlH) : 100, // The width of the comment in workspace units, or 100 if not specified. - w: xmlW ? parseInt(xmlW, 10) : 100, + w: xmlW ? parseInt(xmlW) : 100, // The x position of the comment in workspace coordinates, or NaN if not // specified in the XML. - x: xmlX ? parseInt(xmlX, 10) : NaN, + x: xmlX ? parseInt(xmlX) : NaN, // The y position of the comment in workspace coordinates, or NaN if not // specified in the XML. - y: xmlY ? parseInt(xmlY, 10) : NaN, + y: xmlY ? parseInt(xmlY) : NaN, content: xml.textContent ?? '', }; } diff --git a/core/workspace_svg.ts b/core/workspace_svg.ts index b401412ed..415b7a23e 100644 --- a/core/workspace_svg.ts +++ b/core/workspace_svg.ts @@ -1227,10 +1227,8 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { // Figure out where we want to put the canvas back. The order // in the is important because things are layered. const previousElement = this.svgBlockCanvas_.previousSibling as Element; - const width = - parseInt(this.getParentSvg().getAttribute('width') ?? '0', 10); - const height = - parseInt(this.getParentSvg().getAttribute('height') ?? '0', 10); + const width = parseInt(this.getParentSvg().getAttribute('width') ?? '0'); + const height = parseInt(this.getParentSvg().getAttribute('height') ?? '0'); const coord = svgMath.getRelativeXY(this.getCanvas()); this.workspaceDragSurface_!.setContentsAndShow( this.getCanvas(), this.getBubbleCanvas(), previousElement, width, @@ -1409,11 +1407,11 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { let blockY = 0; if (xmlBlock) { block = Xml.domToBlock(xmlBlock, this) as BlockSvg; - blockX = parseInt(xmlBlock.getAttribute('x') ?? '0', 10); + blockX = parseInt(xmlBlock.getAttribute('x') ?? '0'); if (this.RTL) { blockX = -blockX; } - blockY = parseInt(xmlBlock.getAttribute('y') ?? '0', 10); + blockY = parseInt(xmlBlock.getAttribute('y') ?? '0'); } else if (jsonBlock) { block = blocks.append(jsonBlock, this) as BlockSvg; blockX = jsonBlock['x'] || 10; @@ -1488,8 +1486,8 @@ export class WorkspaceSvg extends Workspace implements IASTNodeLocationSvg { try { comment = WorkspaceCommentSvg.fromXmlRendered(xmlComment, this); // Move the duplicate to original position. - let commentX = parseInt(xmlComment.getAttribute('x') ?? '0', 10); - let commentY = parseInt(xmlComment.getAttribute('y') ?? '0', 10); + let commentX = parseInt(xmlComment.getAttribute('x') ?? '0'); + let commentY = parseInt(xmlComment.getAttribute('y') ?? '0'); if (!isNaN(commentX) && !isNaN(commentY)) { if (this.RTL) { commentX = -commentX; diff --git a/core/xml.ts b/core/xml.ts index 19443453a..4ecde7780 100644 --- a/core/xml.ts +++ b/core/xml.ts @@ -459,14 +459,12 @@ export function domToWorkspace(xml: Element, workspace: Workspace): string[] { // AnyDuringMigration because: Argument of type 'string | null' is not // assignable to parameter of type 'string'. const blockX = xmlChildElement.hasAttribute('x') ? - parseInt( - xmlChildElement.getAttribute('x') as AnyDuringMigration, 10) : + parseInt(xmlChildElement.getAttribute('x') as AnyDuringMigration) : 10; // AnyDuringMigration because: Argument of type 'string | null' is not // assignable to parameter of type 'string'. const blockY = xmlChildElement.hasAttribute('y') ? - parseInt( - xmlChildElement.getAttribute('y') as AnyDuringMigration, 10) : + parseInt(xmlChildElement.getAttribute('y') as AnyDuringMigration) : 10; if (!isNaN(blockX) && !isNaN(blockY)) { block.moveBy(workspace.RTL ? width - blockX : blockX, blockY); @@ -747,12 +745,10 @@ function applyCommentTagNodes(xmlChildren: Element[], block: Block) { const pinned = xmlChild.getAttribute('pinned') === 'true'; // AnyDuringMigration because: Argument of type 'string | null' is not // assignable to parameter of type 'string'. - const width = - parseInt(xmlChild.getAttribute('w') as AnyDuringMigration, 10); + const width = parseInt(xmlChild.getAttribute('w') as AnyDuringMigration); // AnyDuringMigration because: Argument of type 'string | null' is not // assignable to parameter of type 'string'. - const height = - parseInt(xmlChild.getAttribute('h') as AnyDuringMigration, 10); + const height = parseInt(xmlChild.getAttribute('h') as AnyDuringMigration); block.setCommentText(text); block.commentModel.pinned = pinned; diff --git a/core/zoom_controls.ts b/core/zoom_controls.ts index 37c441ed1..d73ff47fd 100644 --- a/core/zoom_controls.ts +++ b/core/zoom_controls.ts @@ -38,7 +38,7 @@ import type {WorkspaceSvg} from './workspace_svg.js'; */ export class ZoomControls implements IPositionable { /** - * The unique id for this component that is used to register with the + * The unique ID for this component that is used to register with the * ComponentManager. */ id = 'zoomControls'; diff --git a/tests/mocha/utils_test.js b/tests/mocha/utils_test.js index c7c38a86b..c70391d9b 100644 --- a/tests/mocha/utils_test.js +++ b/tests/mocha/utils_test.js @@ -344,9 +344,9 @@ suite('Utils', function() { const p = document.createElement('p'); p.className = ' one three two three '; Blockly.utils.dom.removeClass(p, 'two'); - chai.assert.equal(p.className, 'one three three', 'Removing "two"'); + chai.assert.equal(p.className, 'one three', 'Removing "two"'); Blockly.utils.dom.removeClass(p, 'four'); - chai.assert.equal(p.className, 'one three three', 'Removing "four"'); + chai.assert.equal(p.className, 'one three', 'Removing "four"'); Blockly.utils.dom.removeClass(p, 'three'); chai.assert.equal(p.className, 'one', 'Removing "three"'); Blockly.utils.dom.removeClass(p, 'ne'); diff --git a/typings/msg/id.d.ts b/typings/msg/id.d.ts index ddb57b7fb..79a2a28f2 100644 --- a/typings/msg/id.d.ts +++ b/typings/msg/id.d.ts @@ -5,7 +5,7 @@ */ /** - * @fileoverview Type definitions for the Blockly id locale. + * @fileoverview Type definitions for the Blockly ID locale. * @author samelh@google.com (Sam El-Husseini) */ @@ -13,4 +13,3 @@ import BlocklyMsg = Blockly.Msg; export = BlocklyMsg; -