From 94a12715cebd5fe24ae723ef4ec877553d3e76eb Mon Sep 17 00:00:00 2001 From: Perchenko Anton <101743951+PerchAnton@users.noreply.github.com> Date: Sat, 24 Oct 2026 17:44:33 +0300 Subject: [PATCH] Remember the annotation tool pin across restarts (#4253) (#4276) * M14 #4253: remember the annotation tool pin across restarts * M14 #4253: clarify roadmap timezone and Markdown formatting * Keep main's roadmap status lines; only the progress-log entry is new * Take main's forms.rs: main already gives each form test its own folder * Roadmap: drop the form-test folder note (main already has its own) --------- Co-authored-by: Brandon Thomas --- ROADMAP.md | 2 + crates/ui-egui/src/control.rs | 3 + crates/ui-egui/src/lib.rs | 11 ++ crates/ui-egui/tests/it/comment_pin.rs | 161 +++++++++++++++++++++++++ crates/ui-egui/tests/it/control.rs | 19 +++ crates/ui-egui/tests/it/main.rs | 1 + docs/development.md | 18 +++ parity/acrobat-features.toml | 12 ++ 8 files changed, 227 insertions(+) create mode 100644 crates/ui-egui/tests/it/comment_pin.rs diff --git a/ROADMAP.md b/ROADMAP.md index 72e123df4..b16494387 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -199,6 +199,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-24 (M14, #4253):** The annotation quick bar remembers Keep tool selected across restarts, including unpinning. Old and malformed settings keep safe defaults; the opt-in UI control channel reads and changes the same preference. Estimates unchanged. + - **2026-10-23 (M7, #4280):** Edit text names each standard stand-in font after its face (`PCEdTimesBold`, ...) instead of one shared `PCEdHelv`. Rewriting a bold paragraph whose subset lacks an accent no longer repoints the paragraphs set earlier in a regular stand-in at Times-Bold, and the editor's frame stays on the paragraph being edited. Pages saved by older builds keep their shared name and are not repaired. Estimates unchanged. - **2026-10-23 (M8, #3205 follow-up):** Phone-pattern redaction matches complete labelled Italian geographic numbers such as `Tel. +39 0426/123456`, with synthetic matcher and mark/apply/save/reopen coverage; unlabelled international slash numbers remain unsupported. - **2026-10-23 (M7, #2636 partial):** A new `PasteContent` edit pastes an added text box or image copied in the same document or another, keeping its style, picture, turns, flips and crop, as one undo step; `content_copy` drives it headlessly. A paste lands beside earlier copies (18 pt steps) and stays on the page. A pasted image reuses its picture object in its own document and after the first paste into another; there a colour profile is dropped and CMYK colours stored losslessly are turned to RGB. The desktop keys and menus follow in separate PRs. Estimates unchanged. diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 1550d446c..47c5b200a 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -36,6 +36,8 @@ //! for this session without changing the saved preference. `ui.state.default_mode` reports it. //! `theme=light|dark|system` changes the saved theme preference; `ui.state.theme_preference` //! reports the choice and `ui.state.theme` reports the resolved light/dark colours. +//! `comment-tool-pinned=true|false` remembers the annotation quick bar's pin across restarts; +//! `ui.state.comment_tool_pinned` reports the same preference. //! - `ui.open {path}`: open a file. A file whose tab is already open focuses it //! (`"focused": true`) instead of adding a tab; native opens report `pending: true` until //! they complete and are listed in `ui.state.pending_opens`; only a failed open replies `Err`. @@ -731,6 +733,7 @@ impl Host for crate::PdfCraftApp { "home": self.active.is_none(), "mode": format!("{:?}", self.mode), "default_mode": self.default_mode, + "comment_tool_pinned": self.comment_prefs.pinned, "left_panel": if self.left_open { json!(format!("{:?}", self.left)) } else { Value::Null }, "tools_side": if self.docking.tools_on_right() { "right" } else { "left" }, "right_panel": self.right.map(|r| format!("{r:?}")), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 3513dd6fa..e50145f8c 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2637,6 +2637,7 @@ impl PdfCraftApp { v["redact_prefs"] = self.redact_defaults.to_json(); // Set outside json! above, which is at serde_json's macro recursion limit. v["hide_empty_highlight_hover"] = serde_json::Value::Bool(self.comment_prefs.hide_empty_highlight_hover); + v["comment_tool_pinned"] = serde_json::Value::Bool(self.comment_prefs.pinned); v["show_secured_notice"] = serde_json::Value::Bool(self.show_secured_notice); v["organize_zoom"] = serde_json::json!(self.organize_zoom); // Canonical names survive changes to the order of PAPERS and do not depend on UI language. @@ -2769,6 +2770,9 @@ impl PdfCraftApp { if let Some(hide) = v["hide_empty_highlight_hover"].as_bool() { self.comment_prefs.hide_empty_highlight_hover = hide; } + if let Some(pinned) = v["comment_tool_pinned"].as_bool() { + self.comment_prefs.pinned = pinned; + } // Per-tool comment styles (#340): each field goes through its clamping setter, so a // malformed or hostile entry is clamped or ignored rather than trusted. if let Some(styles) = v["comment_styles"].as_array() { @@ -3111,6 +3115,13 @@ impl PdfCraftApp { }; } ("author", _) => self.comment_prefs.author = value.to_string(), + ("comment-tool-pinned", _) => { + self.comment_prefs.pinned = match value { + "true" => true, + "false" => false, + _ => return Err("comment-tool-pinned must be true or false".into()), + }; + } ("hide-empty-highlight-hover", _) => { self.comment_prefs.hide_empty_highlight_hover = match value { "true" => true, diff --git a/crates/ui-egui/tests/it/comment_pin.rs b/crates/ui-egui/tests/it/comment_pin.rs new file mode 100644 index 000000000..af9f80204 --- /dev/null +++ b/crates/ui-egui/tests/it/comment_pin.rs @@ -0,0 +1,161 @@ +//! The annotation quick bar's pin is a preference, including across older settings files. + +use egui::{Pos2, pos2}; +use egui_kittest::{Harness, kittest::Queryable}; +use pdfcraft_ui_egui::{PdfCraftApp, QuickTool, comments::CommentTool}; +use serde_json::{Value, json}; + +// Contributor-original blank page: no document assets or fonts are needed. +const PDF: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 200] >> endobj +xref +0 4 +0000000000 65535 f\r +0000000009 00000 n\r +0000000058 00000 n\r +0000000115 00000 n\r +trailer << /Size 4 /Root 1 0 R >> +startxref +186 +%%EOF"; + +/// Start a fresh shell with the state a previous process saved. +fn harness(saved: Option) -> Harness<'static, PdfCraftApp> { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + if let Some(saved) = saved { + app.restore(&saved); + } + app.set_option("language", "en").unwrap(); + app.set_option("author", "Tester").unwrap(); + app.open_bytes("pin.pdf", None, PDF.to_vec()).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("quick", "callout").unwrap(); + app + }); + crate::support::wait_until(&mut h, "the blank page", |app| app.views[0].page_screen_rect(0).is_some() && !app.render_pending()); + h +} + +/// Place and post a callout at known PDF coordinates, independently of panel and zoom changes. +fn callout(h: &mut Harness<'static, PdfCraftApp>, y: f32) { + let r = h.state().views[0].page_screen_rect(0).unwrap(); + let at = |x: f32| -> Pos2 { pos2(r.left() + x / 300.0 * r.width(), r.top() + (200.0 - y) / 200.0 * r.height()) }; + h.hover_at(at(40.0)); + h.run_steps(1); + h.drag_at(at(40.0)); + h.run_steps(1); + h.hover_at(at(140.0)); + h.run_steps(1); + h.drop_at(at(140.0)); + h.run_steps(3); + h.query_all_by(|node| node.placeholder() == Some("Type text")).next().unwrap().type_text("Remember this"); + h.run_steps(2); + h.get_by_label("Post").click(); + h.run_steps(3); +} + +/// Optional private visual evidence, kept outside the repository by the test runner. +fn shot(h: &mut Harness<'static, PdfCraftApp>, name: &str) { + if let Ok(dir) = std::env::var("PDFCRAFT_PIN_SHOTS") { + h.render().unwrap().save(std::path::Path::new(&dir).join(name)).unwrap(); + } +} + +#[test] +fn pin_from_quick_bar_survives_restart_and_unpinning() { + let _gpu = crate::gpu(); + let mut h = harness(None); + h.get_by_label("Keep tool selected").click(); + h.run_steps(3); + assert!(h.state().comment_prefs.pinned); + let doc = h.state().session.get(h.state().views[0].id).unwrap(); + assert!(doc.repair_log().is_empty(), "the synthetic page opens without repair"); + assert_eq!(doc.bytes.as_slice(), PDF, "changing this preference leaves the PDF unchanged"); + assert!(!doc.dirty); + let saved = h.state().persist(); + drop(h); + + let mut h = harness(Some(saved)); + assert!(h.state().comment_prefs.pinned, "the pin survives a fresh shell"); + h.get_by_label("Keep tool selected: on"); + shot(&mut h, "pin-restored.png"); + callout(&mut h, 140.0); + crate::support::wait_until(&mut h, "the pinned callout", |app| app.session.get(app.views[0].id).unwrap().info.annotations.len() == 1); + assert!(matches!(h.state().quick_tool, QuickTool::Comment(CommentTool::Callout)), "the restored pin keeps the tool selected after use"); + + h.get_by_label("Keep tool selected: on").click(); + h.run_steps(3); + assert!(!h.state().comment_prefs.pinned); + shot(&mut h, "pin-disabled.png"); + let saved = h.state().persist(); + callout(&mut h, 60.0); + crate::support::wait_until(&mut h, "the unpinned callout returning to Select", |app| { + app.session.get(app.views[0].id).unwrap().info.annotations.len() == 2 && matches!(app.quick_tool, QuickTool::Select) + }); + assert!(!matches!(h.state().quick_tool, QuickTool::Comment(_)), "unpinning restores one-shot use"); + + let mut restored = PdfCraftApp::new(); + restored.comment_prefs.pinned = true; + restored.restore(&saved); + assert!(!restored.comment_prefs.pinned, "unpinning survives the next restart too"); +} + +#[test] +fn saved_pin_round_trips_both_boolean_values() { + for pinned in [true, false] { + let mut app = PdfCraftApp::new(); + app.comment_prefs.pinned = pinned; + let saved = app.persist(); + let value: Value = serde_json::from_str(&saved).unwrap(); + assert_eq!(value["comment_tool_pinned"], json!(pinned)); + let mut restored = PdfCraftApp::new(); + restored.comment_prefs.pinned = !pinned; + restored.restore(&saved); + assert_eq!(restored.comment_prefs.pinned, pinned); + assert!(matches!(restored.quick_tool, QuickTool::Select), "remembering the pin does not select a drawing tool at startup"); + } +} + +#[test] +fn restoring_explicit_pin_choice_overrides_current_choice() { + for pinned in [true, false] { + let mut app = PdfCraftApp::new(); + app.comment_prefs.pinned = !pinned; + app.restore(&json!({"comment_tool_pinned": pinned}).to_string()); + assert_eq!(app.comment_prefs.pinned, pinned); + } +} + +#[test] +fn legacy_settings_keep_the_default_and_other_preferences() { + let mut app = PdfCraftApp::new(); + assert!(!app.comment_prefs.pinned); + let legacy = r#"{"author":"Legacy reviewer","comment_styles":[{"tool":"comment.line","color":[0.25,0.5,1.0]}]}"#; + app.restore(legacy); + assert!(!app.comment_prefs.pinned, "old settings retain the default one-shot behavior"); + assert_eq!(app.comment_prefs.author, "Legacy reviewer"); + assert_eq!(app.comment_prefs.style(CommentTool::Line).color, [0.25, 0.5, 1.0]); + app.comment_prefs.pinned = true; + app.restore(legacy); + assert!(app.comment_prefs.pinned, "a partial restore without this key leaves the current choice alone"); +} + +#[test] +fn malformed_pin_values_preserve_the_current_choice() { + for pinned in [false, true] { + let mut app = PdfCraftApp::new(); + app.comment_prefs.pinned = pinned; + for value in [json!(null), json!(0), json!(1), json!(-1), json!("true"), json!("false"), json!([]), json!({})] { + app.restore(&json!({"comment_tool_pinned": value, "author": "Still restored"}).to_string()); + assert_eq!(app.comment_prefs.pinned, pinned, "invalid pin value: {value}"); + assert_eq!(app.comment_prefs.author, "Still restored", "other valid preferences still restore"); + } + for invalid in ["", "{", r#"{"comment_tool_pinned":true"#, "[]", "null", "false", "0"] { + app.restore(invalid); + assert_eq!(app.comment_prefs.pinned, pinned); + } + } +} diff --git a/crates/ui-egui/tests/it/control.rs b/crates/ui-egui/tests/it/control.rs index 1ffd9a443..7099ece40 100644 --- a/crates/ui-egui/tests/it/control.rs +++ b/crates/ui-egui/tests/it/control.rs @@ -88,6 +88,25 @@ fn ok(h: &mut Harness<'static, PdfCraftApp>, c: &ControlClient, method: &str, pa call(h, c, method, params).unwrap_or_else(|e| panic!("{method}: {e}")) } +#[test] +fn comment_pin_is_reported_and_persistent() { + let (mut h, c) = harness(); + assert_eq!(ok(&mut h, &c, "ui.state", json!({}))["comment_tool_pinned"], false); + for pinned in [true, false] { + ok(&mut h, &c, "ui.set", json!({"key":"comment-tool-pinned","value":pinned})); + assert_eq!(ok(&mut h, &c, "ui.state", json!({}))["comment_tool_pinned"], pinned); + let mut restored = PdfCraftApp::new(); + restored.comment_prefs.pinned = !pinned; + restored.restore(&h.state().persist()); + assert_eq!(restored.comment_prefs.pinned, pinned); + for invalid in [json!("yes"), json!(1), json!(null), json!([]), json!({})] { + assert!(call(&mut h, &c, "ui.set", json!({"key":"comment-tool-pinned","value":invalid})).is_err()); + assert_eq!(ok(&mut h, &c, "ui.state", json!({}))["comment_tool_pinned"], pinned); + } + assert_eq!(ok(&mut h, &c, "ui.state", json!({}))["documents"][0]["dirty"], false); + } +} + #[test] fn thumbnail_size_is_settable_and_inspectable_through_the_control_channel() { let (mut h, c) = harness(); diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index 0e543a788..b49642fe0 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -29,6 +29,7 @@ mod camera; mod column_select; mod combine_grid; mod commands; +mod comment_pin; mod comments; mod compare; mod content; diff --git a/docs/development.md b/docs/development.md index 1142fed33..e50907ba5 100644 --- a/docs/development.md +++ b/docs/development.md @@ -28,6 +28,24 @@ the low-disk artifacts, run `cargo clean --profile low-disk`. Building PdfCraft requires Rust 1.95 or later: the UI crate and its `egui` and `craft-ui` dependencies need it, and the workspace `rust-version` states the same minimum. +## Annotation tool pin + +The annotation quick bar's **Keep tool selected** pin is a shared preference, stored as the +boolean `comment_tool_pinned`. It survives application restarts, including when switched off. +Startup uses Select; the remembered pin applies when an annotation tool is picked. Older settings +without the key keep the default one-shot behavior. Non-boolean values are ignored, preserving +the current choice and the other valid settings. + +The existing opt-in UI control channel uses the same preference: + +```sh +pdfcraft-cli ui --control /path/to/control.json set key=comment-tool-pinned value=true +pdfcraft-cli ui --control /path/to/control.json state +``` + +`ui.state.comment_tool_pinned` reports the boolean. `ui.set` accepts `true` or `false` and refuses +other values before changing the preference. This changes interface settings only. + ## Interface appearance Display theme saves System, Light or Dark separately from the palette currently drawn. New diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 37716930f..ecd2d75f3 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -3934,6 +3934,18 @@ tools = [] evidence = ["crates/ui-egui/tests/it/comments.rs::make_current_properties_default", "crates/ui-egui/tests/it/comments.rs::properties_dialog_makes_the_strikethrough_colour_default"] notes = "colour, opacity and line width become the tool's defaults (canvas and Comments panel menus, and Acrobat's Make Properties Default box in Comment Properties, applied on OK with the edited look) and survive a restart: the per-tool styles are persisted with the settings and restored through their clamping setters (#340)" +[[feature]] +id = "comment.keep-tool-selected" +title = "Keep annotation tools selected between uses and restarts" +area = "E" +tier = "P2" +milestone = "M14" +status = "shipped" +commands = ["comment.line"] +tools = [] +evidence = ["crates/ui-egui/tests/it/comment_pin.rs::pin_from_quick_bar_survives_restart_and_unpinning", "crates/ui-egui/tests/it/comment_pin.rs::legacy_settings_keep_the_default_and_other_preferences", "crates/ui-egui/tests/it/comment_pin.rs::malformed_pin_values_preserve_the_current_choice", "crates/ui-egui/tests/it/control.rs::comment_pin_is_reported_and_persistent"] +notes = "The annotation quick bar's shared pin survives restarts as a boolean setting (#4253); unpinning is remembered too. Old settings default to one-shot use, and malformed values keep the current choice. UI-only preference, reachable through opt-in ui.set key=comment-tool-pinned and reported by ui.state.comment_tool_pinned. Startup uses Select; the shared pin applies when an annotation tool is picked. Independent per-tool pins and restoring the active tool are outside this preference." + [[feature]] id = "comment.author-identity" title = "Author name from identity preferences"