Repository navigation
Edit Points: Esc before releasing the mouse button cancels a point or handle drag - #1325
Conversation
… handle drag Esc during a vertex or handle drag in Edit Points used to leave Edit Points while the drag went on: the point kept following the pointer and the release added an undo step. Esc now ends the drag and puts the shape back as it was at the press, with no undo step, and Edit Points stays on. Esc without a drag still leaves Edit Points.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Restore the saved drag state crates/engine/src/tools.rs |
cancel_point_drag clears an active vertex or handle drag. When the saved document differs from the current document, it restores the saved document and selection and increments the revision. It does not add an undo step. |
Handle Escape and test cancellation crates/engine/src/keys.rs, crates/engine/src/tests.rs, ROADMAP.md |
Session::key checks Escape or Esc for an active point drag before other key handlers. A regression test checks restored geometry, no undo step, ignored later drag events, and Edit Points mode behavior. The progress log records the change. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related PRs
- storytold/deckcraft#1260: This PR also changes Escape handling in Edit Points.
Suggested reviewers: marcoarnulfo
Merge Risk: 
8f311
-
Escape cancellation preserves the intended editing flow, with no identified issue requiring a fix before merge.
Security architecture risk: 
8f311
-
The change does not introduce new editing permissions or external access. The main design risk is that cancelling a drag can also discard other edits accepted during that drag, rather than only the point movement.
Retained concerns
- Medium · reliability · inferred: Cancellation restores the complete pre-drag presentation, but successful Session edits are not isolated from the open interaction. An intervening edit can therefore be discarded together with the drag, without a separate undo entry. The base already grouped such edits into the interaction; the new cancellation terminal state adds a path that discards that group. The normal single-document regression test supports the intended gesture behavior but does not resolve this interleaving case.
Security review details
-
Security Blast Radius
- inferred — The introduced rollback outcome is bounded to the active presentation and selection in an existing editing Session. Intervening edits elsewhere in that presentation can be affected because the snapshot covers the whole document.
Trust Boundaries and Controls
- observed — Existing UI, control-channel, and MCP callers already forward key input to Session::key. Those routes are unchanged by this PR; the added Escape branch does not grant an additional caller identity or editing capability.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: pressing Esc during an Edit Points vertex or handle drag cancels the drag before mouse release. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Usage-based review receipt
- Mode: Continue automatically
- Reviewed files: 4
- Waived: $1.00 (charged $0.00)
- View usage details
Note
This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.
Comment @coderabbitai help to get the list of available commands.
In Edit Points, pressing Esc while dragging a point or one of its handles left Edit Points, but the drag went on: the point kept following the pointer, and the release added an "Edit Points" undo step. Microsoft's support article says: "To cancel the change to the point and line segments, press Esc before releasing the mouse button." (Draw or edit a freeform shape)
What changes:
Session::cancel_point_dragincrates/engine/src/tools.rsends a vertex or handle drag and puts back the document and selection from the press. It adds no undo step.crates/engine/src/keys.rs, Esc calls it first, and the rest of the drag and the release change nothing. The support article does not say whether Edit Points stays on; in DeckCraft it stays on after a cancelled drag, and Esc without a drag still leaves Edit Points.ROADMAP.mdprogress log has a line for it.Test:
tests::edit_points_escape_cancels_a_point_or_handle_dragdrags a point of an ellipse, presses Esc, drags further and releases, then drags one of the point's handles, presses Esc and releases. After each, it checks that the shape's geometry matches the geometry before the press, that the undo stack has the same length, and that Edit Points is still on. It fails onmainat the first check after Esc ("Esc puts the point back"). With the line that ends the drag removed, it fails at the check that no drag is in progress.Checks:
cargo test -p deckcraft-engine --lib edit_points(10 tests),cargo fmt --all -- --check,cargo clippy -p deckcraft-engine --all-targets -- -D warnings,cargo xtask assetsandcargo xtask layers.This branch and #1308 (handle drag modifiers) change different lines of
tools.rs,tests.rsandROADMAP.md.git merge-treemerges the two without conflicts, and theedit_pointstests pass on the merged tree (11 tests).On Windows 11 (x86-64) with rustc 1.99.1, on top of
mainat38d2ade, with #1292, #1298 and #1300 merged in:cargo xtask cipasses its 8 steps: 1,881 tests passed, 0 failed, 6 ignored.Without #1298,
control_server::tests::blocked_ui_times_out_and_full_queue_is_busyfails intermittently onmainat38d2adetoo (3 of 40 runs of thecontrol_servertests); #1298 fixes it.git merge-treereports no conflicts withmainat38d2ade.Summary by CodeRabbit