fix: Use a readonly textarea for non-editable comments. (#8632)

* fix: Use a readonly textarea for non-editable comments. * chore: Run formatter. * chore: remove old function definition
google · Nov 4, 2024 · aedcfd6 · aedcfd6
1 parent d053008
commit aedcfd6
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 25 deletions.
diff --git a/core/bubbles/textinput_bubble.ts b/core/bubbles/textinput_bubble.ts
@@ -63,6 +63,8 @@ export class TextInputBubble extends Bubble {
     20 + Bubble.DOUBLE_BORDER,
   );
 
+  private editable = true;
+
   /**
    * @param workspace The workspace this bubble belongs to.
    * @param anchor The anchor location of the thing this bubble is attached to.
@@ -96,6 +98,21 @@ export class TextInputBubble extends Bubble {
     this.onTextChange();
   }
 
+  /** Sets whether or not the text in the bubble is editable. */
+  setEditable(editable: boolean) {
+    this.editable = editable;
+    if (this.editable) {
+      this.textArea.removeAttribute('readonly');
+    } else {
+      this.textArea.setAttribute('readonly', '');
+    }
+  }
+
+  /** Returns whether or not the text in the bubble is editable. */
+  isEditable(): boolean {
+    return this.editable;
+  }
+
   /** Adds a change listener to be notified when this bubble's text changes. */
   addTextChangeListener(listener: () => void) {
     this.textChangeListeners.push(listener);

diff --git a/core/icons/comment_icon.ts b/core/icons/comment_icon.ts
@@ -8,7 +8,6 @@
 
 import type {Block} from '../block.js';
 import type {BlockSvg} from '../block_svg.js';
-import {TextBubble} from '../bubbles/text_bubble.js';
 import {TextInputBubble} from '../bubbles/textinput_bubble.js';
 import {EventType} from '../events/type.js';
 import * as eventUtils from '../events/utils.js';
@@ -47,12 +46,9 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
    */
   static readonly WEIGHT = 3;
 
-  /** The bubble used to show editable text to the user. */
+  /** The bubble used to show comment text to the user. */
   private textInputBubble: TextInputBubble | null = null;
 
-  /** The bubble used to show non-editable text to the user. */
-  private textBubble: TextBubble | null = null;
-
   /** The text of this comment. */
   private text = '';
 
@@ -118,7 +114,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
   override dispose() {
     super.dispose();
     this.textInputBubble?.dispose();
-    this.textBubble?.dispose();
   }
 
   override getWeight(): number {
@@ -133,7 +128,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     super.applyColour();
     const colour = (this.sourceBlock as BlockSvg).style.colourPrimary;
     this.textInputBubble?.setColour(colour);
-    this.textBubble?.setColour(colour);
   }
 
   /**
@@ -153,7 +147,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     super.onLocationChange(blockOrigin);
     const anchorLocation = this.getAnchorLocation();
     this.textInputBubble?.setAnchorLocation(anchorLocation);
-    this.textBubble?.setAnchorLocation(anchorLocation);
   }
 
   /** Sets the text of this comment. Updates any bubbles if they are visible. */
@@ -170,7 +163,6 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
     );
     this.text = text;
     this.textInputBubble?.setText(this.text);
-    this.textBubble?.setText(this.text);
   }
 
   /** Returns the text of this comment. */
@@ -302,33 +294,31 @@ export class CommentIcon extends Icon implements IHasBubble, ISerializable {
    * to update the state of this icon in response to changes in the bubble.
    */
   private showEditableBubble() {
-    this.textInputBubble = new TextInputBubble(
-      this.sourceBlock.workspace as WorkspaceSvg,
-      this.getAnchorLocation(),
-      this.getBubbleOwnerRect(),
-    );
-    this.textInputBubble.setText(this.getText());
-    this.textInputBubble.setSize(this.bubbleSize, true);
-    this.textInputBubble.addTextChangeListener(() => this.onTextChange());
-    this.textInputBubble.addSizeChangeListener(() => this.onSizeChange());
+    this.createBubble();
+    this.textInputBubble?.addTextChangeListener(() => this.onTextChange());
+    this.textInputBubble?.addSizeChangeListener(() => this.onSizeChange());
   }
 
   /** Shows the non editable text bubble for this comment. */
   private showNonEditableBubble() {
-    this.textBubble = new TextBubble(
-      this.getText(),
+    this.createBubble();
+    this.textInputBubble?.setEditable(false);
+  }
+
+  protected createBubble() {
+    this.textInputBubble = new TextInputBubble(
       this.sourceBlock.workspace as WorkspaceSvg,
       this.getAnchorLocation(),
       this.getBubbleOwnerRect(),
     );
+    this.textInputBubble.setText(this.getText());
+    this.textInputBubble.setSize(this.bubbleSize, true);
   }
 
   /** Hides any open bubbles owned by this comment. */
   private hideBubble() {
     this.textInputBubble?.dispose();
     this.textInputBubble = null;
-    this.textBubble?.dispose();
-    this.textBubble = null;
   }
 
   /**

diff --git a/tests/mocha/comment_test.js b/tests/mocha/comment_test.js
@@ -41,12 +41,12 @@ suite('Comments', function () {
     });
 
     function assertEditable(comment) {
-      assert.isNotOk(comment.textBubble);
       assert.isOk(comment.textInputBubble);
+      assert.isTrue(comment.textInputBubble.isEditable());
     }
     function assertNotEditable(comment) {
-      assert.isNotOk(comment.textInputBubble);
-      assert.isOk(comment.textBubble);
+      assert.isOk(comment.textInputBubble);
+      assert.isFalse(comment.textInputBubble.isEditable());
     }
     test('Editable', async function () {
       await this.comment.setBubbleVisible(true);