refactor: Make focusable elements responsible for scrolling themselves into bounds. (#9288)

* refactor: Make focusable elements responsible for scrolling themselves into bounds.

* chore: Add tests for scrolling focused elements into view.

* fix: Removed inadvertent `.only`.

* fix: Scroll parent block of connections into bounds on focus.

core/block_svg.ts

@@ -1844,6 +1844,9 @@ export class BlockSvg
   /** See IFocusableNode.onNodeFocus. */
   onNodeFocus(): void {
     this.select();
+    this.workspace.scrollBoundsIntoView(
+      this.getBoundingRectangleWithoutChildren(),
+    );
   }
 
   /** See IFocusableNode.onNodeBlur. */

core/bubbles/bubble.ts

@@ -707,6 +707,10 @@ export abstract class Bubble implements IBubble, ISelectable, IFocusableNode {
   onNodeFocus(): void {
     this.select();
     this.bringToFront();
+    const xy = this.getRelativeToSurfaceXY();
+    const size = this.getSize();
+    const bounds = new Rect(xy.y, xy.y + size.height, xy.x, xy.x + size.width);
+    this.workspace.scrollBoundsIntoView(bounds);
   }
 
   /** See IFocusableNode.onNodeBlur. */

core/comments/comment_bar_button.ts

@@ -87,7 +87,13 @@ export abstract class CommentBarButton implements IFocusableNode {
   }
 
   /** Called when this button's focusable DOM element gains focus. */
-  onNodeFocus() {}
+  onNodeFocus() {
+    const commentView = this.getCommentView();
+    const xy = commentView.getRelativeToSurfaceXY();
+    const size = commentView.getSize();
+    const bounds = new Rect(xy.y, xy.y + size.height, xy.x, xy.x + size.width);
+    commentView.workspace.scrollBoundsIntoView(bounds);
+  }
 
   /** Called when this button's focusable DOM element loses focus. */
   onNodeBlur() {}

core/comments/comment_editor.ts

@@ -10,8 +10,10 @@ import {IFocusableNode} from '../interfaces/i_focusable_node.js';
 import {IFocusableTree} from '../interfaces/i_focusable_tree.js';
 import * as touch from '../touch.js';
 import * as dom from '../utils/dom.js';
+import {Rect} from '../utils/rect.js';
 import {Size} from '../utils/size.js';
 import {Svg} from '../utils/svg.js';
+import * as svgMath from '../utils/svg_math.js';
 import {WorkspaceSvg} from '../workspace_svg.js';
 
 /**
@@ -188,7 +190,16 @@ export class CommentEditor implements IFocusableNode {
   getFocusableTree(): IFocusableTree {
     return this.workspace;
   }
-  onNodeFocus(): void {}
+  onNodeFocus(): void {
+    const bbox = Rect.from(this.foreignObject.getBoundingClientRect());
+    this.workspace.scrollBoundsIntoView(
+      Rect.createFromPoint(
+        svgMath.screenToWsCoordinates(this.workspace, bbox.getOrigin()),
+        bbox.getWidth(),
+        bbox.getHeight(),
+      ),
+    );
+  }
   onNodeBlur(): void {}
   canBeFocused(): boolean {
     if (this.id) return true;

core/comments/rendered_workspace_comment.ts

@@ -347,6 +347,7 @@ export class RenderedWorkspaceComment
     this.select();
     // Ensure that the comment is always at the top when focused.
     this.workspace.getLayerManager()?.append(this, layers.BLOCK);
+    this.workspace.scrollBoundsIntoView(this.getBoundingRectangle());
   }
 
   /** See IFocusableNode.onNodeBlur. */

core/field.ts

@@ -1380,7 +1380,12 @@ export abstract class Field<T = any>
   }
 
   /** See IFocusableNode.onNodeFocus. */
-  onNodeFocus(): void {}
+  onNodeFocus(): void {
+    const block = this.getSourceBlock() as BlockSvg;
+    block.workspace.scrollBoundsIntoView(
+      block.getBoundingRectangleWithoutChildren(),
+    );
+  }
 
   /** See IFocusableNode.onNodeBlur. */
   onNodeBlur(): void {}

core/flyout_button.ts

@@ -398,7 +398,11 @@ export class FlyoutButton
   }
 
   /** See IFocusableNode.onNodeFocus. */
-  onNodeFocus(): void {}
+  onNodeFocus(): void {
+    const xy = this.getPosition();
+    const bounds = new Rect(xy.y, xy.y + this.height, xy.x, xy.x + this.width);
+    this.workspace.scrollBoundsIntoView(bounds);
+  }
 
   /** See IFocusableNode.onNodeBlur. */
   onNodeBlur(): void {}

core/icons/icon.ts

@@ -14,6 +14,7 @@ import * as tooltip from '../tooltip.js';
 import {Coordinate} from '../utils/coordinate.js';
 import * as dom from '../utils/dom.js';
 import * as idGenerator from '../utils/idgenerator.js';
