From 095f29e2742ace7fb87fa1ed814d4fe86dce4358 Mon Sep 17 00:00:00 2001
From: John Nesky <nesky@google.com>
Date: Mon, 26 Aug 2024 12:22:37 -0700
Subject: [PATCH] fix: Detect if deleting shadow block affects selection
 highlight (#8533)

* fix: Detect if deleting shadow block affects selection highlight

* Use contains instead of descendsFrom for html consistency.

* Added tests.

* Removing and manually inlining new contains function.
---
 core/block_svg.ts         | 21 ++++++++++++++++++++-
 tests/mocha/block_test.js | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/core/block_svg.ts b/core/block_svg.ts
index db75acde2..9797a50e7 100644
--- a/core/block_svg.ts
+++ b/core/block_svg.ts
@@ -279,7 +279,7 @@ export class BlockSvg
     this.addSelect();
   }
 
-  /** Unselects this block. Unhighlights the blockv visually.   */
+  /** Unselects this block. Unhighlights the block visually. */
   unselect() {
     if (this.isShadow()) {
       this.getParent()?.unselect();
@@ -797,6 +797,25 @@ export class BlockSvg
       blockAnimations.disposeUiEffect(this);
     }
 
+    // Selecting a shadow block highlights an ancestor block, but that highlight
+    // should be removed if the shadow block will be deleted. So, before
+    // deleting blocks and severing the connections between them, check whether
+    // doing so would delete a selected block and make sure that any associated
+    // parent is updated.
+    const selection = common.getSelected();
+    if (selection instanceof Block) {
+      let selectionAncestor: Block | null = selection;
+      while (selectionAncestor !== null) {
+        if (selectionAncestor === this) {
+          // The block to be deleted contains the selected block, so remove any
+          // selection highlight associated with the selected block before
+          // deleting them.
+          selection.unselect();
+        }
+        selectionAncestor = selectionAncestor.getParent();
+      }
+    }
+
     super.dispose(!!healStack);
     dom.removeNode(this.svgGroup_);
   }
diff --git a/tests/mocha/block_test.js b/tests/mocha/block_test.js
index 061584401..3f0eba44b 100644
--- a/tests/mocha/block_test.js
+++ b/tests/mocha/block_test.js
@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import * as common from '../../build/src/core/common.js';
 import {ConnectionType} from '../../build/src/core/connection_type.js';
 import {EventType} from '../../build/src/core/events/type.js';
 import * as eventUtils from '../../build/src/core/events/utils.js';
@@ -444,6 +445,39 @@ suite('Blocks', function () {
         });
       });
     });
+
+    suite('Disposing selected shadow block', function () {
+      setup(function () {
+        this.workspace = Blockly.inject('blocklyDiv');
+        this.parentBlock = this.workspace.newBlock('row_block');
+        this.parentBlock.initSvg();
+        this.parentBlock.render();
+        this.parentBlock.inputList[0].connection.setShadowState({
+          'type': 'row_block',
+          'id': 'shadow_child',
+        });
+        this.shadowChild =
+          this.parentBlock.inputList[0].connection.targetConnection.getSourceBlock();
+      });
+
+      teardown(function () {
+        workspaceTeardown.call(this, this.workspace);
+      });
+
+      test('Disposing selected shadow unhighlights parent', function () {
+        const parentBlock = this.parentBlock;
+        common.setSelected(this.shadowChild);
+        assert.isTrue(
+          parentBlock.pathObject.svgRoot.classList.contains('blocklySelected'),
+          'Expected parent to be highlighted after selecting shadow child',
+        );
+        this.shadowChild.dispose();
+        assert.isFalse(
+          parentBlock.pathObject.svgRoot.classList.contains('blocklySelected'),
+          'Expected parent to be unhighlighted after deleting shadow child',
+        );
+      });
+    });
   });
 
   suite('Remove Input', function () {