google / blockly

The web-based visual programming editor.
https://developers.google.com/blockly/
Apache License 2.0
12.52k stars 3.72k forks source link

fix: Detect if deleting shadow block affects selection highlight #8533

Closed johnnesky closed 3 months ago

johnnesky commented 3 months ago

The basics

The details

Resolves

Partially addresses: https://github.com/google/blockly-samples/issues/2426

Proposed Changes

Detects whether deleting a block would delete the currently selected block, and if so, deselects it before severing connections.

Reason for Changes

When a shadow block is selected, an ancestor of that block receives a visual highlight. If the shadow block is then deleted (e.g. because it was replaced with a real block via the shadow block converter plugin), then the highlighted block no longer has any relationship to any selected block. That highlight should go away, but currently it persists and may continue to persist while making further changes to the selection.

Note that Block.dispose() severs a connection, and Block.disposeInternal() also severs connections, so we need to determine whether any parent block needs to be updated before this happens. The subclass BlockSvg overrides these methods, so I chose to perform the detection and resolution in BlockSvg.dispose().

Test Coverage

Existing tests pass. I added new tests for the new features.

Documentation

N/A

Additional Information

I think it feels unintuitive that, when using the shadow block converter plugin and the user edits a shadow block, the current selection gets cleared. As far as the user is concerned, nothing appears to have been deleted (since the real clone of the shadow block seamlessly replaces the shadow block) so it's weird that editing a field of a shadow block deselects the selection.

I propose that we fix this in the shadow block converter plugin by transferring the selection to the new real clone of the selected shadow block after replacing it. Since that's in the samples repo, it would have to be a separate PR. In the meantime, the shadow block converter plugin is already broken and I don't think this PR makes it worse.