From ab03c65f9f6f47b7f76a1cb6fa3dcff70bf14ca5 Mon Sep 17 00:00:00 2001 From: Aaron Dodson Date: Tue, 27 Sep 2022 13:42:52 -0700 Subject: [PATCH] fix: Fix blocks with mutators. (#6440) * refactor: Revert the Mutator/Icon constructor API changes with a deprecation warning. * fix: Update the block definitions to use the new Mutator constructor. --- blocks/lists.js | 2 +- blocks/procedures.js | 4 +-- blocks/text.js | 2 +- core/comment.ts | 21 +++++++------- core/extensions.ts | 2 +- core/icon.ts | 39 ++++++++++++++++++-------- core/mutator.ts | 65 +++++++++++++++++++++++++------------------- core/warning.ts | 6 ++-- 8 files changed, 84 insertions(+), 57 deletions(-) diff --git a/blocks/lists.js b/blocks/lists.js index 1216c171082..c83352aa052 100644 --- a/blocks/lists.js +++ b/blocks/lists.js @@ -133,7 +133,7 @@ blocks['lists_create_with'] = { this.itemCount_ = 3; this.updateShape_(); this.setOutput(true, 'Array'); - this.setMutator(new Mutator(['lists_create_with_item'])); + this.setMutator(new Mutator(['lists_create_with_item'], this)); this.setTooltip(Msg['LISTS_CREATE_WITH_TOOLTIP']); }, /** diff --git a/blocks/procedures.js b/blocks/procedures.js index 6078e55a315..cdb27203121 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -461,7 +461,7 @@ blocks['procedures_defnoreturn'] = { .appendField(Msg['PROCEDURES_DEFNORETURN_TITLE']) .appendField(nameField, 'NAME') .appendField('', 'PARAMS'); - this.setMutator(new Mutator(['procedures_mutatorarg'])); + this.setMutator(new Mutator(['procedures_mutatorarg'], this)); if ((this.workspace.options.comments || (this.workspace.options.parentWorkspace && this.workspace.options.parentWorkspace.options.comments)) && @@ -507,7 +507,7 @@ blocks['procedures_defreturn'] = { this.appendValueInput('RETURN') .setAlign(Align.RIGHT) .appendField(Msg['PROCEDURES_DEFRETURN_RETURN']); - this.setMutator(new Mutator(['procedures_mutatorarg'])); + this.setMutator(new Mutator(['procedures_mutatorarg'], this)); if ((this.workspace.options.comments || (this.workspace.options.parentWorkspace && this.workspace.options.parentWorkspace.options.comments)) && diff --git a/blocks/text.js b/blocks/text.js index 90a932d3a38..88a5b801241 100644 --- a/blocks/text.js +++ b/blocks/text.js @@ -864,7 +864,7 @@ const TEXT_JOIN_EXTENSION = function() { this.itemCount_ = 2; this.updateShape_(); // Configure the mutator UI. - this.setMutator(new Mutator(['text_create_join_item'])); + this.setMutator(new Mutator(['text_create_join_item'], this)); }; // Update the tooltip of 'text_append' block to reference the variable. diff --git a/core/comment.ts b/core/comment.ts index 7831a2a0def..6cf30555ff3 100644 --- a/core/comment.ts +++ b/core/comment.ts @@ -142,7 +142,7 @@ export class Comment extends Icon { HTMLTextAreaElement; const textarea = this.textarea_; textarea.className = 'blocklyCommentTextarea'; - textarea.setAttribute('dir', this.block_.RTL ? 'RTL' : 'LTR'); + textarea.setAttribute('dir', this.getBlock().RTL ? 'RTL' : 'LTR'); textarea.value = this.model_.text ?? ''; this.resizeTextarea_(); @@ -167,7 +167,7 @@ export class Comment extends Icon { function(this: Comment, _e: Event) { if (this.cachedText_ !== this.model_.text) { eventUtils.fire(new (eventUtils.get(eventUtils.BLOCK_CHANGE))( - this.block_, 'comment', null, this.cachedText_, + this.getBlock(), 'comment', null, this.cachedText_, this.model_.text)); } }); @@ -233,7 +233,7 @@ export class Comment extends Icon { return; } eventUtils.fire(new (eventUtils.get(eventUtils.BUBBLE_OPEN))( - this.block_, visible, 'comment')); + this.getBlock(), visible, 'comment')); this.model_.pinned = visible; if (visible) { this.createBubble_(); @@ -244,7 +244,7 @@ export class Comment extends Icon { /** Show the bubble. Handles deciding if it should be editable or not. */ private createBubble_() { - if (!this.block_.isEditable()) { + if (!this.getBlock().isEditable()) { this.createNonEditableBubble_(); } else { this.createEditableBubble_(); @@ -253,12 +253,13 @@ export class Comment extends Icon { /** Show an editable bubble. */ private createEditableBubble_() { + const block = this.getBlock(); this.bubble_ = new Bubble( - this.block_.workspace, this.createEditor_(), - this.block_.pathObject.svgPath, (this.iconXY_ as Coordinate), - this.model_.size.width, this.model_.size.height); + block.workspace, this.createEditor_(), block.pathObject.svgPath, + (this.iconXY_ as Coordinate), this.model_.size.width, + this.model_.size.height); // Expose this comment's block's ID on its top-level SVG group. - this.bubble_.setSvgId(this.block_.id); + this.bubble_.setSvgId(block.id); this.bubble_.registerResizeEvent(this.onBubbleResize_.bind(this)); this.applyColour(); } @@ -272,7 +273,7 @@ export class Comment extends Icon { // TODO (#2917): It would be great if the comment could support line breaks. this.paragraphElement_ = Bubble.textToDom(this.model_.text ?? ''); this.bubble_ = Bubble.createNonEditableBubble( - this.paragraphElement_, (this.block_), this.iconXY_ as Coordinate); + this.paragraphElement_, this.getBlock(), this.iconXY_ as Coordinate); this.applyColour(); } @@ -371,7 +372,7 @@ export class Comment extends Icon { * should not be called directly. Instead call block.setCommentText(null); */ override dispose() { - this.block_.comment = null; + this.getBlock().comment = null; super.dispose(); } } diff --git a/core/extensions.ts b/core/extensions.ts index 99ffa809aca..c30cb314a4e 100644 --- a/core/extensions.ts +++ b/core/extensions.ts @@ -98,7 +98,7 @@ export function registerMutator( // Sanity checks passed. register(name, function(this: Block) { if (hasMutatorDialog) { - this.setMutator(new Mutator(this as BlockSvg, opt_blockList || [])); + this.setMutator(new Mutator(opt_blockList || [], this as BlockSvg)); } // Mixin the object. this.mixin(mixinObj); diff --git a/core/icon.ts b/core/icon.ts index ad726fc6fd3..1512f4a25d6 100644 --- a/core/icon.ts +++ b/core/icon.ts @@ -20,6 +20,7 @@ import * as dom from './utils/dom.js'; import {Size} from './utils/size.js'; import {Svg} from './utils/svg.js'; import * as svgMath from './utils/svg_math.js'; +import * as deprecation from './utils/deprecation.js'; /** @@ -28,7 +29,7 @@ import * as svgMath from './utils/svg_math.js'; * @alias Blockly.Icon */ export abstract class Icon { - protected block_: BlockSvg; + protected block_: BlockSvg|null; /** The icon SVG group. */ iconGroup_: SVGGElement|null = null; @@ -45,7 +46,12 @@ export abstract class Icon { protected iconXY_: Coordinate|null = null; /** @param block The block associated with this icon. */ - constructor(block: BlockSvg) { + constructor(block: BlockSvg|null) { + if (!block) { + deprecation.warn( + 'Calling the Icon constructor with a null block', 'version 9', + 'version 10', 'a non-null block'); + } this.block_ = block; } @@ -62,12 +68,12 @@ export abstract class Icon { */ this.iconGroup_ = dom.createSvgElement(Svg.G, {'class': 'blocklyIconGroup'}); - if (this.block_.isInFlyout) { + if (this.getBlock().isInFlyout) { this.iconGroup_.classList.add('blocklyIconGroupReadonly'); } this.drawIcon_(this.iconGroup_); - this.block_.getSvgRoot().appendChild(this.iconGroup_); + this.getBlock().getSvgRoot().appendChild(this.iconGroup_); browserEvents.conditionalBind( this.iconGroup_, 'mouseup', this, this.iconClick_); this.updateEditable(); @@ -99,19 +105,19 @@ export abstract class Icon { * @param e Mouse click event. */ protected iconClick_(e: MouseEvent) { - if (this.block_.workspace.isDragging()) { + if (this.getBlock().workspace.isDragging()) { // Drag operation is concluding. Don't open the editor. return; } - if (!this.block_.isInFlyout && !browserEvents.isRightButton(e)) { + if (!this.getBlock().isInFlyout && !browserEvents.isRightButton(e)) { this.setVisible(!this.isVisible()); } } /** Change the colour of the associated bubble to match its block. */ applyColour() { - if (this.isVisible()) { - this.bubble_!.setColour(this.block_.style.colourPrimary); + if (this.bubble_ && this.isVisible()) { + this.bubble_.setColour(this.getBlock().style.colourPrimary); } } @@ -122,8 +128,8 @@ export abstract class Icon { */ setIconLocation(xy: Coordinate) { this.iconXY_ = xy; - if (this.isVisible()) { - this.bubble_!.setAnchorLocation(xy); + if (this.bubble_ && this.isVisible()) { + this.bubble_.setAnchorLocation(xy); } } @@ -133,7 +139,7 @@ export abstract class Icon { */ computeIconLocation() { // Find coordinates for the centre of the icon and update the arrow. - const blockXY = this.block_.getRelativeToSurfaceXY(); + const blockXY = this.getBlock().getRelativeToSurfaceXY(); const iconXY = svgMath.getRelativeXY(this.iconGroup_ as SVGElement); const newXY = new Coordinate( blockXY.x + iconXY.x + this.SIZE / 2, @@ -178,5 +184,16 @@ export abstract class Icon { * @param _visible True if the icon should be visible. */ setVisible(_visible: boolean) {} + + /** + * Returns the block this icon is attached to. + */ + protected getBlock(): BlockSvg { + if (!this.block_) { + throw new Error('Block is not set for this icon.'); + } + + return this.block_; + } } // No-op on base class diff --git a/core/mutator.ts b/core/mutator.ts index c470e4898ce..9a3e650c52c 100644 --- a/core/mutator.ts +++ b/core/mutator.ts @@ -32,6 +32,7 @@ import * as dom from './utils/dom.js'; import {Svg} from './utils/svg.js'; import * as toolbox from './utils/toolbox.js'; import * as xml from './utils/xml.js'; +import * as deprecation from './utils/deprecation.js'; import type {WorkspaceSvg} from './workspace_svg.js'; @@ -77,8 +78,14 @@ export class Mutator extends Icon { private updateWorkspacePid_: ReturnType|null = null; /** @param quarkNames List of names of sub-blocks for flyout. */ - constructor(block: BlockSvg, quarkNames: string[]) { - super(block); + constructor(quarkNames: string[], block?: BlockSvg) { + if (!block) { + deprecation.warn( + 'Calling the Mutator constructor without passing the block it is attached to', + 'version 9', 'version 10', + 'the constructor by passing the list of subblocks and the block instance to attach the mutator to'); + } + super(block ?? null); this.quarkNames_ = quarkNames; } @@ -145,7 +152,7 @@ export class Mutator extends Icon { * @param e Mouse click event. */ protected override iconClick_(e: MouseEvent) { - if (this.block_.isEditable()) { + if (this.getBlock().isEditable()) { super.iconClick_(e); } } @@ -175,19 +182,20 @@ export class Mutator extends Icon { } else { quarkXml = null; } + const block = this.getBlock(); const workspaceOptions = new Options(({ // If you want to enable disabling, also remove the // event filter from workspaceChanged_ . 'disable': false, - 'parentWorkspace': this.block_.workspace, - 'media': this.block_.workspace.options.pathToMedia, - 'rtl': this.block_.RTL, + 'parentWorkspace': block.workspace, + 'media': block.workspace.options.pathToMedia, + 'rtl': block.RTL, 'horizontalLayout': false, - 'renderer': this.block_.workspace.options.renderer, - 'rendererOverrides': this.block_.workspace.options.rendererOverrides, + 'renderer': block.workspace.options.renderer, + 'rendererOverrides': block.workspace.options.rendererOverrides, } as BlocklyOptions)); workspaceOptions.toolboxPosition = - this.block_.RTL ? toolbox.Position.RIGHT : toolbox.Position.LEFT; + block.RTL ? toolbox.Position.RIGHT : toolbox.Position.LEFT; const hasFlyout = !!quarkXml; if (hasFlyout) { workspaceOptions.languageTree = toolbox.convertToolboxDefToJson(quarkXml); @@ -227,8 +235,8 @@ export class Mutator extends Icon { /** Add or remove the UI indicating if this icon may be clicked or not. */ override updateEditable() { super.updateEditable(); - if (!this.block_.isInFlyout) { - if (this.block_.isEditable()) { + if (!this.getBlock().isInFlyout) { + if (this.getBlock().isEditable()) { if (this.iconGroup_) { this.iconGroup_.classList.remove('blocklyIconGroupReadonly'); } @@ -255,7 +263,7 @@ export class Mutator extends Icon { height = Math.max(height, flyoutScrollMetrics.height + 20); width += flyout.getWidth(); } - if (this.block_.RTL) { + if (this.getBlock().RTL) { width = -workspaceSize.x; } width += doubleBorderWidth * 3; @@ -275,7 +283,7 @@ export class Mutator extends Icon { this.workspaceWidth_, this.workspaceHeight_); } - if (this.block_.RTL) { + if (this.getBlock().RTL) { // Scroll the workspace to always left-align. const translation = 'translate(' + this.workspaceWidth_ + ',0)'; this.workspace_!.getCanvas().setAttribute('transform', translation); @@ -300,16 +308,16 @@ export class Mutator extends Icon { // No change. return; } + const block = this.getBlock(); eventUtils.fire(new (eventUtils.get(eventUtils.BUBBLE_OPEN))( - this.block_, visible, 'mutator')); + block, visible, 'mutator')); if (visible) { // Create the bubble. this.bubble_ = new Bubble( - (this.block_.workspace as WorkspaceSvg), this.createEditor_(), - this.block_.pathObject.svgPath, (this.iconXY_ as Coordinate), null, - null); + block.workspace, this.createEditor_(), block.pathObject.svgPath, + (this.iconXY_ as Coordinate), null, null); // Expose this mutator's block's ID on its top-level SVG group. - this.bubble_.setSvgId(this.block_.id); + this.bubble_.setSvgId(block.id); this.bubble_.registerMoveEvent(this.onBubbleMove_.bind(this)); const tree = this.workspace_!.options.languageTree; const flyout = this.workspace_!.getFlyout(); @@ -318,7 +326,7 @@ export class Mutator extends Icon { flyout!.show(tree); } - this.rootBlock_ = this.block_!.decompose!(this.workspace_!)!; + this.rootBlock_ = block.decompose!(this.workspace_!)!; const blocks = this.rootBlock_!.getDescendants(false); for (let i = 0, child; child = blocks[i]; i++) { child.render(); @@ -335,20 +343,21 @@ export class Mutator extends Icon { margin = 16; x = margin; } - if (this.block_.RTL) { + if (block.RTL) { x = -x; } this.rootBlock_!.moveBy(x, margin); // Save the initial connections, then listen for further changes. - if (this.block_.saveConnections) { + if (block.saveConnections) { const thisRootBlock = this.rootBlock_; - this.block_.saveConnections(thisRootBlock); + block.saveConnections(thisRootBlock); this.sourceListener_ = () => { - if (this.block_ && this.block_.saveConnections) { - this.block_.saveConnections(thisRootBlock); + const currentBlock = this.getBlock(); + if (currentBlock.saveConnections) { + currentBlock.saveConnections(thisRootBlock); } }; - this.block_.workspace.addChangeListener(this.sourceListener_); + block.workspace.addChangeListener(this.sourceListener_); } this.resizeBubble_(); // When the mutator's workspace changes, update the source block. @@ -367,7 +376,7 @@ export class Mutator extends Icon { this.workspaceWidth_ = 0; this.workspaceHeight_ = 0; if (this.sourceListener_) { - this.block_.workspace.removeChangeListener(this.sourceListener_); + block.workspace.removeChangeListener(this.sourceListener_); this.sourceListener_ = null; } } @@ -438,7 +447,7 @@ export class Mutator extends Icon { if (!existingGroup) { eventUtils.setGroup(true); } - const block = this.block_ as BlockSvg; + const block = this.getBlock(); const oldExtraState = BlockChange.getExtraBlockState_(block); // Switch off rendering while the source block is rebuilt. @@ -482,7 +491,7 @@ export class Mutator extends Icon { /** Dispose of this mutator. */ override dispose() { - this.block_.mutator = null; + this.getBlock().mutator = null; super.dispose(); } diff --git a/core/warning.ts b/core/warning.ts index d02eba5b877..7e207cf4e97 100644 --- a/core/warning.ts +++ b/core/warning.ts @@ -89,7 +89,7 @@ export class Warning extends Icon { return; } eventUtils.fire(new (eventUtils.get(eventUtils.BUBBLE_OPEN))( - this.block_, visible, 'warning')); + this.getBlock(), visible, 'warning')); if (visible) { this.createBubble_(); } else { @@ -101,7 +101,7 @@ export class Warning extends Icon { private createBubble_() { this.paragraphElement_ = Bubble.textToDom(this.getText()); this.bubble_ = Bubble.createNonEditableBubble( - this.paragraphElement_, this.block_, this.iconXY_ as Coordinate); + this.paragraphElement_, this.getBlock(), this.iconXY_ as Coordinate); this.applyColour(); } @@ -151,7 +151,7 @@ export class Warning extends Icon { /** Dispose of this warning. */ override dispose() { - this.block_.warning = null; + this.getBlock().warning = null; super.dispose(); } }