Skip to content

feat(edit): Esc in an open paragraph selects its box, so Delete removes it - #4620

Merged
skylartaylor merged 1 commit into
storytold:mainfrom
meethere:feat/escape-selects-the-paragraph
Oct 30, 2026
Merged

skylartaylor merged 1 commit into
storytold:mainfrom
meethere:feat/escape-selects-the-paragraph

Conversation

@meethere

@meethere meethere commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

What this changes

In Edit text & images, a click inside a paragraph opens it for typing. Selecting the whole box (to delete or move it) meant aiming for its thin frame; a press a few pixels inside put the caret in the text instead, which users found hard (#2203: "it many times selects individual character inside the box").

Now Esc in the inline editor leaves the text and selects the paragraph's box, as Acrobat does when Esc leaves a text box. Esc then Delete removes the paragraph (with #4111's object-selection delete), the arrow keys nudge it and its frame drags it. Esc still discards an unapplied draft, as before.

  • ObjectSelection::select_paragraph_next(page, block): the editor overlay has no document inventory, so it records the request.
  • ObjectSelection::prepare, which has the document, selects that paragraph after its own Escape check. prepare runs before the overlay in the frame, so the Esc that closed the editor cannot clear what it selected.
  • clear() drops a pending request, so a disabled tool or a cleared selection never selects later.

Refs #2203.

How you verified it

  • cargo fmt --check
  • cargo clippy -p pdfcraft-ui-egui -j 2 --all-targets -- -D warnings
  • cargo test -p pdfcraft-ui-egui -j 2: 506 lib + 1150 integration + 3 passed, 0 failed
  • cargo xtask layers, cargo xtask assets, cargo xtask parity

Test. editing::escape_in_an_open_paragraph_selects_its_box_for_delete clicks inside "Page 1" (the editor opens), presses Esc, checks "1 object selected" and that the text is unchanged, then Delete removes the paragraph and undo restores it; once with no draft and once with an unapplied draft. With the select_paragraph_next call removed it fails at "1 object selected".

Never-crash

  • No new unwrap/expect/panic!, indexing or unsafe; a request for a paragraph no longer in the inventory selects nothing

Provenance

  • No assets or copied code; behaviour from black-box use of Acrobat's Edit PDF (Esc leaves the text box selected)

Parity and reach

  • parity/acrobat-features.toml: the edit-existing-text entry notes Esc selects the box, citing the new test
  • Headless: deleting a paragraph is already the text_edit/object tools; this is a UI selection shortcut

Anything a reviewer should look at first

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Pressing Escape while editing a paragraph now discards any unapplied text and selects the unchanged paragraph, leaving it in the document.
    • The selected paragraph can then be deleted, and the deletion can be undone.
  • Tests

    • Added coverage for Escape behavior when the paragraph has no draft or has been changed to “Page one,” including deleting the paragraph and undoing that deletion.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 512b9af and 64fba7d.

Files selected for processing: 4
  • crates/ui-egui/src/tools/edit_text_ui.rs
  • crates/ui-egui/src/tools/object_ui.rs
  • crates/ui-egui/tests/it/editing.rs
  • parity/acrobat-features.toml

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

Escape in an open paragraph editor now discards the draft and schedules selection of the unchanged paragraph box. The next selection preparation pass selects the matching text object when it is present in the page inventory. Regression coverage checks deletion and undo.

Changes: Paragraph Escape selection

Layer / File(s) Summary
Deferred paragraph selection
crates/ui-egui/src/tools/object_ui.rs
ObjectSelection stores and clears a pending paragraph target. During prepare, it consumes the request and selects the matching text object when the page inventory contains it.
Escape cancellation and regression coverage
crates/ui-egui/src/tools/edit_text_ui.rs, crates/ui-egui/tests/it/editing.rs, parity/acrobat-features.toml
Escape cancels the editor and requests paragraph selection. The regression test covers unchanged and edited drafts, deletion, and undo. The parity evidence records the behavior.

Priority: Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant EditTextUi
  participant ObjectSelection
  participant PageInventory
  EditTextUi->>ObjectSelection: request selection for paragraph
  ObjectSelection->>ObjectSelection: prepare consumes deferred target
  ObjectSelection->>PageInventory: look up requested text target
  PageInventory-->>ObjectSelection: return matching text object
  ObjectSelection->>ObjectSelection: select matching text object
Loading

Possibly related issues

  • storytold/pdfcraft#2512 — The issue defines Escape behavior for inline paragraph editing, and this change preserves draft discard while adding paragraph-box selection after the editor closes.

Possibly related PRs

  • storytold/pdfcraft#4394 — Adds selected-object deletion handling and paragraph-deletion tests used after Escape selects the paragraph.
  • storytold/pdfcraft#4391 — Updates selected-object deletion in object_ui.rs and includes paragraph-deletion tests.
  • storytold/pdfcraft#4390 — Adds the Delete-key path for selected objects that this change reaches after Escape.

Suggested reviewers: aiongg, guillehr2

Merge Risk: Minimal · up to 64fba

No identified issue prevents merging the Escape-selection change after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main behavior change: pressing Escape in an open paragraph selects its box so Delete can remove it.
Description check Passed The description follows the repository template. It explains the behavior change, implementation approach, verification commands, regression test coverage, safety checks, parity update, reachability, …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description follows the repository template. It explains the behavior change, implementation approach, verification commands, regression test coverage, safety checks, parity update, reachability, and reviewer considerations.

Full details: Docstring Coverage

Explanation

Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 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

Comment @coderabbitai help to get the list of available commands.

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Esc in an open paragraph now selects its box, so Delete removes it: the paragraph editor asks the object selection to select the paragraph's text object on the next pass, which finds the object with that block index on the page, clears the previous selection and selects it; a pending request is cleared with the selection. I read the whole non-test diff (two small UI changes plus the parity entry); the gate and SonarCloud pass at 512b9af. I read the code and the CI results; I did not run the app.

@meethere

Copy link
Copy Markdown
Contributor Author

Checked by hand in a Windows release build (this branch together with the other two open fixes, #4605, #4620 and #4624, on main 418ac6a): works as described.

…es it

In Edit text & images a click inside a paragraph opens it for typing, and selecting the whole
box meant aiming for its thin frame: a press a few pixels inside put the caret in the text
instead (storytold#2203). Esc in the inline editor now leaves the text and selects the paragraph's box,
as Acrobat does, so Esc then Delete removes it, the arrow keys nudge it and its frame drags it.
Esc still discards an unapplied draft.

The editor asks `ObjectSelection` to select the paragraph, and the next `prepare`, which has the
document's inventory, selects it after its own Escape check, so the Esc that closed the editor
cannot clear the new selection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@meethere
meethere force-pushed the feat/escape-selects-the-paragraph branch from 512b9af to 64fba7d Compare October 27, 2026 02:25
@skylartaylor
skylartaylor merged commit b21af3b into storytold:main Oct 30, 2026
7 checks passed
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.

3 participants