refactor: Associate comment bar buttons with the comment view. (#9278)

Author: gonfunko

core/comments/collapse_comment_bar_button.ts

@@ -10,6 +10,7 @@ import * as dom from '../utils/dom.js';
 import {Svg} from '../utils/svg.js';
 import type {WorkspaceSvg} from '../workspace_svg.js';
 import {CommentBarButton} from './comment_bar_button.js';
+import type {CommentView} from './comment_view.js';
 
 /**
  * Magic string appended to the comment ID to create a unique ID for this button.
@@ -42,8 +43,9 @@ export class CollapseCommentBarButton extends CommentBarButton {
     protected readonly id: string,
     protected readonly workspace: WorkspaceSvg,
     protected readonly container: SVGGElement,
+    protected readonly commentView: CommentView,
   ) {
-    super(id, workspace, container);
+    super(id, workspace, container, commentView);
 
     this.icon = dom.createSvgElement(
       Svg.IMAGE,
@@ -86,14 +88,13 @@ export class CollapseCommentBarButton extends CommentBarButton {
   override performAction(e?: Event) {
     touch.clearTouchIdentifier();
 
-    const comment = this.getParentComment();
-    comment.view.bringToFront();
+    this.getCommentView().bringToFront();
     if (e && e instanceof PointerEvent && browserEvents.isRightButton(e)) {
       e.stopPropagation();
       return;
     }
 
-    comment.setCollapsed(!comment.isCollapsed());
+    this.getCommentView().setCollapsed(!this.getCommentView().isCollapsed());
     this.workspace.hideChaff();
 
     e?.stopPropagation();

core/comments/comment_bar_button.ts

@@ -7,7 +7,7 @@
 import type {IFocusableNode} from '../interfaces/i_focusable_node.js';
 import {Rect} from '../utils/rect.js';
 import type {WorkspaceSvg} from '../workspace_svg.js';
-import type {RenderedWorkspaceComment} from './rendered_workspace_comment.js';
+import type {CommentView} from './comment_view.js';
 
 /**
  * Button displayed on a comment's top bar.
@@ -29,6 +29,7 @@ export abstract class CommentBarButton implements IFocusableNode {
     protected readonly id: string,
     protected readonly workspace: WorkspaceSvg,
     protected readonly container: SVGGElement,
+    protected readonly commentView: CommentView,
   ) {}
 
   /**
@@ -39,17 +40,10 @@ export abstract class CommentBarButton implements IFocusableNode {
   }
 
   /**
-   * Returns the parent comment of this comment bar button.
+   * Returns the parent comment view of this comment bar button.
    */
-  getParentComment(): RenderedWorkspaceComment {
-    const comment = this.workspace.getCommentById(this.id);
-    if (!comment) {
-      throw new Error(
-        `Comment bar button ${this.id} has no corresponding comment`,
-      );
-    }
-
-    return comment;
+  getCommentView(): CommentView {
+    return this.commentView;
   }
 
   /** Adjusts the position of this button within its parent container. */

core/comments/comment_view.ts

@@ -102,7 +102,7 @@ export class CommentView implements IRenderedElement {
 
   constructor(
     readonly workspace: WorkspaceSvg,
-    private commentId: string,
+    readonly commentId: string,
   ) {
     this.svgRoot = dom.createSvgElement(Svg.G, {
       'class': 'blocklyComment blocklyEditable blocklyDraggable',
@@ -176,12 +176,18 @@ export class CommentView implements IRenderedElement {
       this.commentId,
       this.workspace,
       topBarGroup,
+      this,
     );
     const foldoutButton = new CollapseCommentBarButton(
       this.commentId,
       this.workspace,
       topBarGroup,
+      this,
     );
+    this.addDisposeListener(() => {
+      deleteButton.dispose();
+      foldoutButton.dispose();
+    });
     const textPreview = dom.createSvgElement(
       Svg.TEXT,
       {
@@ -612,13 +618,12 @@ export class CommentView implements IRenderedElement {
   /** Disposes of this comment view. */
   dispose() {
     this.disposing = true;
-    this.foldoutButton.dispose();
-    this.deleteButton.dispose();
     dom.removeNode(this.svgRoot);
     // Loop through listeners backwards in case they remove themselves.
     for (let i = this.disposeListeners.length - 1; i >= 0; i--) {
       this.disposeListeners[i]();
     }
+    this.disposeListeners.length = 0;
     this.disposed = true;
   }
 

core/comments/delete_comment_bar_button.ts

@@ -11,6 +11,7 @@ import * as dom from '../utils/dom.js';
 import {Svg} from '../utils/svg.js';
 import type {WorkspaceSvg} from '../workspace_svg.js';
 import {CommentBarButton} from './comment_bar_button.js';
+import type {CommentView} from './comment_view.js';
 
 /**
  * Magic string appended to the comment ID to create a unique ID for this button.
@@ -42,8 +43,9 @@ export class DeleteCommentBarButton extends CommentBarButton {
     protected readonly id: string,
     protected readonly workspace: WorkspaceSvg,
     protected readonly container: SVGGElement,
+    protected readonly commentView: CommentView,
   ) {
-    super(id, workspace, container);
+    super(id, workspace, container, commentView);
 
     this.icon = dom.createSvgElement(
       Svg.IMAGE,
@@ -97,7 +99,7 @@ export class DeleteCommentBarButton extends CommentBarButton {
       return;
     }
 
-    this.getParentComment().dispose();
+    this.getCommentView().dispose();
     e?.stopPropagation();
     getFocusManager().focusNode(this.workspace);
   }

core/keyboard_nav/comment_bar_button_navigation_policy.ts

@@ -31,7 +31,9 @@ export class CommentBarButtonNavigationPolicy
    * @returns The parent comment of the given CommentBarButton.
    */
   getParent(current: CommentBarButton): IFocusableNode | null {
-    return current.getParentComment();
+    return current
+      .getCommentView()
+      .workspace.getCommentById(current.getCommentView().commentId);
   }
 
   /**
@@ -41,7 +43,7 @@ export class CommentBarButtonNavigationPolicy
    * @returns The next CommentBarButton, if any.
    */
   getNextSibling(current: CommentBarButton): IFocusableNode | null {
-    const children = current.getParentComment().view.getCommentBarButtons();
+    const children = current.getCommentView().getCommentBarButtons();
     const currentIndex = children.indexOf(current);
     if (currentIndex >= 0 && currentIndex + 1 < children.length) {
       return children[currentIndex + 1];
@@ -56,7 +58,7 @@ export class CommentBarButtonNavigationPolicy
    * @returns The CommentBarButton's previous CommentBarButton, if any.
    */
   getPreviousSibling(current: CommentBarButton): IFocusableNode | null {
-    const children = current.getParentComment().view.getCommentBarButtons();
+    const children = current.getCommentView().getCommentBarButtons();
     const currentIndex = children.indexOf(current);
     if (currentIndex > 0) {
       return children[currentIndex - 1];

core/keyboard_nav/line_cursor.ts

@@ -20,6 +20,7 @@ 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 {Marker} from './marker.js';
 
@@ -405,8 +406,16 @@ export class LineCursor extends Marker {
     } else if (newNode instanceof RenderedWorkspaceComment) {
       newNode.workspace.scrollBoundsIntoView(newNode.getBoundingRectangle());
     } else if (newNode instanceof CommentBarButton) {
-      const comment = newNode.getParentComment();
-      comment.workspace.scrollBoundsIntoView(comment.getBoundingRectangle());
+      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);
     }
   }