From f027b827f0f788746a08169c9e77f8225b1166f8 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 12 Jan 2024 13:43:55 -0800 Subject: [PATCH 1/4] fix: triggering flyout show from field render causing infinite loop (#7784) * fix: triggering flyout show from field render causing infinite loop * chore: add tests for triggering queued renders (cherry picked from commit 5ade042e9596cfb155d080c8cc4f375bc787b078) --- core/flyout_base.ts | 2 +- core/render_management.ts | 21 ++++++--- tests/mocha/render_management_test.js | 65 +++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 76d2e8f8c..1ee1a55d5 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -649,7 +649,7 @@ export abstract class Flyout const parsedContent = toolbox.convertFlyoutDefToJsonArray(flyoutDef); const flyoutInfo = this.createFlyoutInfo(parsedContent); - renderManagement.triggerQueuedRenders(); + renderManagement.triggerQueuedRenders(this.workspace_); this.layout_(flyoutInfo.contents, flyoutInfo.gaps); diff --git a/core/render_management.ts b/core/render_management.ts index 541459860..d3f1ed4f1 100644 --- a/core/render_management.ts +++ b/core/render_management.ts @@ -6,6 +6,7 @@ import {BlockSvg} from './block_svg.js'; import * as userAgent from './utils/useragent.js'; +import type {WorkspaceSvg} from './workspace_svg.js'; /** The set of all blocks in need of rendering which don't have parents. */ const rootBlocks = new Set(); @@ -75,11 +76,13 @@ export function finishQueuedRenders(): Promise { * cases where queueing renders breaks functionality + backwards compatibility * (such as rendering icons). * + * @param workspace If provided, only rerender blocks in this workspace. + * * @internal */ -export function triggerQueuedRenders() { +export function triggerQueuedRenders(workspace?: WorkspaceSvg) { window.cancelAnimationFrame(animationRequestId); - doRenders(); + doRenders(workspace); if (afterRendersResolver) afterRendersResolver(); } @@ -110,10 +113,16 @@ function queueBlock(block: BlockSvg) { /** * Rerenders all of the blocks in the queue. + * + * @param workspace If provided, only rerender blocks in this workspace. */ -function doRenders() { - const workspaces = new Set([...rootBlocks].map((block) => block.workspace)); - const blocks = [...rootBlocks].filter(shouldRenderRootBlock); +function doRenders(workspace?: WorkspaceSvg) { + const workspaces = workspace + ? new Set([workspace]) + : new Set([...rootBlocks].map((block) => block.workspace)); + const blocks = [...rootBlocks] + .filter(shouldRenderRootBlock) + .filter((b) => workspaces.has(b.workspace)); for (const block of blocks) { renderBlock(block); } @@ -126,7 +135,7 @@ function doRenders() { } rootBlocks.clear(); - dirtyBlocks = new Set(); + dirtyBlocks = new WeakSet(); afterRendersPromise = null; } diff --git a/tests/mocha/render_management_test.js b/tests/mocha/render_management_test.js index d5d957df4..4a69d2bab 100644 --- a/tests/mocha/render_management_test.js +++ b/tests/mocha/render_management_test.js @@ -57,4 +57,69 @@ suite('Render Management', function () { return promise; }); }); + + suite('triggering queued renders', function () { + function createMockBlock(ws) { + return { + hasRendered: false, + renderEfficiently: function () { + this.hasRendered = true; + }, + + // All of the APIs the render management system needs. + getParent: () => null, + getChildren: () => [], + isDisposed: () => false, + getRelativeToSurfaceXY: () => ({x: 0, y: 0}), + updateComponentLocations: () => {}, + workspace: ws || createMockWorkspace(), + }; + } + + function createMockWorkspace() { + return { + resizeContents: () => {}, + }; + } + + test('triggering queued renders rerenders blocks', function () { + const block = createMockBlock(); + Blockly.renderManagement.queueRender(block); + + Blockly.renderManagement.triggerQueuedRenders(); + + chai.assert.isTrue(block.hasRendered, 'Expected block to be rendered'); + }); + + test('triggering queued renders rerenders blocks in all workspaces', function () { + const workspace1 = createMockWorkspace(); + const workspace2 = createMockWorkspace(); + const block1 = createMockBlock(workspace1); + const block2 = createMockBlock(workspace2); + Blockly.renderManagement.queueRender(block1); + Blockly.renderManagement.queueRender(block2); + + Blockly.renderManagement.triggerQueuedRenders(); + + chai.assert.isTrue(block1.hasRendered, 'Expected block1 to be rendered'); + chai.assert.isTrue(block2.hasRendered, 'Expected block2 to be rendered'); + }); + + test('triggering queued renders in one workspace does not rerender blocks in another workspace', function () { + const workspace1 = createMockWorkspace(); + const workspace2 = createMockWorkspace(); + const block1 = createMockBlock(workspace1); + const block2 = createMockBlock(workspace2); + Blockly.renderManagement.queueRender(block1); + Blockly.renderManagement.queueRender(block2); + + Blockly.renderManagement.triggerQueuedRenders(workspace1); + + chai.assert.isTrue(block1.hasRendered, 'Expected block1 to be rendered'); + chai.assert.isFalse( + block2.hasRendered, + 'Expected block2 to not be rendered', + ); + }); + }); }); From dccedbfe96159441566083a7d9f2bd32758c587f Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 16 Jan 2024 14:23:58 -0800 Subject: [PATCH 2/4] fix: cancelling all renders on triggering queued renders (#7787) (cherry picked from commit 0d1245c6469667e9acc19887c3f47eea85c3bcbc) --- core/render_management.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/core/render_management.ts b/core/render_management.ts index d3f1ed4f1..f0ec2a133 100644 --- a/core/render_management.ts +++ b/core/render_management.ts @@ -12,7 +12,7 @@ import type {WorkspaceSvg} from './workspace_svg.js'; const rootBlocks = new Set(); /** The set of all blocks in need of rendering. */ -let dirtyBlocks = new WeakSet(); +const dirtyBlocks = new WeakSet(); /** * The promise which resolves after the current set of renders is completed. Or @@ -81,9 +81,9 @@ export function finishQueuedRenders(): Promise { * @internal */ export function triggerQueuedRenders(workspace?: WorkspaceSvg) { - window.cancelAnimationFrame(animationRequestId); + if (!workspace) window.cancelAnimationFrame(animationRequestId); doRenders(workspace); - if (afterRendersResolver) afterRendersResolver(); + if (!workspace && afterRendersResolver) afterRendersResolver(); } /** @@ -134,9 +134,19 @@ function doRenders(workspace?: WorkspaceSvg) { block.updateComponentLocations(blockOrigin); } - rootBlocks.clear(); - dirtyBlocks = new WeakSet(); - afterRendersPromise = null; + for (const block of blocks) { + dequeueBlock(block); + } + if (!workspace) afterRendersPromise = null; +} + +/** Removes the given block and children from the render queue. */ +function dequeueBlock(block: BlockSvg) { + rootBlocks.delete(block); + dirtyBlocks.delete(block); + for (const child of block.getChildren(false)) { + dequeueBlock(child); + } } /** From 960b89e8feb4f7d5394e3fa16bc83d08f75a335d Mon Sep 17 00:00:00 2001 From: Maribeth Bottorff Date: Wed, 17 Jan 2024 12:05:49 -0800 Subject: [PATCH 3/4] fix: first block dragged from flyout will have same id (#7788) (cherry picked from commit 49f65a235c361cbb931042ed9bb6da4c7c94b82a) --- core/flyout_base.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/flyout_base.ts b/core/flyout_base.ts index 1ee1a55d5..77eae8cca 100644 --- a/core/flyout_base.ts +++ b/core/flyout_base.ts @@ -1235,8 +1235,7 @@ export abstract class Flyout } // Clone the block. - // TODO(#7432): Add a saveIds parameter to `save`. - const json = blocks.save(oldBlock, {saveIds: false}) as blocks.State; + const json = blocks.save(oldBlock) as blocks.State; // Normallly this resizes leading to weird jumps. Save it for terminateDrag. targetWorkspace.setResizesEnabled(false); const block = blocks.append(json, targetWorkspace) as BlockSvg; From 498fc2c1ce3fec0a93c23b4d68c6adda207f4005 Mon Sep 17 00:00:00 2001 From: Maribeth Moffatt Date: Wed, 17 Jan 2024 12:31:55 -0800 Subject: [PATCH 4/4] release: Update version number to 10.3.1 --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 63fa0b775..d9fdbeded 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "blockly", - "version": "10.3.0", + "version": "10.3.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "blockly", - "version": "10.3.0", + "version": "10.3.1", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index aaf3fad2a..8c0b05750 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "blockly", - "version": "10.3.0", + "version": "10.3.1", "description": "Blockly is a library for building visual programming editors.", "keywords": [ "blockly"