collective
Browse / github.com/storytold/pdfcraft / patch
M5: E and B swap the Draw tool and the Eraser; holding E is temporary (#3900)
Dual-Behavior Eraser Tool Shortcut pdate the E shortcut key to support dual-mode behavior: • Tap E (Toggle): Pressing and releasing the key permanently switches the tool to the Eraser. Pressing it again (or tapping B) switches back to the Draw tool. • Hold E (Temporary Switch): Holding down the E key temporarily switches to the Eraser. Releasing the key instantly reverts the tool back to the active Draw tool.
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- AdrianTM (imported from pull request #4916, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +232 −2 in 7 files
- Review
- pending
- Shared
- 2026-10-28
- Id
09fb21f1b48d876940360e61
How it went
No other operator has reported using it yet.
Diff
raw · ROADMAP.md crates/ui-egui/src/app.rs crates/ui-egui/src/app/keys.rs crates/ui-egui/src/commands.rs crates/ui-egui/src/lib.rs crates/ui-egui/tests/it/comments.rs parity/acrobat-features.toml
@@ -745,3 +745,4 @@ Newest first: the date, what moved, and the new overall estimate. - **2026-10-18 (renderer, hayro patches 19/35):** A mesh shading (types 4–7) is sampled over its own triangles instead of the whole path: a BricsCAD plan with 310 one-triangle `sh` fills took 2 s per 1024 px tile at every zoom (and hit the 20 s watchdog on a busy machine); now about 70 ms, so zooming into dense topology drawings stays fluid. Estimates unchanged. - **2026-10-22 (PR #3227 integration):** M11: integrate upstream `80b7d215`; retain partial target-version compatibility and upstream content-boundary and image-size regression coverage. Scope and estimates unchanged.+- **2026-10-27 (M5, #3900):** E and B swap the Draw tool and the Eraser: a tap of E switches to the Eraser and stays (E again or B goes back), and holding E is the Eraser only while held.@@ -54,6 +54,7 @@ impl PdfCraftApp { right_panel_closed: false, quick_tool: QuickTool::Select, space_hand: None,+ eraser_key: None, full_screen_tool: None, comment_prefs: Default::default(), theme: ThemeKind::Light,@@ -3,6 +3,7 @@ use super::*; impl PdfCraftApp { pub(super) fn shortcuts(&mut self, ctx: &egui::Context) { use egui::Key;+ self.eraser_key_frame(ctx); let pasted = self.paste_key_released(ctx); // "Save changes?" is modal: its keys are its own (⌘D is Don't save there, not Document // properties; Escape cancels it rather than clearing a selection), and nothing may run@@ -81,7 +82,48 @@ impl PdfCraftApp { } } +/// How long E must be held for the Eraser to be only temporary (#3900).+use crate::comments::CommentTool;++const ERASER_HOLD_SECS: f64 = 0.35;+const DRAW: QuickTool = QuickTool::Comment(CommentTool::Ink);+const ERASER: QuickTool = QuickTool::Comment(CommentTool::Eraser);+ impl PdfCraftApp {+ /// Make `tool` the active one and the one its quick-bar group shows, as choosing it from the+ /// bar or its command does.+ fn set_drawing_tool(&mut self, tool: CommentTool) {+ self.comment_prefs.group_tool[tool.group()] = tool;+ self.quick_tool = QuickTool::Comment(tool);+ }++ /// The end of a held E (#3900), settled every frame, even under a dialog, so that the+ /// moment the key comes up is the moment it is read. A hold of [`ERASER_HOLD_SECS`] or more+ /// gives the Draw tool back; a tap leaves the Eraser. The hold is over, with nothing given+ /// back, once the user has chosen another tool. Space's temporary Hand may sit on top of the+ /// Eraser meanwhile: it then returns to Draw when Space is let go.+ pub(super) fn eraser_key_frame(&mut self, ctx: &egui::Context) {+ let Some(since) = self.eraser_key else { return };+ let on_top = self.quick_tool == QuickTool::Hand && self.space_hand == Some(ERASER);+ if self.quick_tool != ERASER && !on_top {+ self.eraser_key = None;+ return;+ }+ if ctx.input(|input| input.key_down(egui::Key::E)) {+ return;+ }+ self.eraser_key = None;+ if ctx.input(|input| input.time) - since >= ERASER_HOLD_SECS {+ if on_top {+ // The Hand gives Draw back when Space is let go, and the bar shows it already.+ self.space_hand = Some(DRAW);+ self.comment_prefs.group_tool[CommentTool::Ink.group()] = CommentTool::Ink;+ } else {+ self.set_drawing_tool(CommentTool::Ink);+ }+ }+ }+ /// The quick tools' keys, as in Acrobat: V selects, H pans, and holding Space pans until it /// is released. Plain letters, so not while a text field, a form field, a dialog or the /// palette has the keyboard.@@ -100,6 +142,25 @@ impl PdfCraftApp { if !free || !plain { return; }+ // E and B swap the Draw tool and the Eraser (#3900): a tap of E switches to the Eraser and+ // stays, another tap of E or a tap of B goes back; holding E is the Eraser only while held.+ // Only from one of the two, so E means nothing to any other tool.+ let first_press = |key: Key| {+ ctx.input(|input| input.events.iter().any(|e| matches!(e, egui::Event::Key { key: k, pressed: true, repeat: false, .. } if *k == key)))+ };+ if first_press(Key::E) {+ if self.quick_tool == ERASER {+ self.set_drawing_tool(CommentTool::Ink);+ self.eraser_key = None;+ } else if self.quick_tool == DRAW {+ self.set_drawing_tool(CommentTool::Eraser);+ self.eraser_key = Some(ctx.input(|input| input.time));+ }+ }+ if first_press(Key::B) && self.quick_tool == ERASER {+ self.set_drawing_tool(CommentTool::Ink);+ self.eraser_key = None;+ } if ctx.input(|input| input.key_pressed(Key::V)) { self.quick_tool = QuickTool::Select; self.space_hand = None;@@ -305,6 +305,9 @@ impl PdfCraftApp { let Some(tool) = crate::comments::CommentTool::from_command(tool) else { return false }; self.comment_prefs.group_tool[tool.group()] = tool; self.quick_tool = crate::QuickTool::Comment(tool);+ // Chosen on purpose, from the bar, a menu or a key: a held E no longer owns the+ // tool, so letting it go does not take the choice back (#3900).+ self.eraser_key = None; // Acrobat opens the Comments panel with the commenting tools, unless the user closed it. if self.right.is_none() && !self.comments_panel_closed { self.choose_right_panel(Some(RightPanel::Comments));@@ -269,6 +269,9 @@ pub struct PdfCraftApp { pub quick_tool: QuickTool, /// The tool to go back to when Space, held for a temporary Hand, is released. space_hand: Option<QuickTool>,+ /// When E went down for the Eraser (from the Draw tool), so that holding it gives the Draw+ /// tool back on release while a tap keeps the Eraser (#3900).+ eraser_key: Option<f64>, /// The tool before full screen, given back on leaving it (see [`Self::apply_full_screen`]). full_screen_tool: Option<QuickTool>, /// Comment author, per-tool colours and widths, pin.@@ -644,6 +644,167 @@ fn pen_strokes_stay_unselected_so_writing_continues() { assert_eq!(h.state().views[0].comments.selected, Some((0, 2))); } +/// The Draw and Eraser tools for the E-key tests, and what the quick bar's group shows for them.+mod eraser_key {+ pub use pdfcraft_ui_egui::QuickTool;+ pub use pdfcraft_ui_egui::comments::CommentTool;++ pub const DRAW: QuickTool = QuickTool::Comment(CommentTool::Ink);+ pub const ERASER: QuickTool = QuickTool::Comment(CommentTool::Eraser);+}++/// The active tool, and the tool the quick bar's group shows for it, which must agree.+fn tool_and_bar(h: &Harness<'static, PdfCraftApp>) -> (QuickTool, pdfcraft_ui_egui::comments::CommentTool) {+ let s = h.state();+ (s.quick_tool, s.comment_prefs.group_tool[eraser_key::CommentTool::Ink.group()])+}++fn eraser_harness() -> Harness<'static, PdfCraftApp> {+ let mut h = harness(|app| app.set_option("quick", "ink").unwrap());+ h.state_mut().set_option("panel", "none").unwrap();+ h.run_steps(2);+ h+}++/// #3900: E and B swap the Draw tool and the Eraser. A tap of E stays on the Eraser until E or B+/// is tapped again; holding E is the Eraser only while it is held. The quick bar follows.+#[test]+fn the_e_key_toggles_the_eraser_when_tapped_and_borrows_it_when_held() {+ use eraser_key::*;+ let mut h = eraser_harness();+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink));++ // A tap goes to the Eraser and stays; the next tap of E goes back.+ h.key_press(egui::Key::E);+ h.run_steps(4);+ assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "a tap of E stays on the Eraser, and the bar shows it");+ h.key_press(egui::Key::E);+ h.run_steps(4);+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "tapping E again goes back to Draw");++ // B also goes back.+ h.key_press(egui::Key::E);+ h.run_steps(4);+ assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser));+ h.key_press(egui::Key::B);+ h.run_steps(4);+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "B goes back to Draw");++ // Held: the Eraser while the key is down, Draw again the moment it is let go.+ h.key_down(egui::Key::E);+ h.run_steps(2);+ assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "held E is the Eraser");+ h.run_steps(6);+ assert_eq!(h.state().quick_tool, ERASER, "and stays while it is held");+ h.key_up(egui::Key::E);+ h.run_steps(3);+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "letting go gives the Draw tool back");++ // No other tool reads E.+ h.state_mut().set_option("quick", "square").unwrap();+ h.run_steps(2);+ h.key_press(egui::Key::E);+ h.run_steps(3);+ assert_eq!(h.state().quick_tool, QuickTool::Comment(CommentTool::Rectangle), "E leaves other tools alone");+}++/// #3900: E let go while a dialog is open is read when it comes up, not when the dialog closes: a+/// tap stays on the Eraser however long the dialog stays, a hold gives Draw back under it.+#[test]+fn the_e_key_is_settled_when_it_comes_up_even_under_a_dialog() {+ use eraser_key::*;+ // A tap whose key comes up under a dialog that then stays open a long time.+ let mut h = eraser_harness();+ h.key_down(egui::Key::E);+ h.run_steps(1);+ assert_eq!(h.state().quick_tool, ERASER);+ h.state_mut().set_option("dialog", "about").unwrap();+ h.key_up(egui::Key::E);+ h.run_steps(1);+ h.run_steps(8);+ h.state_mut().dialog = None;+ h.run_steps(4);+ assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "a tap is still a tap after the dialog");++ // A hold that comes up under a dialog gives Draw back at once.+ let mut h = eraser_harness();+ h.key_down(egui::Key::E);+ h.run_steps(1);+ h.state_mut().set_option("dialog", "about").unwrap();+ h.run_steps(4);+ h.key_up(egui::Key::E);+ h.run_steps(1);+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "a long hold ends with the key, dialog or not");++ // With a dialog open already, E starts nothing.+ let mut h = eraser_harness();+ h.state_mut().set_option("dialog", "about").unwrap();+ h.run_steps(3);+ h.key_press(egui::Key::E);+ h.run_steps(3);+ assert_eq!(h.state().quick_tool, DRAW, "a dialog has the keyboard");+}++/// #3900: choosing a tool while E is held ends the hold, even choosing the Eraser itself.+#[test]+fn choosing_a_tool_while_e_is_held_ends_the_hold() {+ use eraser_key::*;+ let mut h = eraser_harness();+ h.key_down(egui::Key::E);+ h.run_steps(2);+ assert_eq!(h.state().quick_tool, ERASER);+ h.state_mut().set_option("quick", "square").unwrap();+ h.run_steps(2);+ h.state_mut().execute("comment.eraser");+ h.run_steps(6);+ assert_eq!(h.state().quick_tool, ERASER, "the Eraser was chosen on purpose");+ h.key_up(egui::Key::E);+ h.run_steps(3);+ assert_eq!(h.state().quick_tool, ERASER, "and a late release does not take it away");++ // Choosing the Eraser itself, with nothing between: it was the Eraser already, but the+ // choice still ends the hold, whether the bar, a menu or a command made it.+ let mut h = eraser_harness();+ h.key_down(egui::Key::E);+ h.run_steps(6);+ assert!(h.state_mut().execute("comment.eraser"));+ h.run_steps(1);+ h.key_up(egui::Key::E);+ h.run_steps(3);+ assert_eq!(tool_and_bar(&h), (ERASER, CommentTool::Eraser), "choosing the Eraser while E is held keeps it");+}++/// #3900: Space's temporary Hand on top of a held E returns to Draw, whichever key comes up first.+#[test]+fn a_held_e_and_a_held_space_unwind_in_either_order() {+ use eraser_key::*;+ // E up first, while Space still holds the Hand.+ let mut h = eraser_harness();+ h.key_down(egui::Key::E);+ h.run_steps(6);+ h.key_down(egui::Key::Space);+ h.run_steps(2);+ assert_eq!(h.state().quick_tool, QuickTool::Hand, "Space is the Hand");+ h.key_up(egui::Key::E);+ h.run_steps(2);+ h.key_up(egui::Key::Space);+ h.run_steps(3);+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "E ended first: Draw comes back with the Hand");++ // Space up first: the Eraser is back under the still-held E, then E gives Draw back.+ let mut h = eraser_harness();+ h.key_down(egui::Key::E);+ h.run_steps(6);+ h.key_down(egui::Key::Space);+ h.run_steps(2);+ h.key_up(egui::Key::Space);+ h.run_steps(2);+ assert_eq!(h.state().quick_tool, ERASER, "Space up gives the Eraser back");+ h.key_up(egui::Key::E);+ h.run_steps(3);+ assert_eq!(tool_and_bar(&h), (DRAW, CommentTool::Ink), "and E up gives Draw");+}+ #[test] fn sticky_note_composer_posts_and_returns_to_select() { let mut h = harness(|app| app.set_option("quick", "note").unwrap());@@ -3739,8 +3739,8 @@ milestone = "M5" status = "shipped" commands = ["comment.eraser"] tools = []-evidence = ["crates/annot/src/tests.rs::the_eraser_cuts_strokes_and_removes_empty_drawings", "crates/ui-egui/tests/it/comments.rs::erasing_part_of_a_drawing", "crates/ui-egui/tests/it/comments.rs::the_eraser_rubs_as_wide_as_its_thickness_setting"]-notes = "cuts strokes where the eraser passes; an emptied drawing is deleted; one undo step. Its thickness setting sizes it on screen (3 to 72 pixels across; the default 2 pt is 12), preview included (#3747)"+evidence = ["crates/annot/src/tests.rs::the_eraser_cuts_strokes_and_removes_empty_drawings", "crates/ui-egui/tests/it/comments.rs::erasing_part_of_a_drawing", "crates/ui-egui/tests/it/comments.rs::the_eraser_rubs_as_wide_as_its_thickness_setting", "crates/ui-egui/tests/it/comments.rs::the_e_key_toggles_the_eraser_when_tapped_and_borrows_it_when_held"]+notes = "cuts strokes where the eraser passes; an emptied drawing is deleted; one undo step. Its thickness setting sizes it on screen (3 to 72 pixels across; the default 2 pt is 12), preview included (#3747). E and B swap the Draw tool and the Eraser (#3900): a tap of E (under 0.35 s) switches to the Eraser and stays, a second tap of E or a tap of B goes back to Draw, and holding E is the Eraser only until it is let go. The keys act only from the Draw tool or the Eraser, and not while a text field, a form field, a dialog or the palette has the keyboard; the shortcut is not yet in the shortcuts list and cannot be rebound" [[feature]] id = "comment.line"