fix: remove specific warning text (#7376)

* fix: remove specific warning text * chore: teardown rendered workspace after test * chore: reformat new tests
google · Aug 11, 2023 · 8b6c780 · 8b6c780
1 parent a8fca2d
commit 8b6c780
Show file tree

Hide file tree

Showing 2 changed files with 99 additions and 4 deletions.
diff --git a/core/block_svg.ts b/core/block_svg.ts
@@ -896,11 +896,10 @@ export class BlockSvg
    * Set this block's warning text.
    *
    * @param text The text, or null to delete.
-   * @param opt_id An optional ID for the warning text to be able to maintain
+   * @param id An optional ID for the warning text to be able to maintain
    *     multiple warnings.
    */
-  override setWarningText(text: string | null, opt_id?: string) {
-    const id = opt_id || '';
+  override setWarningText(text: string | null, id: string = '') {
     if (!id) {
       // Kill all previous pending processes, this edit supersedes them all.
       for (const timeout of this.warningTextDb.values()) {
@@ -931,8 +930,9 @@ export class BlockSvg
     }
 
     const icon = this.getIcon(WarningIcon.TYPE) as WarningIcon | undefined;
-    if (typeof text === 'string') {
+    if (text) {
       // Bubble up to add a warning on top-most collapsed block.
+      // TODO(#6020): This warning is never removed.
       let parent = this.getSurroundParent();
       let collapsedParent = null;
       while (parent) {
@@ -958,6 +958,9 @@ export class BlockSvg
       if (!id) {
         this.removeIcon(WarningIcon.TYPE);
       } else {
+        // Remove just this warning id's message.
+        icon.addMessage('', id);
+        // Then remove the entire icon if there is no longer any text.
         if (!icon.getText()) this.removeIcon(WarningIcon.TYPE);
       }
     }

diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js
@@ -1612,6 +1612,98 @@ suite('Blocks', function () {
       });
     });
 
+    suite('Warning icons', function () {
+      setup(function () {
+        this.workspace = Blockly.inject('blocklyDiv');
+
+        this.block = this.workspace.newBlock('stack_block');
+        this.block.initSvg();
+        this.block.render();
+      });
+
+      teardown(function () {
+        workspaceTeardown.call(this, this.workspace);
+      });
+
+      test('Block with no warning text does not have warning icon', function () {
+        const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
+
+        chai.assert.isUndefined(
+          icon,
+          'Block with no warning should not have warning icon',
+        );
+      });
+
+      test('Set warning text creates new icon if none existed', function () {
+        const text = 'Warning Text';
+
+        this.block.setWarningText(text);
+
+        const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
+        chai.assert.equal(
+          icon.getText(),
+          text,
+          'Expected warning icon text to be set',
+        );
+      });
+
+      test('Set warning text adds text to existing icon if needed', function () {
+        const text1 = 'Warning Text 1';
+        const text2 = 'Warning Text 2';
+
+        this.block.setWarningText(text1, '1');
+        this.block.setWarningText(text2, '2');
+
+        const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
+        chai.assert.equal(icon.getText(), `${text1}\n${text2}`);
+      });
+
+      test('Clearing all warning text deletes the warning icon', function () {
+        const text = 'Warning Text';
+        this.block.setWarningText(text);
+
+        this.block.setWarningText(null);
+
+        const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
+        chai.assert.isUndefined(
+          icon,
+          'Expected warning icon to be undefined after deleting all warning text',
+        );
+      });
+
+      test('Clearing specific warning does not delete the icon if other warnings present', function () {
+        const text1 = 'Warning Text 1';
+        const text2 = 'Warning Text 2';
+
+        this.block.setWarningText(text1, '1');
+        this.block.setWarningText(text2, '2');
+        this.block.setWarningText(null, '1');
+
+        const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
+        chai.assert.equal(
+          icon.getText(),
+          text2,
+          'Expected first warning text to be deleted',
+        );
+      });
+
+      test('Clearing specific warning removes icon if it was only warning present', function () {
+        const text1 = 'Warning Text 1';
+        const text2 = 'Warning Text 2';
+
+        this.block.setWarningText(text1, '1');
+        this.block.setWarningText(text2, '2');
+        this.block.setWarningText(null, '1');
+        this.block.setWarningText(null, '2');
+
+        const icon = this.block.getIcon(Blockly.icons.WarningIcon.TYPE);
+        chai.assert.isUndefined(
+          icon,
+          'Expected warning icon to be deleted after all warning text is cleared',
+        );
+      });
+    });
+
     suite('Bubbles and collapsing', function () {
       setup(function () {
         this.workspace = Blockly.inject('blocklyDiv');