+import {Rect} from '../utils/rect.js';
 import {Size} from '../utils/size.js';
 import {Svg} from '../utils/svg.js';
 import type {WorkspaceSvg} from '../workspace_svg.js';
@@ -168,7 +169,16 @@ export abstract class Icon implements IIcon {
   }
 
   /** See IFocusableNode.onNodeFocus. */
-  onNodeFocus(): void {}
+  onNodeFocus(): void {
+    const blockBounds = (this.sourceBlock as BlockSvg).getBoundingRectangle();
+    const bounds = new Rect(
+      blockBounds.top + this.offsetInBlock.y,
+      blockBounds.top + this.offsetInBlock.y + this.getSize().height,
+      blockBounds.left + this.offsetInBlock.x,
+      blockBounds.left + this.offsetInBlock.x + this.getSize().width,
+    );
+    (this.sourceBlock as BlockSvg).workspace.scrollBoundsIntoView(bounds);
+  }
 
   /** See IFocusableNode.onNodeBlur. */
   onNodeBlur(): void {}

core/keyboard_nav/line_cursor.ts

@@ -14,14 +14,11 @@
  */
 
 import {BlockSvg} from '../block_svg.js';
-import {CommentBarButton} from '../comments/comment_bar_button.js';
 import {RenderedWorkspaceComment} from '../comments/rendered_workspace_comment.js';
-import {Field} from '../field.js';
 import {getFocusManager} from '../focus_manager.js';
 import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
 import * as registry from '../registry.js';
-import {Rect} from '../utils/rect.js';
-import {WorkspaceSvg} from '../workspace_svg.js';
+import type {WorkspaceSvg} from '../workspace_svg.js';
 import {Marker} from './marker.js';
 
 /**
@@ -392,31 +389,6 @@ export class LineCursor extends Marker {
    */
   setCurNode(newNode: IFocusableNode) {
     getFocusManager().focusNode(newNode);
-
-    // Try to scroll cursor into view.
-    if (newNode instanceof BlockSvg) {
-      newNode.workspace.scrollBoundsIntoView(
-        newNode.getBoundingRectangleWithoutChildren(),
-      );
-    } else if (newNode instanceof Field) {
-      const block = newNode.getSourceBlock() as BlockSvg;
-      block.workspace.scrollBoundsIntoView(
-        block.getBoundingRectangleWithoutChildren(),
-      );
-    } else if (newNode instanceof RenderedWorkspaceComment) {
-      newNode.workspace.scrollBoundsIntoView(newNode.getBoundingRectangle());
-    } else if (newNode instanceof CommentBarButton) {
-      const commentView = newNode.getCommentView();
-      const xy = commentView.getRelativeToSurfaceXY();
-      const size = commentView.getSize();
-      const bounds = new Rect(
-        xy.y,
-        xy.y + size.height,
-        xy.x,
-        xy.x + size.width,
-      );
-      commentView.workspace.scrollBoundsIntoView(bounds);
-    }
   }
 
   /**

core/rendered_connection.ts

@@ -644,6 +644,9 @@ export class RenderedConnection
   /** See IFocusableNode.onNodeFocus. */
   onNodeFocus(): void {
     this.highlight();
+    this.getSourceBlock().workspace.scrollBoundsIntoView(
+      this.getSourceBlock().getBoundingRectangleWithoutChildren(),
+    );
   }
 
   /** See IFocusableNode.onNodeBlur. */
@@ -656,12 +659,12 @@ export class RenderedConnection
     return true;
   }
 
-  private findHighlightSvg(): SVGElement | null {
+  private findHighlightSvg(): SVGPathElement | null {
     // This cast is valid as TypeScript's definition is wrong. See:
     // https://github.com/microsoft/TypeScript/issues/60996.
     return document.getElementById(this.id) as
       | unknown
-      | null as SVGElement | null;
+      | null as SVGPathElement | null;
   }
 }
 

tests/browser/test/basic_playground_test.mjs

@@ -238,3 +238,224 @@ suite('Disabling', function () {
     },
   );
 });
