fix: preserve stack below when deleting a top stack block

[Joeclinton1]

Resolves

• spork issue tracker scratch-editor#533 (reported in comments by @Joeclinton1 (aka me))

Deleting a hat block (or any top-of-stack block) via right-click → Delete Block also deletes every block connected below it. Keyboard Delete/Backspace has the same bug. Expected: only the top block is removed; the stack below stays.

Proposed Changes

• Extract a shared deleteBlock helper in context_menu_items.ts. For a top stack block (no previousConnection target), disconnect nextConnection before dispose so the block below is left as a new top block.
• Route BlockSvg.prototype.checkAndDelete through the same helper in index.ts so keyboard delete matches the context-menu path.

Reason for Changes

dispose(healStack=true) on a top block has nothing to heal to, so the next block gets disposed with it. Explicitly detaching fixes both paths through one helper.

Test Coverage

Two unit tests in tests/unit/context_menu_items.test.ts covering both delete paths. Both fail without the fix.

Manually verified in scratch-editor (right-click + keyboard) that deleting a hat leaves its stack intact, mid-stack delete still heals above↔below, reporter delete is unaffected, and undo restores in one step.

[Joeclinton1]

This was another GPT 5.5 fast test in codex. It took 12 minutes from prompt to pr. This time I tested it better to be sure it actually fixed it, as the previous with the shadow block duplication I messed up the repro, and thought it had fixed it when it hadn't.

[Copilot]

Pull request overview

This PR fixes a Scratch-specific deletion bug where deleting a top-of-stack block (via context menu or keyboard) incorrectly deletes the entire stack below it, by detaching the nextConnection before disposal and routing both delete paths through a shared helper.

Changes:

• Added a shared deleteBlock helper to centralize delete behavior and preserve the stack below when deleting a top stack block.
• Overrode Blockly.BlockSvg.prototype.checkAndDelete to use the same delete logic for keyboard Delete/Backspace.
• Added unit tests covering both the context-menu delete callback and the keyboard-delete wiring.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/context_menu_items.ts	Extracts deleteBlock helper and updates the context-menu delete callback to use it.
src/index.ts	Routes keyboard delete (checkAndDelete) through the new shared helper.
tests/unit/context_menu_items.test.ts	Adds unit tests ensuring deleting a top stack block preserves the next block for both delete paths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[cwillisf]

I like how careful deleteBlock is. Thanks for the help!

[cwillisf]

OK, the Copilot review has some good points...

[cwillisf]

(I recommend rebasing on or merging from develop to get the changes from #3574 here)

[Joeclinton1]

(I recommend rebasing on or merging from develop to get the changes from #3574 here)

Sure let me see what I can do, I will get a second version out soon.

[Joeclinton1]

@cwillisf I tested the proposed bugs by the reviewer and they don't exist. So I can do these changes but you can't reach the proposed bugs.

Also you can delete a custom block with delete blocks even though it has usages which is a new bug, and not caused by this change (i tested in production).

[Joeclinton1]

I did the changes related to block.nextConnection?.disconnect() but the rest just isn't valid feedback. I cannot actually preproduce the lone stack block not being deletable bug, but to be safe I chose to implement it.

non-deletable blocks can't test because, the only non deletable is the procedure definition and that's currently deletable in production which is an upstream bug. Shadow block inputs weren't deletable with keyboard, maybe because you can never focus them in such a way that would let that be possible

The undo group thing can't replicate so didn't fix.

[mxmou]

The undo group thing can't replicate so didn't fix.

It can cause issues if someone writes code like this in the future:

Blockly.Events.setGroup(true)
// do things that create events ...
deleteBlock(block)
Blockly.Events.setGroup(false)

The other events should be grouped together with the deletion, but they won't be because deleteBlock() creates a new group unconditionally.

[Joeclinton1]

I pushed changes to address the other two reviewer feedbacks. They are not accessible through the ui, but it is possible that by not guarding against them it leaves the possibility of a bug later on in the codebase development. So even though it seems a little overly defensive I've done those changes.