feat(canvas): Delete key removes the selected node (#2336) - #30
Merged
Conversation
Selecting a node set _selectedId but nothing listened for a keypress, so Delete/Backspace did nothing — the canvas had no keyboard handler at all. Wire one: a Focus wrapper (autofocus, re-requested on tap so a keypress right after selecting always lands here) whose onKeyEvent, on Delete/Backspace with a selection, tombstones that one node. New single-box delete path mirroring the Clear middleware, scoped to one id: DeleteClassBoxAction(id) → DeleteClassBoxMiddleware tombstones just that box via classBoxTombstone + backend.updateGraphNode (same converging read-merge-write as any edit; a peer re-adding with a higher HLC wins). The tombstone carries the box's RETAINED type per the #2432 writer census, not a hardcoded ClassBox. Interim step toward replacing the blunt Clear-all with proper multi-select delete (#2336). Full marquee-select + move + "+ Box" button + Clear removal lands in the follow-up interaction-model PR. 196 tests (3 new: targeted-only tombstone, retained-type sourcing, missing-id no-op). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Visit the preview URL for this PR (updated for commit c8529d5): https://domain-visualiser-app--pr30-feat-delete-selected-9aspkwtx.web.app (expires Tue, 18 Aug 2026 10:00:38 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 0becdc05fe3c2187f0b584b6fed0b0a8209712f6 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
You reported: "double click to select works but then Delete doesn't do anything." Root cause — the canvas had no keyboard handler at all. Tapping a node set
_selectedId, but nothing listened for a keypress, so Delete/Backspace fell on the floor.The fix
Focuswrapper (autofocus, re-requested on tap so a keypress right after selecting always lands here) whoseonKeyEventdeletes the selected node on Delete/Backspace.DeleteClassBoxAction(id)→DeleteClassBoxMiddlewaretombstones just that box viaclassBoxTombstone+backend.updateGraphNode— the same converging read-merge-write as any edit (a peer re-adding with a higher HLC wins), carrying the box's retained type (not a hardcoded ClassBox).Scope note
This is the interim step of #2336. The full interaction model you picked — empty-drag = marquee-select, "+ Box" button, drag-a-box = move, multi-delete, and removing the Clear button — lands in a follow-up PR. This one just makes select-then-Delete work now.
Caveat: deleting a projected community node (Person/Repo) tombstones it, but the producer re-adds it on its next run — so single-delete is most meaningful for human-drawn boxes. The follow-up can refine per-node-type behaviour.
Tests
196 pass (3 new: targeted-only tombstone, retained-type sourcing, missing-id no-op).
🤖 Generated with Claude Code