+
+suite('Focused nodes are scrolled into bounds', function () {
+  // Setting timeout to unlimited as the webdriver takes a longer time to run
+  // than most mocha tests.
+  this.timeout(0);
+
+  // Setup Selenium for all of the tests
+  suiteSetup(async function () {
+    this.browser = await testSetup(testFileLocations.PLAYGROUND);
+    await this.browser.execute(() => {
+      window.focusScrollTest = async (testcase) => {
+        const workspace = Blockly.getMainWorkspace();
+        const metrics = workspace.getMetricsManager();
+        const initialViewport = metrics.getViewMetrics(true);
+        const elementBounds = await testcase(workspace);
+        await Blockly.renderManagement.finishQueuedRenders();
+        const scrolledViewport = metrics.getViewMetrics(true);
+        const workspaceBounds = new Blockly.utils.Rect(
+          scrolledViewport.top,
+          scrolledViewport.top + scrolledViewport.height,
+          scrolledViewport.left,
+          scrolledViewport.left + scrolledViewport.width,
+        );
+        return {
+          changed:
+            JSON.stringify(initialViewport) !==
+            JSON.stringify(scrolledViewport),
+          intersects: elementBounds.intersects(workspaceBounds),
+          contains: workspaceBounds.contains(
+            elementBounds.getOrigin().x,
+            elementBounds.getOrigin().y,
+          ),
+          elementBounds,
+          workspaceBounds,
+        };
+      };
+    });
+  });
+
+  setup(async function () {
+    await this.browser.execute(() => {
+      Blockly.serialization.blocks.append(
+        {
+          'type': 'text',
+          'x': -500,
+          'y': -500,
+        },
+        Blockly.getMainWorkspace(),
+      );
+      Blockly.serialization.blocks.append(
+        {
+          'type': 'controls_if',
+          'x': 500,
+          'y': 500,
+        },
+        Blockly.getMainWorkspace(),
+      );
+      Blockly.getMainWorkspace().zoomCenter(1);
+    });
+  });
+
+  test('Focused blocks scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const block = workspace.getTopBlocks()[0];
+        Blockly.getFocusManager().focusNode(block);
+        return block.getBoundingRectangleWithoutChildren();
+      });
+    });
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Focused bubbles scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const block = workspace.getTopBlocks()[0];
+        block.setCommentText('hello world');
+        const icon = block.getIcon(Blockly.icons.IconType.COMMENT);
+        icon.setBubbleVisible(true);
+        await Blockly.renderManagement.finishQueuedRenders();
+        icon.setBubbleLocation(new Blockly.utils.Coordinate(-510, -510));
+        Blockly.getFocusManager().focusNode(icon.getBubble());
+        const xy = icon.getBubble().getRelativeToSurfaceXY();
+        const size = icon.getBubble().getSize();
+        return new Blockly.utils.Rect(
+          xy.y,
+          xy.y + size.height,
+          xy.x,
+          xy.x + size.width,
+        );
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Comment bar buttons scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const comment = new Blockly.comments.RenderedWorkspaceComment(
+          workspace,
+        );
+        comment.moveTo(new Blockly.utils.Coordinate(-300, 500));
+        const commentBarButton = comment.view.getCommentBarButtons()[0];
+        Blockly.getFocusManager().focusNode(commentBarButton);
+        const xy = comment.view.getRelativeToSurfaceXY();
+        const size = comment.view.getSize();
+        // Comment bar buttons scroll their parent comment view into view.
+        return new Blockly.utils.Rect(
+          xy.y,
+          xy.y + size.height,
+          xy.x,
+          xy.x + size.width,
+        );
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Comment editors scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const comment = new Blockly.comments.RenderedWorkspaceComment(
+          workspace,
+        );
+        comment.moveTo(new Blockly.utils.Coordinate(-300, 500));
+        const commentEditor = comment.view.getEditorFocusableNode();
+        Blockly.getFocusManager().focusNode(commentEditor);
+        // Comment editor bounds can't be calculated externally since they
+        // depend on private properties, but the comment view is a reasonable
+        // proxy.
+        const xy = comment.view.getRelativeToSurfaceXY();
+        const size = comment.view.getSize();
+        return new Blockly.utils.Rect(
+          xy.y,
+          xy.y + size.height,
+          xy.x,
+          xy.x + size.width,
+        );
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Workspace comments scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const comment = new Blockly.comments.RenderedWorkspaceComment(
+          workspace,
+        );
+        comment.moveTo(new Blockly.utils.Coordinate(-500, 500));
+        Blockly.getFocusManager().focusNode(comment);
+        return comment.getBoundingRectangle();
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Icons scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const block = workspace.getTopBlocks()[0];
+        block.setWarningText('this is bad');
+        const icon = block.getIcon(Blockly.icons.IconType.WARNING);
+        Blockly.getFocusManager().focusNode(icon);
+        // Icon bounds can't be calculated externally since they depend on
+        // protected properties, but the parent block is a reasonable proxy.
+        return block.getBoundingRectangleWithoutChildren();
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Fields scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const block = workspace.getTopBlocks()[0];
+        const field = block.getField('TEXT');
+        Blockly.getFocusManager().focusNode(field);
+        // Fields scroll their source block into view.
+        return block.getBoundingRectangleWithoutChildren();
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+
+  test('Connections scroll into bounds', async function () {
+    const result = await this.browser.execute(async () => {
+      return await window.focusScrollTest(async (workspace) => {
+        const block = workspace.getBlocksByType('controls_if')[0];
+        Blockly.getFocusManager().focusNode(block.nextConnection);
+        // Connection bounds can't be calculated externally since they depend on
+        // protected properties, but the parent block is a reasonable proxy.
+        return block.getBoundingRectangleWithoutChildren();
+      });
+    });
+
+    chai.assert.isTrue(result.intersects);
+    chai.assert.isTrue(result.contains);
+    chai.assert.isTrue(result.changed);
+  });
+});