Skip to content

Artboard tool: Delete and Backspace delete the active artboard - #650

Merged
bflatastic merged 1 commit into
storytold:mainfrom
mateususa:fix/artboard-tool-delete-key
Oct 9, 2026
Merged

bflatastic merged 1 commit into
storytold:mainfrom
mateususa:fix/artboard-tool-delete-key

Conversation

@mateususa

Copy link
Copy Markdown
Contributor

What and why

With the Artboard tool (Shift+O), clicking an artboard and pressing Delete or Backspace does nothing. The artboard can only be removed with the trash button in the Artboards panel.

Cause: ArtboardTool::key handles Delete/Backspace, but the tool never overrides Tool::claims_key. ui-egui/src/shortcuts.rs only hands Delete/Backspace to a tool that claims them, so the keypress goes to the Clear shortcut instead. Clear deletes selected objects and leaves the artboard alone. Tools such as Gradient, Puppet Warp and Slice already claim their keys this way; the Artboard tool was missing it. The existing tests call ArtboardTool::key / Session::tool_key directly, which bypasses the routing, so they didn't catch it.

Fix: ArtboardTool::claims_key claims Delete/Backspace while no drag is in progress and the document has more than one artboard. key() checks the same condition, so the two can't drift apart. With a single artboard (which can't be deleted), the keys stay with the shortcut, as before.

How it was tested

  • crates/engine/src/tests_xform.rs (artboard_tool_moves_with_art_creates_and_deletes) now asserts tool_claims_key(Delete) before deleting, and asserts that the last artboard doesn't claim it. This test fails on main (assertion failed: s.tool_claims_key(ToolKey::Delete, …)) and passes with this change.
  • crates/tools/src/xform/artboard.rs (move_resize_create_delete) asserts that the tool claims both Delete and Backspace.
  • cargo test -p vectorcraft-tools artboard (12 passed) and cargo test -p vectorcraft-engine artboard_tool (4 passed).
  • cargo fmt --check and cargo clippy -p vectorcraft-tools --all-targets -D warnings are clean.
  • Found while using a local Windows build: the panel's trash button worked, the Delete key didn't.

Checklist

  • cargo xtask ci passes. I ran the touched crates' tests, fmt and clippy, but not the full xtask ci.
  • No panics in shipped code: no unwrap(), expect() or similar outside tests, and no unchecked indexing.
  • Clean-room: nothing copied from Adobe products or GPL/AGPL code.
  • No new assets.

🤖 Generated with Claude Code

The Artboard tool handled Delete/Backspace in Tool::key, but never claimed
the keys through Tool::claims_key. The UI only routes Delete/Backspace to a
tool that claims them, so the keypress went to the Clear shortcut instead
and the artboard stayed. The existing tests called the tool directly and
missed it.

The tool now claims Delete/Backspace while no drag is in progress and more
than one artboard exists. With only one artboard left, the keys stay with
the shortcut.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bflatastic
bflatastic merged commit 11fceda into storytold:main Oct 9, 2026
@bflatastic

Copy link
Copy Markdown
Member

Merged. Thank you, @mateususa! 🎉

This is a great first PR. You found the real cause (the Artboard tool handled Delete but never claimed it, so the shortcut router gave the key to Clear) instead of patching around it. Having key() reuse claims_key() means the two can't drift apart. Pointing out that the existing tests bypassed the routing, and adding the assertion that fails on main, made it quick to review.

I checked it through the real keyboard path in the running app: with the Artboard tool, clicking the second artboard and pressing Delete removes it, and with one artboard left, Delete and Backspace leave it alone. Full cargo xtask ci passed on top of current main.

If you'd like to chat with us and other contributors, come join https://discord.gg/artcraft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants