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
google · Jan 12, 2024 · 5ade042 · 5ade042
1 parent 7243b48
commit 5ade042
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 7 deletions.
diff --git 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
@@ -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<BlockSvg>();
@@ -75,11 +76,13 @@ export function finishQueuedRenders(): Promise<void> {
  * 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
@@ -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',
+      );
+    });
+  });
 });