diff --git a/core/block_svg.ts b/core/block_svg.ts index ee21986f848..a4d22b3f128 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -123,6 +123,7 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, /** @internal */ pathObject: IPathObject; override rendered = false; + private visuallyDisabled = false; /** * Is this block currently rendering? Used to stop recursive render calls @@ -889,15 +890,12 @@ export class BlockSvg extends Block implements IASTNodeLocationSvg, * @internal */ updateDisabled() { - const children = (this.getChildren(false)); + const disabled = !this.isEnabled() || this.getInheritedDisabled(); + if (this.visuallyDisabled === disabled) return; this.applyColour(); - if (this.isCollapsed()) { - return; - } - for (let i = 0, child; child = children[i]; i++) { - if (child.rendered) { - child.updateDisabled(); - } + this.visuallyDisabled = disabled; + for (const child of this.getChildren(false)) { + child.updateDisabled(); } } diff --git a/core/connection.ts b/core/connection.ts index 76536aa8bb5..485f3895ec4 100644 --- a/core/connection.ts +++ b/core/connection.ts @@ -93,7 +93,7 @@ export class Connection implements IASTNodeLocationWithBlock { // Make sure the childConnection is available. if (childConnection.isConnected()) { - childConnection.disconnect(); + childConnection.disconnectInternal(false); } // Make sure the parentConnection is available. @@ -104,7 +104,7 @@ export class Connection implements IASTNodeLocationWithBlock { if (target!.isShadow()) { target!.dispose(false); } else { - this.disconnect(); + this.disconnectInternal(); orphan = target; } this.applyShadowState_(shadowState); @@ -243,67 +243,75 @@ export class Connection implements IASTNodeLocationWithBlock { return this.isConnected(); } - /** Disconnect this connection. */ + /** + * Disconnect this connection. + */ disconnect() { - const otherConnection = this.targetConnection; - if (!otherConnection) { - throw Error('Source connection not connected.'); - } - if (otherConnection.targetConnection !== this) { - throw Error('Target connection not connected to source connection.'); - } - let parentBlock; - let childBlock; - let parentConnection; - if (this.isSuperior()) { - // Superior block. - parentBlock = this.sourceBlock_; - childBlock = otherConnection.getSourceBlock(); - /* eslint-disable-next-line @typescript-eslint/no-this-alias */ - parentConnection = this; - } else { - // Inferior block. - parentBlock = otherConnection.getSourceBlock(); - childBlock = this.sourceBlock_; - parentConnection = otherConnection; - } - - const eventGroup = eventUtils.getGroup(); - if (!eventGroup) { - eventUtils.setGroup(true); - } - this.disconnectInternal_(parentBlock, childBlock); - if (!childBlock.isShadow()) { - // If we were disconnecting a shadow, no need to spawn a new one. - parentConnection.respawnShadow_(); - } - if (!eventGroup) { - eventUtils.setGroup(false); - } + this.disconnectInternal(); } /** * Disconnect two blocks that are connected by this connection. * - * @param parentBlock The superior block. - * @param childBlock The inferior block. + * @param setParent Whether to set the parent of the disconnected block or + * not, defaults to true. + * If you do not set the parent, ensure that a subsequent action does, + * otherwise the view and model will be out of sync. */ - protected disconnectInternal_(parentBlock: Block, childBlock: Block) { + protected disconnectInternal(setParent = true) { + const {parentConnection, childConnection} = + this.getParentAndChildConnections(); + if (!parentConnection || !childConnection) { + throw Error('Source connection not connected.'); + } + + const eventGroup = eventUtils.getGroup(); + if (!eventGroup) eventUtils.setGroup(true); + let event; if (eventUtils.isEnabled()) { - event = - new (eventUtils.get(eventUtils.BLOCK_MOVE))(childBlock) as BlockMove; + event = new (eventUtils.get(eventUtils.BLOCK_MOVE))( + childConnection.getSourceBlock()) as BlockMove; } const otherConnection = this.targetConnection; if (otherConnection) { otherConnection.targetConnection = null; } this.targetConnection = null; - childBlock.setParent(null); + if (setParent) childConnection.getSourceBlock().setParent(null); if (event) { event.recordNew(); eventUtils.fire(event); } + + if (!childConnection.getSourceBlock().isShadow()) { + // If we were disconnecting a shadow, no need to spawn a new one. + parentConnection.respawnShadow_(); + } + + if (!eventGroup) eventUtils.setGroup(false); + } + + /** + * Returns the parent connection (superior) and child connection (inferior) + * given this connection and the connection it is connected to. + * + * @returns The parent connection and child connection, given this connection + * and the connection it is connected to. + */ + protected getParentAndChildConnections(): + {parentConnection?: Connection, childConnection?: Connection} { + if (!this.targetConnection) return {}; + if (this.isSuperior()) { + return { + parentConnection: this, + childConnection: this.targetConnection, + }; + } + return { + parentConnection: this.targetConnection, + childConnection: this, + }; } /** diff --git a/core/contextmenu.ts b/core/contextmenu.ts index ec56d90382b..8122b1b03c4 100644 --- a/core/contextmenu.ts +++ b/core/contextmenu.ts @@ -117,11 +117,15 @@ function populate_( if (option.enabled) { const actionHandler = function() { hide(); - // If .scope does not exist on the option, then the callback will not - // be expecting a scope parameter, so there should be no problems. Just - // assume it is a ContextMenuOption and we'll pass undefined if it's - // not. - option.callback((option as ContextMenuOption).scope); + requestAnimationFrame(() => { + setTimeout(() => { + // If .scope does not exist on the option, then the callback + // will not be expecting a scope parameter, so there should be + // no problems. Just assume it is a ContextMenuOption and we'll + // pass undefined if it's not. + option.callback((option as ContextMenuOption).scope); + }, 0); + }); }; menuItem.onAction(actionHandler, {}); } diff --git a/core/insertion_marker_manager.ts b/core/insertion_marker_manager.ts index 20e27d42650..532fb11bed8 100644 --- a/core/insertion_marker_manager.ts +++ b/core/insertion_marker_manager.ts @@ -180,19 +180,14 @@ export class InsertionMarkerManager { */ applyConnections() { if (!this.activeCandidate) return; - // Don't fire events for insertion markers. + const {local, closest} = this.activeCandidate; + local.connect(closest); eventUtils.disable(); this.hidePreview(); eventUtils.enable(); - const {local, closest} = this.activeCandidate; - // Connect two blocks together. - local.connect(closest); if (this.topBlock.rendered) { - // Trigger a connection animation. - // Determine which connection is inferior (lower in the source stack). const inferiorConnection = local.isSuperior() ? closest : local; blockAnimations.connectionUiEffect(inferiorConnection.getSourceBlock()); - // Bring the just-edited stack to the front. const rootBlock = this.topBlock.getRootBlock(); setTimeout(() => { rootBlock.bringToFront(); @@ -604,38 +599,16 @@ export class InsertionMarkerManager { const markerConn = this.markerConnection; const imBlock = markerConn.getSourceBlock(); - const markerNext = imBlock.nextConnection; const markerPrev = imBlock.previousConnection; const markerOutput = imBlock.outputConnection; - const isNext = markerConn === markerNext; - - const isFirstInStatementStack = - isNext && !(markerPrev && markerPrev.targetConnection); - - const isFirstInOutputStack = - markerConn.type === ConnectionType.INPUT_VALUE && - !(markerOutput && markerOutput.targetConnection); - // The insertion marker is the first block in a stack. Unplug won't do - // anything in that case. Instead, unplug the following block. - if (isFirstInStatementStack || isFirstInOutputStack) { - markerConn.targetBlock()!.unplug(false); - } else if (markerConn.type === ConnectionType.NEXT_STATEMENT && !isNext) { - // Inside of a C-block, first statement connection. - const innerConnection = markerConn.targetConnection; - if (innerConnection) { - innerConnection.getSourceBlock().unplug(false); - } - - const previousBlockNextConnection = - markerPrev ? markerPrev.targetConnection : null; - - imBlock.unplug(true); - if (previousBlockNextConnection && innerConnection) { - previousBlockNextConnection.connect(innerConnection); - } + if (!markerPrev?.targetConnection && !markerOutput?.targetConnection) { + // If we are the top block, unplugging doesn't do anything. + // The marker connection may not have a target block if we are hiding + // as part of applying connections. + markerConn.targetBlock()?.unplug(false); } else { - imBlock.unplug(/* healStack */ true); + imBlock.unplug(true); } if (markerConn.targetConnection) { diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index dbe729dd321..583a9aff8e3 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -479,23 +479,27 @@ export class RenderedConnection extends Connection { /** * Disconnect two blocks that are connected by this connection. * - * @param parentBlock The superior block. - * @param childBlock The inferior block. + * @param setParent Whether to set the parent of the disconnected block or + * not, defaults to true. + * If you do not set the parent, ensure that a subsequent action does, + * otherwise the view and model will be out of sync. */ - protected override disconnectInternal_( - parentBlock: Block, childBlock: Block) { - super.disconnectInternal_(parentBlock, childBlock); - const renderedParent = parentBlock as BlockSvg; - const renderedChild = childBlock as BlockSvg; + override disconnectInternal(setParent = true) { + const {parentConnection, childConnection} = + this.getParentAndChildConnections(); + if (!parentConnection || !childConnection) return; + const parent = parentConnection.getSourceBlock() as BlockSvg; + const child = childConnection.getSourceBlock() as BlockSvg; + super.disconnectInternal(setParent); // Rerender the parent so that it may reflow. - if (renderedParent.rendered) { - renderedParent.queueRender(); + if (parent.rendered) { + parent.queueRender(); } - if (renderedChild.rendered) { - renderedChild.updateDisabled(); - renderedChild.queueRender(); + if (child.rendered) { + child.updateDisabled(); + child.queueRender(); // Reset visibility, since the child is now a top block. - renderedChild.getSvgRoot().style.display = 'block'; + child.getSvgRoot().style.display = 'block'; } } diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js index d04868b560d..3c1ecc95c1c 100644 --- a/tests/mocha/block_test.js +++ b/tests/mocha/block_test.js @@ -1825,25 +1825,6 @@ suite('Blocks', function() { chai.assert.isFalse(blockB.disabled); chai.assert.isFalse(blockB.getSvgRoot().classList.contains('blocklyDisabled')); }); - test('Children of Collapsed Block Should Not Update', function() { - const blockA = createRenderedBlock(this.workspace, 'statement_block'); - const blockB = createRenderedBlock(this.workspace, 'stack_block'); - blockA.getInput('STATEMENT').connection - .connect(blockB.previousConnection); - - // Disable the block and collapse it. - blockA.setEnabled(false); - blockA.setCollapsed(true); - - const blockUpdateDisabled = sinon.stub(blockB, 'updateDisabled'); - - // Enable the block before expanding it. - blockA.setEnabled(true); - - // For performance reasons updateDisabled should not be called - // on children of collapsed blocks. - sinon.assert.notCalled(blockUpdateDisabled); - }); test('Disabled Children of Collapsed Blocks Should Stay Disabled', function() { const blockA = createRenderedBlock(this.workspace, 'statement_block'); const blockB = createRenderedBlock(this.workspace, 'stack_block');