From 43f6df92a3a7564ad6189f5fca6f077a24400f8e Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 10 Jan 2024 11:13:30 -0800 Subject: [PATCH] fix!: `rendered` meaning (#7747) * fix: make rendered strictly for differentiating blocksvgs * chore: fix references to rendered * chore: fix tests * chore: delete TODO --- core/block.ts | 3 +- core/block_svg.ts | 124 +++++++++----------------- core/inputs/input.ts | 4 +- core/insertion_marker_manager.ts | 20 ++--- core/rendered_connection.ts | 45 +++------- tests/mocha/icon_test.js | 54 ----------- tests/mocha/test_helpers/workspace.js | 115 ++++++++++++++++++++---- tests/mocha/workspace_svg_test.js | 44 ++------- 8 files changed, 168 insertions(+), 241 deletions(-) diff --git a/core/block.ts b/core/block.ts index 8f64b20a2..53a1063d8 100644 --- a/core/block.ts +++ b/core/block.ts @@ -202,7 +202,8 @@ export class Block implements IASTNodeLocation, IDeletable { /** Name of the type of hat. */ hat?: string; - rendered: boolean | null = null; + /** Is this block a BlockSVG? */ + readonly rendered: boolean = false; /** * String for block help, or function that returns a URL. Null for no help. diff --git a/core/block_svg.ts b/core/block_svg.ts index f8ac57684..17e6aa3a4 100644 --- a/core/block_svg.ts +++ b/core/block_svg.ts @@ -117,7 +117,10 @@ export class BlockSvg style: BlockStyle; /** @internal */ pathObject: IPathObject; - override rendered = false; + + /** Is this block a BlockSVG? */ + override readonly rendered = true; + private visuallyDisabled = false; /** @@ -650,12 +653,6 @@ export class BlockSvg * @internal */ updateComponentLocations(blockOrigin: Coordinate) { - if (!this.rendered) { - // Rendering is required to lay out the blocks. - // This is probably an invisible block attached to a collapsed block. - return; - } - if (!this.dragging) this.updateConnectionLocations(blockOrigin); this.updateIconLocations(blockOrigin); this.updateFieldLocations(blockOrigin); @@ -789,7 +786,7 @@ export class BlockSvg Tooltip.dispose(); ContextMenu.hide(); - if (animate && this.rendered) { + if (animate) { this.unplug(healStack); blockAnimations.disposeUiEffect(this); } @@ -806,8 +803,6 @@ export class BlockSvg if (this.isDeadOrDying()) return; super.disposeInternal(); - this.rendered = false; - if (common.getSelected() === this) { this.unselect(); this.workspace.cancelCurrentGesture(); @@ -991,13 +986,11 @@ export class BlockSvg if (icon instanceof MutatorIcon) this.mutator = icon; - if (this.rendered) { - icon.initView(this.createIconPointerDownListener(icon)); - icon.applyColour(); - icon.updateEditable(); - this.queueRender(); - this.bumpNeighbours(); - } + icon.initView(this.createIconPointerDownListener(icon)); + icon.applyColour(); + icon.updateEditable(); + this.queueRender(); + this.bumpNeighbours(); return icon; } @@ -1021,10 +1014,9 @@ export class BlockSvg if (type.equals(MutatorIcon.TYPE)) this.mutator = null; - if (this.rendered) { - this.queueRender(); - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); + return removed; } @@ -1036,7 +1028,7 @@ export class BlockSvg override setEnabled(enabled: boolean) { if (this.isEnabled() !== enabled) { super.setEnabled(enabled); - if (this.rendered && !this.getInheritedDisabled()) { + if (!this.getInheritedDisabled()) { this.updateDisabled(); } } @@ -1049,9 +1041,6 @@ export class BlockSvg * @param highlighted True if highlighted. */ setHighlighted(highlighted: boolean) { - if (!this.rendered) { - return; - } this.pathObject.updateHighlighted(highlighted); } @@ -1183,11 +1172,8 @@ export class BlockSvg opt_check?: string | string[] | null, ) { super.setPreviousStatement(newBoolean, opt_check); - - if (this.rendered) { - this.queueRender(); - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); } /** @@ -1202,11 +1188,8 @@ export class BlockSvg opt_check?: string | string[] | null, ) { super.setNextStatement(newBoolean, opt_check); - - if (this.rendered) { - this.queueRender(); - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); } /** @@ -1221,11 +1204,8 @@ export class BlockSvg opt_check?: string | string[] | null, ) { super.setOutput(newBoolean, opt_check); - - if (this.rendered) { - this.queueRender(); - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); } /** @@ -1235,11 +1215,8 @@ export class BlockSvg */ override setInputsInline(newBoolean: boolean) { super.setInputsInline(newBoolean); - - if (this.rendered) { - this.queueRender(); - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); } /** @@ -1253,13 +1230,8 @@ export class BlockSvg */ override removeInput(name: string, opt_quiet?: boolean): boolean { const removed = super.removeInput(name, opt_quiet); - - if (this.rendered) { - this.queueRender(); - // Removing an input will cause the block to change shape. - this.bumpNeighbours(); - } - + this.queueRender(); + this.bumpNeighbours(); return removed; } @@ -1271,23 +1243,15 @@ export class BlockSvg */ override moveNumberedInputBefore(inputIndex: number, refIndex: number) { super.moveNumberedInputBefore(inputIndex, refIndex); - - if (this.rendered) { - this.queueRender(); - // Moving an input will cause the block to change shape. - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); } /** @override */ override appendInput(input: Input): Input { super.appendInput(input); - - if (this.rendered) { - this.queueRender(); - // Adding an input will cause the block to change shape. - this.bumpNeighbours(); - } + this.queueRender(); + this.bumpNeighbours(); return input; } @@ -1341,28 +1305,25 @@ export class BlockSvg * Returns connections originating from this block. * * @param all If true, return all connections even hidden ones. - * Otherwise, for a non-rendered block return an empty list, and for a - * collapsed block don't return inputs connections. + * Otherwise, for a collapsed block don't return inputs connections. * @returns Array of connections. * @internal */ override getConnections_(all: boolean): RenderedConnection[] { const myConnections = []; - if (all || this.rendered) { - if (this.outputConnection) { - myConnections.push(this.outputConnection); - } - if (this.previousConnection) { - myConnections.push(this.previousConnection); - } - if (this.nextConnection) { - myConnections.push(this.nextConnection); - } - if (all || !this.collapsed_) { - for (let i = 0, input; (input = this.inputList[i]); i++) { - if (input.connection) { - myConnections.push(input.connection as RenderedConnection); - } + if (this.outputConnection) { + myConnections.push(this.outputConnection); + } + if (this.previousConnection) { + myConnections.push(this.previousConnection); + } + if (this.nextConnection) { + myConnections.push(this.nextConnection); + } + if (all || !this.collapsed_) { + for (let i = 0, input; (input = this.inputList[i]); i++) { + if (input.connection) { + myConnections.push(input.connection as RenderedConnection); } } } @@ -1573,7 +1534,6 @@ export class BlockSvg * @internal */ renderEfficiently() { - this.rendered = true; dom.startTextWidthCache(); if (this.isCollapsed()) { diff --git a/core/inputs/input.ts b/core/inputs/input.ts index 96498ec1b..a94102aca 100644 --- a/core/inputs/input.ts +++ b/core/inputs/input.ts @@ -124,7 +124,6 @@ export class Input { if (this.sourceBlock.rendered) { (this.sourceBlock as BlockSvg).queueRender(); - // Adding a field will cause the block to change shape. this.sourceBlock.bumpNeighbours(); } return index; @@ -146,7 +145,6 @@ export class Input { this.fieldRow.splice(i, 1); if (this.sourceBlock.rendered) { (this.sourceBlock as BlockSvg).queueRender(); - // Removing a field will cause the block to change shape. this.sourceBlock.bumpNeighbours(); } return true; @@ -274,7 +272,7 @@ export class Input { /** Initialize the fields on this input. */ init() { - if (!this.sourceBlock.workspace.rendered) { + if (!this.sourceBlock.rendered) { return; // Headless blocks don't need fields initialized. } for (let i = 0; i < this.fieldRow.length; i++) { diff --git a/core/insertion_marker_manager.ts b/core/insertion_marker_manager.ts index cc507bd70..302dd47c6 100644 --- a/core/insertion_marker_manager.ts +++ b/core/insertion_marker_manager.ts @@ -176,18 +176,16 @@ export class InsertionMarkerManager { eventUtils.enable(); const {local, closest} = this.activeCandidate; local.connect(closest); - if (this.topBlock.rendered) { - const inferiorConnection = local.isSuperior() ? closest : local; - const rootBlock = this.topBlock.getRootBlock(); + const inferiorConnection = local.isSuperior() ? closest : local; + const rootBlock = this.topBlock.getRootBlock(); - finishQueuedRenders().then(() => { - blockAnimations.connectionUiEffect(inferiorConnection.getSourceBlock()); - // bringToFront is incredibly expensive. Delay until the next frame. - setTimeout(() => { - rootBlock.bringToFront(); - }, 0); - }); - } + finishQueuedRenders().then(() => { + blockAnimations.connectionUiEffect(inferiorConnection.getSourceBlock()); + // bringToFront is incredibly expensive. Delay until the next frame. + setTimeout(() => { + rootBlock.bringToFront(); + }, 0); + }); } /** diff --git a/core/rendered_connection.ts b/core/rendered_connection.ts index 08c0471a2..e68450883 100644 --- a/core/rendered_connection.ts +++ b/core/rendered_connection.ts @@ -502,16 +502,12 @@ export class RenderedConnection extends Connection { const parent = parentConnection.getSourceBlock() as BlockSvg; const child = childConnection.getSourceBlock() as BlockSvg; super.disconnectInternal(setParent); - // Rerender the parent so that it may reflow. - if (parent.rendered) { - parent.queueRender(); - } - if (child.rendered) { - child.updateDisabled(); - child.queueRender(); - // Reset visibility, since the child is now a top block. - child.getSvgRoot().style.display = 'block'; - } + + parent.queueRender(); + child.updateDisabled(); + child.queueRender(); + // Reset visibility, since the child is now a top block. + child.getSvgRoot().style.display = 'block'; } /** @@ -554,29 +550,10 @@ export class RenderedConnection extends Connection { const parentBlock = this.getSourceBlock(); const childBlock = renderedChildConnection.getSourceBlock(); - const parentRendered = parentBlock.rendered; - const childRendered = childBlock.rendered; - if (parentRendered) { - parentBlock.updateDisabled(); - } - if (childRendered) { - childBlock.updateDisabled(); - } - if (parentRendered && childRendered) { - if ( - this.type === ConnectionType.NEXT_STATEMENT || - this.type === ConnectionType.PREVIOUS_STATEMENT - ) { - // Child block may need to square off its corners if it is in a stack. - // Rendering a child will render its parent. - childBlock.queueRender(); - } else { - // Child block does not change shape. Rendering the parent node will - // move its connected children into position. - parentBlock.queueRender(); - } - } + parentBlock.updateDisabled(); + childBlock.updateDisabled(); + childBlock.queueRender(); // The input the child block is connected to (if any). const parentInput = parentBlock.getInputWithBlock(childBlock); @@ -617,9 +594,7 @@ export class RenderedConnection extends Connection { */ override setCheck(check: string | string[] | null): RenderedConnection { super.setCheck(check); - if (this.sourceBlock_.rendered) { - this.sourceBlock_.queueRender(); - } + this.sourceBlock_.queueRender(); return this; } } diff --git a/tests/mocha/icon_test.js b/tests/mocha/icon_test.js index 4ff27997f..63c723495 100644 --- a/tests/mocha/icon_test.js +++ b/tests/mocha/icon_test.js @@ -68,24 +68,6 @@ suite('Icon', function () { ); }); - test('initView is called by headful blocks during initSvg', function () { - const workspace = createWorkspaceSvg(); - const block = createUninitializedBlock(workspace); - const icon = new MockIcon(); - const initViewSpy = sinon.spy(icon, 'initView'); - - block.addIcon(icon); - chai.assert.isFalse( - initViewSpy.called, - 'Expected initView to not be called before initing svg', - ); - block.initSvg(); - chai.assert.isTrue( - initViewSpy.calledOnce, - 'Expected initView to be called', - ); - }); - test( 'initView is called by headful blocks that are currently ' + 'rendered when the icon is added', @@ -120,24 +102,6 @@ suite('Icon', function () { ); }); - test('applyColour is called by headful blocks during initSvg', function () { - const workspace = createWorkspaceSvg(); - const block = createUninitializedBlock(workspace); - const icon = new MockIcon(); - const applyColourSpy = sinon.spy(icon, 'applyColour'); - - block.addIcon(icon); - chai.assert.isFalse( - applyColourSpy.called, - 'Expected applyCOlour to not be called before initing svg', - ); - block.initSvg(); - chai.assert.isTrue( - applyColourSpy.calledOnce, - 'Expected applyColour to be called', - ); - }); - test( 'applyColour is called by headful blocks that are currently ' + 'rendered when the icon is added', @@ -231,24 +195,6 @@ suite('Icon', function () { ); }); - test('updateEditable is called by headful blocks during initSvg', function () { - const workspace = createWorkspaceSvg(); - const block = createUninitializedBlock(workspace); - const icon = new MockIcon(); - const updateEditableSpy = sinon.spy(icon, 'updateEditable'); - - block.addIcon(icon); - chai.assert.isFalse( - updateEditableSpy.called, - 'Expected updateEditable to not be called before initing svg', - ); - block.initSvg(); - chai.assert.isTrue( - updateEditableSpy.calledOnce, - 'Expected updateEditable to be called', - ); - }); - test( 'updateEditable is called by headful blocks that are currently ' + 'rendered when the icon is added', diff --git a/tests/mocha/test_helpers/workspace.js b/tests/mocha/test_helpers/workspace.js index 7654feba2..6da113f8b 100644 --- a/tests/mocha/test_helpers/workspace.js +++ b/tests/mocha/test_helpers/workspace.js @@ -781,7 +781,9 @@ export function testAWorkspace() { const xml = Blockly.utils.xml.textToDom(xmlText); Blockly.Xml.domToBlock(xml, this.workspace); this.workspace.getTopBlocks()[0].dispose(false); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); const newXml = Blockly.Xml.workspaceToDom(this.workspace); assertNodesEqual(newXml.firstChild, xml); } @@ -909,11 +911,14 @@ export function testAWorkspace() { function testUndoConnect(xmlText, parentId, childId, func) { const xml = Blockly.utils.xml.textToDom(xmlText); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); const parent = this.workspace.getBlockById(parentId); const child = this.workspace.getBlockById(childId); func.call(this, parent, child); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); const newXml = Blockly.Xml.workspaceToDom(this.workspace); assertNodesEqual(newXml, xml); @@ -922,8 +927,8 @@ export function testAWorkspace() { test('Stack', function () { const xml = '' + - ' ' + - ' ' + + ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -934,8 +939,8 @@ export function testAWorkspace() { test('Row', function () { const xml = '' + - ' ' + - ' ' + + ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -946,8 +951,8 @@ export function testAWorkspace() { test('Statement', function () { const xml = '' + - ' ' + - ' ' + + ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -960,12 +965,12 @@ export function testAWorkspace() { test('Stack w/ child', function () { const xml = '' + - ' ' + + ' ' + ' ' + ' ' + ' ' + ' ' + - ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -976,12 +981,12 @@ export function testAWorkspace() { test('Row w/ child', function () { const xml = '' + - ' ' + + ' ' + ' ' + ' ' + ' ' + ' ' + - ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -992,12 +997,12 @@ export function testAWorkspace() { test('Statement w/ child', function () { const xml = '' + - ' ' + + ' ' + ' ' + ' ' + ' ' + ' ' + - ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -1010,12 +1015,12 @@ export function testAWorkspace() { test('Stack w/ shadow', function () { const xml = '' + - ' ' + + ' ' + ' ' + ' ' + ' ' + ' ' + - ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -1026,12 +1031,12 @@ export function testAWorkspace() { test('Row w/ shadow', function () { const xml = '' + - ' ' + + ' ' + ' ' + ' ' + ' ' + ' ' + - ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -1042,12 +1047,12 @@ export function testAWorkspace() { test('Statement w/ shadow', function () { const xml = '' + - ' ' + + ' ' + ' ' + ' ' + ' ' + ' ' + - ' ' + + ' ' + ''; testUndoConnect.call(this, xml, '1', '2', (parent, child) => { @@ -1102,6 +1107,7 @@ export function testAWorkspace() { function testUndoDisconnect(xmlText, childId) { const xml = Blockly.utils.xml.textToDom(xmlText); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); const child = this.workspace.getBlockById(childId); if (child.outputConnection) { @@ -1109,7 +1115,9 @@ export function testAWorkspace() { } else { child.previousConnection.disconnect(); } + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); const newXml = Blockly.Xml.workspaceToDom(this.workspace); assertNodesEqual(newXml, xml); @@ -1262,8 +1270,10 @@ export function testAWorkspace() { suite('createVariable', function () { test('Undo only', function () { createTwoVarsDifferentTypes(this.workspace); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); chai.assert.isNull(this.workspace.getVariableById('id2')); @@ -1274,8 +1284,10 @@ export function testAWorkspace() { test('Undo and redo', function () { createTwoVarsDifferentTypes(this.workspace); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); chai.assert.isNull(this.workspace.getVariableById('id2')); @@ -1300,14 +1312,18 @@ export function testAWorkspace() { suite('deleteVariableById', function () { test('Undo only no usages', function () { createTwoVarsDifferentTypes(this.workspace); + this.clock.runAll(); this.workspace.deleteVariableById('id1'); this.workspace.deleteVariableById('id2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); chai.assert.isNull(this.workspace.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); }); @@ -1316,15 +1332,19 @@ export function testAWorkspace() { createTwoVarsDifferentTypes(this.workspace); // Create blocks to refer to both of them. createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); + this.clock.runAll(); this.workspace.deleteVariableById('id1'); this.workspace.deleteVariableById('id2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); chai.assert.isNull(this.workspace.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); assertBlockVarModelName(this.workspace, 1, 'name1'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); @@ -1333,24 +1353,31 @@ export function testAWorkspace() { test('Reference exists no usages', function () { createTwoVarsDifferentTypes(this.workspace); + this.clock.runAll(); this.workspace.deleteVariableById('id1'); this.workspace.deleteVariableById('id2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); chai.assert.isNull(this.workspace.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); + this.clock.runAll(); // Expect that both variables are deleted chai.assert.isNull(this.workspace.getVariableById('id1')); chai.assert.isNull(this.workspace.getVariableById('id2')); this.workspace.undo(); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); + this.clock.runAll(); // Expect that variable 'id2' is recreated chai.assert.isNull(this.workspace.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); @@ -1360,28 +1387,35 @@ export function testAWorkspace() { createTwoVarsDifferentTypes(this.workspace); // Create blocks to refer to both of them. createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); + this.clock.runAll(); this.workspace.deleteVariableById('id1'); this.workspace.deleteVariableById('id2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); chai.assert.isNull(this.workspace.getVariableById('id1')); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); + this.clock.runAll(); // Expect that both variables are deleted chai.assert.equal(this.workspace.getTopBlocks(false).length, 0); chai.assert.isNull(this.workspace.getVariableById('id1')); chai.assert.isNull(this.workspace.getVariableById('id2')); this.workspace.undo(); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); assertBlockVarModelName(this.workspace, 1, 'name1'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); + this.clock.runAll(); // Expect that variable 'id2' is recreated assertBlockVarModelName(this.workspace, 0, 'name2'); chai.assert.isNull(this.workspace.getVariableById('id1')); @@ -1391,6 +1425,7 @@ export function testAWorkspace() { test('Delete same variable twice no usages', function () { this.workspace.createVariable('name1', 'type1', 'id1'); this.workspace.deleteVariableById('id1'); + this.clock.runAll(); const workspace = this.workspace; assertWarnings(() => { workspace.deleteVariableById('id1'); @@ -1406,21 +1441,26 @@ export function testAWorkspace() { // Undo delete this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); // Redo delete this.workspace.undo(true); + this.clock.runAll(); chai.assert.isNull(this.workspace.getVariableById('id1')); // Redo delete, nothing should happen this.workspace.undo(true); + this.clock.runAll(); chai.assert.isNull(this.workspace.getVariableById('id1')); }); test('Delete same variable twice with usages', function () { this.workspace.createVariable('name1', 'type1', 'id1'); createVarBlocksNoEvents(this.workspace, ['id1']); + this.clock.runAll(); this.workspace.deleteVariableById('id1'); + this.clock.runAll(); const workspace = this.workspace; assertWarnings(() => { workspace.deleteVariableById('id1'); @@ -1437,16 +1477,19 @@ export function testAWorkspace() { // Undo delete this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name1'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); // Redo delete this.workspace.undo(true); + this.clock.runAll(); chai.assert.equal(this.workspace.getTopBlocks(false).length, 0); chai.assert.isNull(this.workspace.getVariableById('id1')); // Redo delete, nothing should happen this.workspace.undo(true); + this.clock.runAll(); chai.assert.equal(this.workspace.getTopBlocks(false).length, 0); chai.assert.isNull(this.workspace.getVariableById('id1')); }); @@ -1459,46 +1502,58 @@ export function testAWorkspace() { test('Reference exists no usages rename to name2', function () { this.workspace.renameVariableById('id1', 'name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'name2', 'type1', 'id1'); }); test('Reference exists with usages rename to name2', function () { createVarBlocksNoEvents(this.workspace, ['id1']); this.workspace.renameVariableById('id1', 'name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name1'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); this.workspace.undo(true); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name2'); assertVariableValues(this.workspace, 'name2', 'type1', 'id1'); }); test('Reference exists different capitalization no usages rename to Name1', function () { this.workspace.renameVariableById('id1', 'Name1'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'Name1', 'type1', 'id1'); }); test('Reference exists different capitalization with usages rename to Name1', function () { createVarBlocksNoEvents(this.workspace, ['id1']); this.workspace.renameVariableById('id1', 'Name1'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name1'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); this.workspace.undo(true); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'Name1'); assertVariableValues(this.workspace, 'Name1', 'type1', 'id1'); }); @@ -1507,12 +1562,15 @@ export function testAWorkspace() { test('Same type no usages rename variable with id1 to name2', function () { this.workspace.createVariable('name2', 'type1', 'id2'); this.workspace.renameVariableById('id1', 'name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); chai.assert.isNull(this.workspace.getVariableById('id1')); }); @@ -1521,14 +1579,17 @@ export function testAWorkspace() { this.workspace.createVariable('name2', 'type1', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); this.workspace.renameVariableById('id1', 'name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name1'); assertBlockVarModelName(this.workspace, 1, 'name2'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); chai.assert.isNull(this.workspace.getVariableById('id1')); }); @@ -1536,12 +1597,15 @@ export function testAWorkspace() { test('Same type different capitalization no usages rename variable with id1 to Name2', function () { this.workspace.createVariable('name2', 'type1', 'id2'); this.workspace.renameVariableById('id1', 'Name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'Name2', 'type1', 'id2'); chai.assert.isNull(this.workspace.getVariable('name1')); }); @@ -1550,14 +1614,17 @@ export function testAWorkspace() { this.workspace.createVariable('name2', 'type1', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); this.workspace.renameVariableById('id1', 'Name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertBlockVarModelName(this.workspace, 0, 'name1'); assertBlockVarModelName(this.workspace, 1, 'name2'); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type1', 'id2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'Name2', 'type1', 'id2'); chai.assert.isNull(this.workspace.getVariableById('id1')); assertBlockVarModelName(this.workspace, 0, 'Name2'); @@ -1567,12 +1634,15 @@ export function testAWorkspace() { test('Different type no usages rename variable with id1 to name2', function () { this.workspace.createVariable('name2', 'type2', 'id2'); this.workspace.renameVariableById('id1', 'name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'name2', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); }); @@ -1581,14 +1651,17 @@ export function testAWorkspace() { this.workspace.createVariable('name2', 'type2', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); this.workspace.renameVariableById('id1', 'name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); assertBlockVarModelName(this.workspace, 0, 'name1'); assertBlockVarModelName(this.workspace, 1, 'name2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'name2', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); assertBlockVarModelName(this.workspace, 0, 'name2'); @@ -1598,12 +1671,15 @@ export function testAWorkspace() { test('Different type different capitalization no usages rename variable with id1 to Name2', function () { this.workspace.createVariable('name2', 'type2', 'id2'); this.workspace.renameVariableById('id1', 'Name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'Name2', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); }); @@ -1612,14 +1688,17 @@ export function testAWorkspace() { this.workspace.createVariable('name2', 'type2', 'id2'); createVarBlocksNoEvents(this.workspace, ['id1', 'id2']); this.workspace.renameVariableById('id1', 'Name2'); + this.clock.runAll(); this.workspace.undo(); + this.clock.runAll(); assertVariableValues(this.workspace, 'name1', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); assertBlockVarModelName(this.workspace, 0, 'name1'); assertBlockVarModelName(this.workspace, 1, 'name2'); this.workspace.undo(true); + this.clock.runAll(); assertVariableValues(this.workspace, 'Name2', 'type1', 'id1'); assertVariableValues(this.workspace, 'name2', 'type2', 'id2'); assertBlockVarModelName(this.workspace, 0, 'Name2'); diff --git a/tests/mocha/workspace_svg_test.js b/tests/mocha/workspace_svg_test.js index cea58cdf9..98ca14d57 100644 --- a/tests/mocha/workspace_svg_test.js +++ b/tests/mocha/workspace_svg_test.js @@ -21,7 +21,7 @@ import {testAWorkspace} from './test_helpers/workspace.js'; suite('WorkspaceSvg', function () { setup(function () { - sharedTestSetup.call(this); + this.clock = sharedTestSetup.call(this, {fireEventsNow: false}).clock; const toolbox = document.getElementById('toolbox-categories'); this.workspace = Blockly.inject('blocklyDiv', {toolbox: toolbox}); Blockly.defineBlocksWithJsonArray([ @@ -168,8 +168,7 @@ suite('WorkspaceSvg', function () { }); suite('Viewport change events', function () { - function resetEventHistory(eventsFireStub, changeListenerSpy) { - eventsFireStub.resetHistory(); + function resetEventHistory(changeListenerSpy) { changeListenerSpy.resetHistory(); } function assertSpyFiredViewportEvent(spy, workspace, expectedProperties) { @@ -187,7 +186,6 @@ suite('WorkspaceSvg', function () { ); } function assertViewportEventFired( - eventsFireStub, changeListenerSpy, workspace, expectedEventCount = 1, @@ -200,32 +198,25 @@ suite('WorkspaceSvg', function () { viewLeft: metrics.viewLeft, type: eventUtils.VIEWPORT_CHANGE, }; - assertSpyFiredViewportEvent( - eventsFireStub, - workspace, - expectedProperties, - ); assertSpyFiredViewportEvent( changeListenerSpy, workspace, expectedProperties, ); sinon.assert.callCount(changeListenerSpy, expectedEventCount); - sinon.assert.callCount(eventsFireStub, expectedEventCount); } function runViewportEventTest( eventTriggerFunc, - eventsFireStub, changeListenerSpy, workspace, clock, expectedEventCount = 1, ) { clock.runAll(); - resetEventHistory(eventsFireStub, changeListenerSpy); + resetEventHistory(changeListenerSpy); eventTriggerFunc(); + clock.runAll(); assertViewportEventFired( - eventsFireStub, changeListenerSpy, workspace, expectedEventCount, @@ -243,7 +234,6 @@ suite('WorkspaceSvg', function () { test('setScale', function () { runViewportEventTest( () => this.workspace.setScale(2), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -252,7 +242,6 @@ suite('WorkspaceSvg', function () { test('zoom(50, 50, 1)', function () { runViewportEventTest( () => this.workspace.zoom(50, 50, 1), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -261,7 +250,6 @@ suite('WorkspaceSvg', function () { test('zoom(50, 50, -1)', function () { runViewportEventTest( () => this.workspace.zoom(50, 50, -1), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -270,7 +258,6 @@ suite('WorkspaceSvg', function () { test('zoomCenter(1)', function () { runViewportEventTest( () => this.workspace.zoomCenter(1), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -279,7 +266,6 @@ suite('WorkspaceSvg', function () { test('zoomCenter(-1)', function () { runViewportEventTest( () => this.workspace.zoomCenter(-1), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -291,7 +277,6 @@ suite('WorkspaceSvg', function () { block.render(); runViewportEventTest( () => this.workspace.zoomToFit(), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -305,7 +290,6 @@ suite('WorkspaceSvg', function () { block.render(); runViewportEventTest( () => this.workspace.centerOnBlock(block.id), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -314,7 +298,6 @@ suite('WorkspaceSvg', function () { test('scroll', function () { runViewportEventTest( () => this.workspace.scroll(50, 50), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -323,7 +306,6 @@ suite('WorkspaceSvg', function () { test('scrollCenter', function () { runViewportEventTest( () => this.workspace.scrollCenter(), - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -336,13 +318,12 @@ suite('WorkspaceSvg', function () { block.initSvg(); block.render(); this.clock.runAll(); - resetEventHistory(this.eventsFireStub, this.changeListenerSpy); + resetEventHistory(this.changeListenerSpy); // Expect 2 events, 1 move, 1 viewport runViewportEventTest( () => { block.moveBy(1000, 1000); }, - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock, @@ -366,7 +347,7 @@ suite('WorkspaceSvg', function () { '', ); this.clock.runAll(); - resetEventHistory(this.eventsFireStub, this.changeListenerSpy); + resetEventHistory(this.changeListenerSpy); // Add block in center of other blocks, not triggering scroll. Blockly.Xml.domToWorkspace( Blockly.utils.xml.textToDom( @@ -375,11 +356,6 @@ suite('WorkspaceSvg', function () { this.workspace, ); this.clock.runAll(); - assertEventNotFired( - this.eventsFireStub, - Blockly.Events.ViewportChange, - {type: eventUtils.VIEWPORT_CHANGE}, - ); assertEventNotFired( this.changeListenerSpy, Blockly.Events.ViewportChange, @@ -403,15 +379,10 @@ suite('WorkspaceSvg', function () { '', ); this.clock.runAll(); - resetEventHistory(this.eventsFireStub, this.changeListenerSpy); + resetEventHistory(this.changeListenerSpy); // Add block in center of other blocks, not triggering scroll. Blockly.Xml.domToWorkspace(xmlDom, this.workspace); this.clock.runAll(); - assertEventNotFired( - this.eventsFireStub, - Blockly.Events.ViewportChange, - {type: eventUtils.VIEWPORT_CHANGE}, - ); assertEventNotFired( this.changeListenerSpy, Blockly.Events.ViewportChange, @@ -436,7 +407,6 @@ suite('WorkspaceSvg', function () { // Expect 10 events, 4 create, 4 move, 1 viewport, 1 finished loading runViewportEventTest( addingMultipleBlocks, - this.eventsFireStub, this.changeListenerSpy, this.workspace, this.clock,