CodingPirates/blockly
1
0
Fork 0
mirror of https://github.com/google/blockly.git synced 2026-01-08 01:20:12 +01:00
Code Issues Packages Projects Releases Wiki Activity

fix: improve performance of connecting blocks (#6876)

Browse Source 
* fix: early return from updateDisabled if it is noop

* chore: trigger connect and disconnect before hiding

* chore: remove disconnectInternal

* fix: skip setting parent when disconnecting before connecting

* chore: fixup docs

* chore: remove erroneous test

* fix: add delay to context menu callback.

Improve INP by allowing the browser to do a paint (closing the context
menu) before we trigger callbacks. This improves the user experience
for expensive callbacks (e.g. collapsing, or updating disabled).

* fix: rendering bug

When disconnecting the last block in the stack, the block would not be
rerendered correctly (the top-start corner would not be reshaped)

* fix: connecting bug

The order for applying connections was changed so that connections were
applied and then the insertion marker was hidden. This caused an error
because hiding the insertion marker expected there to be a child block
when there was not.

* chore: remove setParent param from public API

* chore: tsdoc
This commit is contained in:
Beka Westberg
2023-03-16 13:40:33 -07:00
committed by GitHub
parent 4efd2042f9
commit c2919c51bd
6 changed files with 92 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
core/block_svg.ts
View File

@@ -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
@@ -961,15 +962,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();
}
}

96
core/connection.ts
View File

@@ -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,
};
}
/**

14
core/contextmenu.ts
View File

@@ -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, {});
}

43
core/insertion_marker_manager.ts
View File

@@ -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();
// bringToFront is incredibly expensive. Delay by at least a frame.
@@ -608,38 +603,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) {

30
core/rendered_connection.ts
View File

@@ -477,23 +477,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';
}
}

19
tests/mocha/block_test.js
View File

@@ -1828,25 +1828,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');