From daf78af13e5ce7cebefd0e7596f63e8644c324ba Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Fri, 22 Apr 2022 15:55:55 -0700 Subject: [PATCH] fix!: fix or silence type errors (#6105) * fix: add isBlockCreatable to IFlyout interface * fix: add getClickTarget to IToolboxItem interface * fix: fix some types in zelos renderer * fix: add scrollToStart to IFlyout interface * fix: add setVisible_ to IToolboxItem * fix: use instanceof check for workspace comments in gesture code * fix: data stored on the DOM for tooltips * fix: use blockSvg to access icons * fix: add instanceof check in shortcut_items.js * fix: suppress warning about onKeyDown in tolbox * fix: add instanceof check in workspace_svg * fix: don't use dot accessor to avoid type problem * fix: silence type errors in ast_node.js --- core/flyout_base.js | 2 +- core/gesture.js | 10 +++++++--- core/interfaces/i_flyout.js | 15 +++++++++++++++ core/interfaces/i_toolbox_item.js | 15 +++++++++++++++ core/keyboard_nav/ast_node.js | 12 +++++++++--- core/renderers/common/i_path_object.js | 12 ++++++++++++ core/renderers/zelos/drawer.js | 3 ++- core/renderers/zelos/info.js | 8 +++++++- core/serialization/blocks.js | 2 +- core/shortcut_items.js | 7 +++---- core/toolbox/toolbox.js | 17 +++++++++++++---- core/toolbox/toolbox_item.js | 10 ++++++++++ core/tooltip.js | 10 ++++++---- core/workspace_svg.js | 4 ++-- tests/deps.js | 4 ++-- 15 files changed, 105 insertions(+), 26 deletions(-) diff --git a/core/flyout_base.js b/core/flyout_base.js index 82be038ab..2e0eada90 100644 --- a/core/flyout_base.js +++ b/core/flyout_base.js @@ -951,7 +951,7 @@ class Flyout extends DeleteArea { * otherwise. * @package */ - isBlockCreatable_(block) { + isBlockCreatable(block) { return block.isEnabled(); } diff --git a/core/gesture.js b/core/gesture.js index 51374278f..7161ce77d 100644 --- a/core/gesture.js +++ b/core/gesture.js @@ -38,6 +38,8 @@ const {IBlockDragger} = goog.requireType('Blockly.IBlockDragger'); const {IBubble} = goog.requireType('Blockly.IBubble'); /* eslint-disable-next-line no-unused-vars */ const {IFlyout} = goog.requireType('Blockly.IFlyout'); +/* eslint-disable-next-line no-unused-vars */ +const {WorkspaceCommentSvg} = goog.require('Blockly.WorkspaceCommentSvg'); const {WorkspaceDragger} = goog.require('Blockly.WorkspaceDragger'); /* eslint-disable-next-line no-unused-vars */ const {WorkspaceSvg} = goog.requireType('Blockly.WorkspaceSvg'); @@ -332,7 +334,7 @@ class Gesture { if (!this.targetBlock_) { return false; } - if (!this.flyout_.isBlockCreatable_(this.targetBlock_)) { + if (!this.flyout_.isBlockCreatable(this.targetBlock_)) { return false; } if (!this.flyout_.isScrollable() || @@ -743,8 +745,10 @@ class Gesture { */ doBubbleClick_() { // TODO (#1673): Consistent handling of single clicks. - this.startBubble_.setFocus && this.startBubble_.setFocus(); - this.startBubble_.select && this.startBubble_.select(); + if (this.startBubble_ instanceof WorkspaceCommentSvg) { + this.startBubble_.setFocus(); + this.startBubble_.select(); + } } /** diff --git a/core/interfaces/i_flyout.js b/core/interfaces/i_flyout.js index 90668780d..f4ea22a4f 100644 --- a/core/interfaces/i_flyout.js +++ b/core/interfaces/i_flyout.js @@ -198,4 +198,19 @@ IFlyout.prototype.position; */ IFlyout.prototype.isDragTowardWorkspace; +/** + * Does this flyout allow you to create a new instance of the given block? + * Used for deciding if a block can be "dragged out of" the flyout. + * @param {!BlockSvg} block The block to copy from the flyout. + * @return {boolean} True if you can create a new instance of the block, false + * otherwise. + * @package + */ +IFlyout.prototype.isBlockCreatable; + +/** + * Scroll the flyout to the beginning of its contents. + */ +IFlyout.prototype.scrollToStart; + exports.IFlyout = IFlyout; diff --git a/core/interfaces/i_toolbox_item.js b/core/interfaces/i_toolbox_item.js index 8b20cf647..b5b4bc3cb 100644 --- a/core/interfaces/i_toolbox_item.js +++ b/core/interfaces/i_toolbox_item.js @@ -82,4 +82,19 @@ IToolboxItem.prototype.isCollapsible; */ IToolboxItem.prototype.dispose; +/** + * Gets the HTML element that is clickable. + * @return {?Element} The HTML element that receives clicks. + * @public + */ +IToolboxItem.prototype.getClickTarget; + +/** + * Sets whether the category is visible or not. + * For a category to be visible its parent category must also be expanded. + * @param {boolean} isVisible True if category should be visible. + * @protected + */ +IToolboxItem.prototype.setVisible_; + exports.IToolboxItem = IToolboxItem; diff --git a/core/keyboard_nav/ast_node.js b/core/keyboard_nav/ast_node.js index 44b9f65dc..00bd7fbbb 100644 --- a/core/keyboard_nav/ast_node.js +++ b/core/keyboard_nav/ast_node.js @@ -271,14 +271,20 @@ class ASTNode { */ navigateBetweenStacks_(forward) { let curLocation = this.getLocation(); - if (curLocation.getSourceBlock) { + // TODO(#6097): Use instanceof checks to exit early for values of + // curLocation that don't make sense. + if ((/** @type {!IASTNodeLocationWithBlock} */ (curLocation)) + .getSourceBlock) { curLocation = /** @type {!IASTNodeLocationWithBlock} */ (curLocation) .getSourceBlock(); } - if (!curLocation || !curLocation.workspace) { + // TODO(#6097): Use instanceof checks to exit early for values of + // curLocation that don't make sense. + const curLocationAsBlock = /** @type {!Block} */ (curLocation); + if (!curLocationAsBlock || !curLocationAsBlock.workspace) { return null; } - const curRoot = curLocation.getRootBlock(); + const curRoot = curLocationAsBlock.getRootBlock(); const topBlocks = curRoot.workspace.getTopBlocks(true); for (let i = 0; i < topBlocks.length; i++) { const topBlock = topBlocks[i]; diff --git a/core/renderers/common/i_path_object.js b/core/renderers/common/i_path_object.js index 06939bb3c..000d29b6e 100644 --- a/core/renderers/common/i_path_object.js +++ b/core/renderers/common/i_path_object.js @@ -21,6 +21,8 @@ goog.module('Blockly.blockRendering.IPathObject'); /* eslint-disable-next-line no-unused-vars */ const {BlockSvg} = goog.requireType('Blockly.BlockSvg'); /* eslint-disable-next-line no-unused-vars */ +const {Connection} = goog.requireType('Blockly.Connection'); +/* eslint-disable-next-line no-unused-vars */ const {ConstantProvider} = goog.requireType('Blockly.blockRendering.ConstantProvider'); /* eslint-disable-next-line no-unused-vars */ const {Theme} = goog.requireType('Blockly.Theme'); @@ -159,4 +161,14 @@ IPathObject.prototype.updateMovable; */ IPathObject.prototype.updateReplacementFade; + +/** + * Add or remove styling that shows that if the dragging block is dropped, + * this block will be connected to the input. + * @param {Connection} conn The connection on the input to highlight. + * @param {boolean} enable True if styling should be added. + * @package + */ +IPathObject.prototype.updateShapeForInputHighlight; + exports.IPathObject = IPathObject; diff --git a/core/renderers/zelos/drawer.js b/core/renderers/zelos/drawer.js index 9f36c6297..46f8b0e1f 100644 --- a/core/renderers/zelos/drawer.js +++ b/core/renderers/zelos/drawer.js @@ -226,7 +226,8 @@ class Drawer extends BaseDrawer { .pathRightDown(input.height) + svgPaths.lineOnAxis('h', -width) + (/** @type {!DynamicShape} */ (input.shape)).pathUp(input.height) + 'z'; - this.block_.pathObject.setOutlinePath(inputName, outlinePath); + const pathObject = /** @type {!PathObject} */ (this.block_.pathObject); + pathObject.setOutlinePath(inputName, outlinePath); } /** diff --git a/core/renderers/zelos/info.js b/core/renderers/zelos/info.js index 977a16527..8d4962d0a 100644 --- a/core/renderers/zelos/info.js +++ b/core/renderers/zelos/info.js @@ -34,6 +34,8 @@ const {InputConnection} = goog.require('Blockly.blockRendering.InputConnection') const {InRowSpacer} = goog.require('Blockly.blockRendering.InRowSpacer'); /* eslint-disable-next-line no-unused-vars */ const {Measurable} = goog.requireType('Blockly.blockRendering.Measurable'); +/* eslint-disable-next-line no-unused-vars */ +const {PathObject} = goog.requireType('Blockly.zelos.PathObject'); const {RenderInfo: BaseRenderInfo} = goog.require('Blockly.blockRendering.RenderInfo'); /* eslint-disable-next-line no-unused-vars */ const {Renderer} = goog.requireType('Blockly.zelos.Renderer'); @@ -528,8 +530,12 @@ class RenderInfo extends BaseRenderInfo { if (Types.isInlineInput(elem) && elem instanceof InputConnection) { const connectedBlock = elem.connectedBlock; const innerShape = connectedBlock ? - connectedBlock.pathObject.outputShapeType : + /** @type {!PathObject} */ (connectedBlock.pathObject) + .outputShapeType : elem.shape.type; + if (innerShape == null) { + return 0; + } // Special case for value to stack / value to statement blocks. if (connectedBlock && connectedBlock.outputConnection && (connectedBlock.statementInputCount || diff --git a/core/serialization/blocks.js b/core/serialization/blocks.js index 5bcd309dd..781dec01d 100644 --- a/core/serialization/blocks.js +++ b/core/serialization/blocks.js @@ -625,7 +625,7 @@ const initBlock = function(block, rendered) { blockSvg.render(false); // fixes #6076 JSO deserialization doesn't // set .iconXY_ property so here it will be set - const icons = block.getIcons(); + const icons = blockSvg.getIcons(); for (let i = 0; i < icons.length; i++) { icons[i].computeIconLocation(); } diff --git a/core/shortcut_items.js b/core/shortcut_items.js index fea163111..b67ca2801 100644 --- a/core/shortcut_items.js +++ b/core/shortcut_items.js @@ -17,8 +17,7 @@ goog.module('Blockly.ShortcutItems'); const clipboard = goog.require('Blockly.clipboard'); const common = goog.require('Blockly.common'); -/* eslint-disable-next-line no-unused-vars */ -const {BlockSvg} = goog.requireType('Blockly.BlockSvg'); +const {BlockSvg} = goog.require('Blockly.BlockSvg'); const {Gesture} = goog.require('Blockly.Gesture'); /* eslint-disable-next-line no-unused-vars */ const {ICopyable} = goog.requireType('Blockly.ICopyable'); @@ -145,8 +144,8 @@ const registerCut = function() { preconditionFn: function(workspace) { const selected = common.getSelected(); return !workspace.options.readOnly && !Gesture.inProgress() && selected && - selected.isDeletable() && selected.isMovable() && - !selected.workspace.isFlyout; + selected instanceof BlockSvg && selected.isDeletable() && + selected.isMovable() && !selected.workspace.isFlyout; }, callback: function() { const selected = common.getSelected(); diff --git a/core/toolbox/toolbox.js b/core/toolbox/toolbox.js index 9ca7af684..006526589 100644 --- a/core/toolbox/toolbox.js +++ b/core/toolbox/toolbox.js @@ -54,6 +54,8 @@ const {Rect} = goog.require('Blockly.utils.Rect'); /* eslint-disable-next-line no-unused-vars */ const {ShortcutRegistry} = goog.requireType('Blockly.ShortcutRegistry'); /* eslint-disable-next-line no-unused-vars */ +const {ToolboxCategory} = goog.requireType('Blockly.ToolboxCategory'); +/* eslint-disable-next-line no-unused-vars */ const {WorkspaceSvg} = goog.requireType('Blockly.WorkspaceSvg'); /** @suppress {extraRequire} */ goog.require('Blockly.Events.ToolboxItemSelect'); @@ -371,8 +373,13 @@ class Toolbox extends DeleteArea { handled = false; break; } - if (!handled && this.selectedItem_ && this.selectedItem_.onKeyDown) { - handled = this.selectedItem_.onKeyDown(e); + if (!handled && this.selectedItem_) { + // TODO(#6097): Figure out who implements onKeyDown and which interface it + // should be part of. + const untypedItem = /** @type {?} */ (this.selectedItem_); + if (untypedItem.onKeyDown) { + handled = untypedItem.onKeyDown(e); + } } if (handled) { @@ -816,8 +823,10 @@ class Toolbox extends DeleteArea { refreshTheme() { for (let i = 0; i < this.contents_.length; i++) { const child = this.contents_[i]; - if (child.refreshTheme) { - child.refreshTheme(); + // TODO(#6097): Fix types or add refreshTheme to IToolboxItem. + const childAsCategory = /** @type {ToolboxCategory} */ (child); + if (childAsCategory.refreshTheme) { + childAsCategory.refreshTheme(); } } } diff --git a/core/toolbox/toolbox_item.js b/core/toolbox/toolbox_item.js index 7e88ee181..f3a435c8c 100644 --- a/core/toolbox/toolbox_item.js +++ b/core/toolbox/toolbox_item.js @@ -168,6 +168,16 @@ class ToolboxItem { * @public */ dispose() {} + + /** + * Sets whether the category is visible or not. + * For a category to be visible its parent category must also be expanded. + * @param {boolean} _isVisible True if category should be visible. + * @protected + */ + setVisible_(_isVisible) { + // nop by default + } } exports.ToolboxItem = ToolboxItem; diff --git a/core/tooltip.js b/core/tooltip.js index f5925a723..e3436ed0f 100644 --- a/core/tooltip.js +++ b/core/tooltip.js @@ -287,9 +287,10 @@ exports.createDom = createDom; * @alias Blockly.Tooltip.bindMouseEvents */ const bindMouseEvents = function(element) { - element.mouseOverWrapper_ = + // TODO (#6097): Don't stash wrapper info on the DOM. + (/** @type {?} */ (element)).mouseOverWrapper_ = browserEvents.bind(element, 'mouseover', null, onMouseOver); - element.mouseOutWrapper_ = + (/** @type {?} */ (element)).mouseOutWrapper_ = browserEvents.bind(element, 'mouseout', null, onMouseOut); // Don't use bindEvent_ for mousemove since that would create a @@ -308,8 +309,9 @@ const unbindMouseEvents = function(element) { if (!element) { return; } - browserEvents.unbind(element.mouseOverWrapper_); - browserEvents.unbind(element.mouseOutWrapper_); + // TODO (#6097): Don't stash wrapper info on the DOM. + browserEvents.unbind((/** @type {?} */ (element)).mouseOverWrapper_); + browserEvents.unbind((/** @type {?} */ (element)).mouseOutWrapper_); element.removeEventListener('mousemove', onMouseMove); }; exports.unbindMouseEvents = unbindMouseEvents; diff --git a/core/workspace_svg.js b/core/workspace_svg.js index 81990bd9b..8b9e83c7b 100644 --- a/core/workspace_svg.js +++ b/core/workspace_svg.js @@ -1519,7 +1519,7 @@ class WorkspaceSvg extends Workspace { * the paste was not successful. */ paste(state) { - if (!this.rendered || !state['type'] && !state.tagName) { + if (!this.rendered || !state['type'] && !state['tagName']) { return null; } if (this.currentGesture_) { @@ -1938,7 +1938,7 @@ class WorkspaceSvg extends Workspace { // Start at 1 since the 0th block was used for initialization. for (let i = 1; i < topElements.length; i++) { const topElement = topElements[i]; - if (topElement.isInsertionMarker && topElement.isInsertionMarker()) { + if (topElement instanceof BlockSvg && topElement.isInsertionMarker()) { continue; } const blockBoundary = topElement.getBoundingRectangle(); diff --git a/tests/deps.js b/tests/deps.js index f5b83a45e..a0e041a7b 100644 --- a/tests/deps.js +++ b/tests/deps.js @@ -87,7 +87,7 @@ goog.addDependency('../../core/flyout_horizontal.js', ['Blockly.HorizontalFlyout goog.addDependency('../../core/flyout_metrics_manager.js', ['Blockly.FlyoutMetricsManager'], ['Blockly.MetricsManager'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/flyout_vertical.js', ['Blockly.VerticalFlyout'], ['Blockly.Block', 'Blockly.Flyout', 'Blockly.Scrollbar', 'Blockly.WidgetDiv', 'Blockly.browserEvents', 'Blockly.constants', 'Blockly.dropDownDiv', 'Blockly.registry', 'Blockly.utils.Rect', 'Blockly.utils.toolbox'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/generator.js', ['Blockly.Generator'], ['Blockly.Names', 'Blockly.common', 'Blockly.utils.deprecation'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../core/gesture.js', ['Blockly.Gesture'], ['Blockly.BlockDragger', 'Blockly.BubbleDragger', 'Blockly.Events.Click', 'Blockly.Events.utils', 'Blockly.Tooltip', 'Blockly.Touch', 'Blockly.Workspace', 'Blockly.WorkspaceDragger', 'Blockly.blockAnimations', 'Blockly.browserEvents', 'Blockly.common', 'Blockly.config', 'Blockly.internalConstants', 'Blockly.registry', 'Blockly.utils.Coordinate'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../core/gesture.js', ['Blockly.Gesture'], ['Blockly.BlockDragger', 'Blockly.BubbleDragger', 'Blockly.Events.Click', 'Blockly.Events.utils', 'Blockly.Tooltip', 'Blockly.Touch', 'Blockly.Workspace', 'Blockly.WorkspaceCommentSvg', 'Blockly.WorkspaceDragger', 'Blockly.blockAnimations', 'Blockly.browserEvents', 'Blockly.common', 'Blockly.config', 'Blockly.internalConstants', 'Blockly.registry', 'Blockly.utils.Coordinate'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/grid.js', ['Blockly.Grid'], ['Blockly.utils.Svg', 'Blockly.utils.dom', 'Blockly.utils.userAgent'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/icon.js', ['Blockly.Icon'], ['Blockly.browserEvents', 'Blockly.utils.Coordinate', 'Blockly.utils.Size', 'Blockly.utils.Svg', 'Blockly.utils.dom', 'Blockly.utils.svgMath'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/inject.js', ['Blockly.inject'], ['Blockly.BlockDragSurfaceSvg', 'Blockly.Css', 'Blockly.Grid', 'Blockly.Msg', 'Blockly.Options', 'Blockly.ScrollbarPair', 'Blockly.ShortcutRegistry', 'Blockly.Tooltip', 'Blockly.Touch', 'Blockly.WidgetDiv', 'Blockly.Workspace', 'Blockly.WorkspaceDragSurfaceSvg', 'Blockly.WorkspaceSvg', 'Blockly.browserEvents', 'Blockly.bumpObjects', 'Blockly.common', 'Blockly.dropDownDiv', 'Blockly.utils.Svg', 'Blockly.utils.aria', 'Blockly.utils.dom', 'Blockly.utils.userAgent'], {'lang': 'es6', 'module': 'goog'}); @@ -210,7 +210,7 @@ goog.addDependency('../../core/serialization/priorities.js', ['Blockly.serializa goog.addDependency('../../core/serialization/registry.js', ['Blockly.serialization.registry'], ['Blockly.registry'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/serialization/variables.js', ['Blockly.serialization.variables'], ['Blockly.serialization.ISerializer', 'Blockly.serialization.priorities', 'Blockly.serialization.registry'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/serialization/workspaces.js', ['Blockly.serialization.workspaces'], ['Blockly.Events.utils', 'Blockly.Workspace', 'Blockly.WorkspaceSvg', 'Blockly.registry', 'Blockly.utils.dom'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../core/shortcut_items.js', ['Blockly.ShortcutItems'], ['Blockly.Gesture', 'Blockly.ShortcutRegistry', 'Blockly.clipboard', 'Blockly.common', 'Blockly.utils.KeyCodes'], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../core/shortcut_items.js', ['Blockly.ShortcutItems'], ['Blockly.BlockSvg', 'Blockly.Gesture', 'Blockly.ShortcutRegistry', 'Blockly.clipboard', 'Blockly.common', 'Blockly.utils.KeyCodes'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/shortcut_registry.js', ['Blockly.ShortcutRegistry'], ['Blockly.utils.KeyCodes', 'Blockly.utils.object'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/sprites.js', ['Blockly.sprite'], [], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/theme.js', ['Blockly.Theme'], ['Blockly.registry', 'Blockly.utils.object'], {'lang': 'es6', 'module': 'goog'});