From 6d2bd31b2399d49054c87d8eb4b784284759a34f Mon Sep 17 00:00:00 2001 From: Livio Oswald <125505497+Lcoding15@users.noreply.github.com> Date: Mon, 9 Nov 2026 11:30:10 +0100 Subject: [PATCH] M3: a pinned folder on Home collapses to its heading, and stays collapsed (#5295) (#5341) --- crates/ui-egui/src/app/control.rs | 1 + crates/ui-egui/src/app/prefs.rs | 2 + crates/ui-egui/src/control.rs | 2 + crates/ui-egui/src/control/host.rs | 2 + crates/ui-egui/src/home/folders_ui.rs | 73 ++++++++++++++++++++++++++- crates/ui-egui/tests/it/control.rs | 41 +++++++++++++++ crates/ui-egui/tests/it/folders.rs | 65 ++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 8 files changed, 187 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/app/control.rs b/crates/ui-egui/src/app/control.rs index 065bd7a228..aaf5c009f8 100644 --- a/crates/ui-egui/src/app/control.rs +++ b/crates/ui-egui/src/app/control.rs @@ -357,6 +357,7 @@ impl PdfCraftApp { }; } ("author", _) => self.comment_prefs.author = value.to_string(), + ("folder-collapsed", _) => self.set_folder_collapsed(value)?, ("comment-tool-pinned", _) => { self.comment_prefs.pinned = match value { "true" => true, diff --git a/crates/ui-egui/src/app/prefs.rs b/crates/ui-egui/src/app/prefs.rs index 1ec33884fe..6a6452d463 100644 --- a/crates/ui-egui/src/app/prefs.rs +++ b/crates/ui-egui/src/app/prefs.rs @@ -87,6 +87,7 @@ impl PdfCraftApp { "reload_changed_files": self.reload_changed_files, "system_title_bar": self.system_title_bar, "pinned_folders": self.pinned.folders, + "pinned_folders_collapsed": self.pinned.collapsed, "theme": self.theme_preference, "scroll_speed": self.scroll_speed, "invert_middle_drag": self.invert_middle_drag, @@ -252,6 +253,7 @@ impl PdfCraftApp { self.text_links = on; } self.pinned.restore(&v["pinned_folders"]); + self.pinned.restore_collapsed(&v["pinned_folders_collapsed"]); if let Some(items) = v.get("pinned_tools").and_then(serde_json::Value::as_array) { let mut tools = Vec::new(); for item in items { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 44e5481a11..59fb97d7d1 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -45,6 +45,8 @@ //! `hotbar-size=24..64` controls its button size in logical pixels; `ui.state.hotbar` //! reports its tools, relative position, orientation and strip preference. These options persist across restarts. //! `ui.state.comment_tool_pinned` reports the same preference. +//! `folder-collapsed==on|off` folds a pinned folder on Home down to its heading or +//! opens it again; `ui.state.pinned_folders` lists each pinned folder and whether it is. //! - `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`. diff --git a/crates/ui-egui/src/control/host.rs b/crates/ui-egui/src/control/host.rs index acccf5ead3..cf8e5e7fcc 100644 --- a/crates/ui-egui/src/control/host.rs +++ b/crates/ui-egui/src/control/host.rs @@ -146,6 +146,8 @@ impl Host for crate::PdfCraftApp { "default_mode": self.default_mode, "hotbar": self.hotbar, "comment_tool_pinned": self.comment_prefs.pinned, + // Home's pinned folders, in order, and whether each is folded down to its heading. + "pinned_folders": self.pinned.folders.iter().map(|f| json!({ "folder": f, "collapsed": self.pinned.collapsed.contains(f) })).collect::>(), "left_panel": if self.left_open { json!(format!("{:?}", self.left)) } else { Value::Null }, "tools_side": if self.side_panels.tools_on_right() { "right" } else { "left" }, "rail_side": if self.side_panels.rail_on_right() { "right" } else { "left" }, diff --git a/crates/ui-egui/src/home/folders_ui.rs b/crates/ui-egui/src/home/folders_ui.rs index fe9b2c3706..60e130420b 100644 --- a/crates/ui-egui/src/home/folders_ui.rs +++ b/crates/ui-egui/src/home/folders_ui.rs @@ -55,6 +55,8 @@ pub struct PinnedFolders { pub listings: Vec, /// Folders whose "Show more" is open. pub expanded: Vec, + /// Folders folded down to their heading (#5295); kept in settings. + pub collapsed: Vec, /// A listing in progress: the worker fills the slot. scan: Option>>>>, /// When (egui time) the last listing started. @@ -86,8 +88,34 @@ impl PinnedFolders { self.folders = folders; self.listings.clear(); self.expanded.clear(); + self.collapsed.clear(); self.stale = true; } + + /// Restore which folders are collapsed, after [`Self::restore`]: only folders that are + /// pinned, each once, so untrusted settings can't grow the list. + pub(crate) fn restore_collapsed(&mut self, value: &serde_json::Value) { + let Some(items) = value.as_array() else { return }; + self.collapsed.clear(); + for folder in items.iter().filter_map(serde_json::Value::as_str) { + if self.folders.iter().any(|f| f == folder) && !self.collapsed.iter().any(|f| f == folder) { + self.collapsed.push(folder.to_string()); + } + } + } + + /// Fold pinned `folder` down to its heading (`collapsed`) or open it again. Returns false, + /// changing nothing, when `folder` isn't pinned. + pub(crate) fn set_collapsed(&mut self, folder: &str, collapsed: bool) -> bool { + if !self.folders.iter().any(|f| f == folder) { + return false; + } + self.collapsed.retain(|f| f != folder); + if collapsed { + self.collapsed.push(folder.to_string()); + } + true + } } /// `path` as a folder to pin: trimmed, without a trailing separator, and absolute; `None` if it @@ -221,6 +249,25 @@ impl PdfCraftApp { self.pinned.folders.retain(|f| f != folder); self.pinned.listings.retain(|l| l.folder != folder); self.pinned.expanded.retain(|f| f != folder); + self.pinned.collapsed.retain(|f| f != folder); + } + + /// `ui.set folder-collapsed =on|off` (CodeRabbit on #5341): the control channel's + /// way to fold a pinned folder down to its heading or open it again, as its chevron does. + /// `ui.state` lists the pinned folders and which are collapsed. The folder is named by its + /// path as pinned; the last `=` splits it from the state, so a path may hold `=` too. + pub(crate) fn set_folder_collapsed(&mut self, value: &str) -> Result<(), String> { + let (folder, state) = value.rsplit_once('=').ok_or("folder-collapsed must be =on or =off")?; + let collapsed = match state { + "on" => true, + "off" => false, + _ => return Err("folder-collapsed must be =on or =off".into()), + }; + if self.pinned.set_collapsed(folder, collapsed) { + Ok(()) + } else { + Err(format!("folder-collapsed: {folder} is not a pinned folder (ui.state lists them)")) + } } /// List the pinned folders again if the last listing is older than a few seconds or the @@ -308,10 +355,27 @@ pub(crate) fn section(app: &mut PdfCraftApp, ui: &mut egui::Ui) { let mut open = None; let mut unpin = None; let mut toggle = None; + let mut fold = None; for folder in &app.pinned.folders { let name = folder_name(folder); + // Buttons name their folder; two pinned folders of the same name add where each is. + let shared = app.pinned.folders.iter().any(|f| f != folder && folder_name(f) == name); + let named = if shared { format!("{name} ({})", location(folder)) } else { name.clone() }; + let collapsed = app.pinned.collapsed.contains(folder); ui.add_space(6.0); ui.horizontal(|ui| { + let (chevron, tip, label) = if collapsed { + ("chevron-right", tl!("Expand"), tl!("Expand {folder}")) + } else { + ("chevron-down", tl!("Collapse"), tl!("Collapse {folder}")) + }; + let resp = icons::button(ui, chevron, 22.0, false, tip); + // Every folder has one: say which folder it folds. + let label = crate::i18n::fmt(label, &[("folder", &named)]); + resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, &label)); + if resp.clicked() { + fold = Some(folder.clone()); + } let (icon, _) = ui.allocate_exact_size(vec2(22.0, 22.0), Sense::hover()); icons::paint(ui, icon, "folder", 20.0, t.icon); // Folder names and paths are file/path chrome: explicitly left to right, so an @@ -327,13 +391,16 @@ pub(crate) fn section(app: &mut PdfCraftApp, ui: &mut egui::Ui) { ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { let resp = widgets::ghost_button(ui, "x", tl!("Unpin")); // Every folder has an Unpin button: say which one it is. - let label = crate::i18n::fmt(tl!("Unpin {folder}"), &[("folder", &name)]); + let label = crate::i18n::fmt(tl!("Unpin {folder}"), &[("folder", &named)]); resp.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, &label)); if resp.clicked() { unpin = Some(folder.clone()); } }); }); + if collapsed { + continue; + } let note = |ui: &mut egui::Ui, text: &str| { ui.add_space(2.0); ui.label(egui::RichText::new(text).color(t.text_muted)); @@ -405,6 +472,10 @@ pub(crate) fn section(app: &mut PdfCraftApp, ui: &mut egui::Ui) { app.pinned.expanded.push(folder); } } + if let Some(folder) = fold { + let collapsed = app.pinned.collapsed.contains(&folder); + app.pinned.set_collapsed(&folder, !collapsed); + } if let Some(path) = open { app.open_recent(&path); } diff --git a/crates/ui-egui/tests/it/control.rs b/crates/ui-egui/tests/it/control.rs index 5bbc26c991..8e397c803b 100644 --- a/crates/ui-egui/tests/it/control.rs +++ b/crates/ui-egui/tests/it/control.rs @@ -2176,6 +2176,47 @@ fn routine_continuous_scrolling_does_not_add_navigation_history() { assert!(h.state().views[0].forward.is_empty()); } +/// CodeRabbit on #5341: the control channel folds a pinned folder on Home down to its heading +/// and opens it again, as its chevron does, and `ui.state` says which are folded. Folders that +/// aren't pinned, and states other than on and off, are refused with the reason. +#[test] +fn control_collapses_and_expands_a_pinned_folder() { + let dir = std::env::temp_dir().join(format!("pdfcraft-ui-fold-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("inside.pdf"), fixture(1)).unwrap(); + let folder = dir.to_string_lossy().into_owned(); + let slot: Arc>> = Arc::default(); + let (s, pin) = (slot.clone(), folder.clone()); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 900.0)).build_eframe(move |cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.run_inline = true; + app.pin_folder(&pin); + *s.lock().unwrap() = Some(app.attach_control(&cc.egui_ctx)); + app + }); + h.run_steps(4); + let c = slot.lock().unwrap().take().unwrap(); + let folded = |h: &mut Harness<'static, PdfCraftApp>| ok(h, &c, "ui.state", json!({}))["pinned_folders"].clone(); + assert_eq!(folded(&mut h), json!([{ "folder": folder, "collapsed": false }])); + h.get_by_label("inside.pdf"); + ok(&mut h, &c, "ui.set", json!({ "key": "folder-collapsed", "value": format!("{folder}=on") })); + h.run_steps(2); + assert_eq!(folded(&mut h), json!([{ "folder": folder, "collapsed": true }])); + assert!(h.query_by_label("inside.pdf").is_none(), "a collapsed folder lists nothing"); + let unpinned = call(&mut h, &c, "ui.set", json!({ "key": "folder-collapsed", "value": "/not/pinned=on" })).unwrap_err(); + assert!(unpinned.contains("not a pinned folder"), "{unpinned}"); + let bad = call(&mut h, &c, "ui.set", json!({ "key": "folder-collapsed", "value": format!("{folder}=yes") })).unwrap_err(); + assert!(bad.contains("=on or"), "{bad}"); + assert_eq!(folded(&mut h), json!([{ "folder": folder, "collapsed": true }]), "refusals change nothing"); + ok(&mut h, &c, "ui.set", json!({ "key": "folder-collapsed", "value": format!("{folder}=off") })); + h.run_steps(2); + assert_eq!(folded(&mut h), json!([{ "folder": folder, "collapsed": false }])); + h.get_by_label("inside.pdf"); + let _ = std::fs::remove_dir_all(&dir); +} + #[test] fn accessible_labels_on_format_text_and_pin_dialog() { let (mut h, c) = harness(); diff --git a/crates/ui-egui/tests/it/folders.rs b/crates/ui-egui/tests/it/folders.rs index 583cc35eaa..a3e954b6a2 100644 --- a/crates/ui-egui/tests/it/folders.rs +++ b/crates/ui-egui/tests/it/folders.rs @@ -140,6 +140,71 @@ fn unpinning_removes_the_folder_but_not_its_files() { assert!(path.exists(), "unpinning never touches the files"); } +/// #5295: a pinned folder folds down to its heading and back, the other folders stay as they +/// are, and the folded state is kept across a restart. +#[test] +fn a_pinned_folder_collapses_to_its_heading_and_stays_collapsed_after_a_restart() { + let (first, second) = (TempFolder::new("fold-a"), TempFolder::new("fold-b")); + first.file("alpha.pdf", FIXTURE, Duration::from_secs(10)); + second.file("beta.pdf", FIXTURE, Duration::from_secs(10)); + let (a, b) = (first.path(), second.path()); + let pins = (a.clone(), b.clone()); + let mut h = harness(move |app| { + app.pin_folder(&pins.0); + app.pin_folder(&pins.1); + }); + h.run_steps(2); + let name = pdfcraft_ui_egui::folders_ui::folder_name(&a); + h.get_by_label(&format!("Collapse {name}")).click(); + h.run_steps(2); + assert!(h.query_by_label("alpha.pdf").is_none(), "a collapsed folder lists nothing"); + h.get_by_label("beta.pdf"); + h.get_by_label(&format!("Unpin {name}")); + assert_eq!(h.state().pinned.collapsed, std::slice::from_ref(&a)); + + let mut restarted = PdfCraftApp::new(); + restarted.restore(&h.state().persist()); + assert_eq!(restarted.pinned.collapsed, std::slice::from_ref(&a)); + + h.get_by_label(&format!("Expand {name}")).click(); + h.run_steps(2); + h.get_by_label("alpha.pdf"); + assert!(h.state().pinned.collapsed.is_empty()); + + // Settings are untrusted: only pinned folders, each once. + let settings = serde_json::json!({ "pinned_folders": [a, b], "pinned_folders_collapsed": [b, b, "/not/pinned", 5] }); + restarted.restore(&settings.to_string()); + assert_eq!(restarted.pinned.collapsed, std::slice::from_ref(&b)); + restarted.unpin_folder(&b); + assert!(restarted.pinned.collapsed.is_empty(), "unpinning forgets the folded state"); +} + +/// Two pinned folders of the same name: their buttons say where each one is, so a screen reader +/// can tell them apart. +#[test] +fn folders_of_the_same_name_have_buttons_that_tell_them_apart() { + let root = TempFolder::new("same-name"); + let (a, b) = (root.0.join("work").join("Scans"), root.0.join("home").join("Scans")); + std::fs::create_dir_all(&a).unwrap(); + std::fs::create_dir_all(&b).unwrap(); + let (a, b) = (a.to_string_lossy().into_owned(), b.to_string_lossy().into_owned()); + let pins = (a.clone(), b.clone()); + let mut h = harness(move |app| { + app.pin_folder(&pins.0); + app.pin_folder(&pins.1); + }); + h.run_steps(2); + for (folder, other) in [(&a, &b), (&b, &a)] { + let named = format!("Scans ({})", pdfcraft_ui_egui::folders_ui::location(folder)); + assert_ne!(named, format!("Scans ({})", pdfcraft_ui_egui::folders_ui::location(other))); + h.get_by_label(&format!("Collapse {named}")); + h.get_by_label(&format!("Unpin {named}")); + } + h.get_by_label(&format!("Collapse Scans ({})", pdfcraft_ui_egui::folders_ui::location(&a))).click(); + h.run_steps(2); + assert_eq!(h.state().pinned.collapsed, std::slice::from_ref(&a), "the button folds its own folder"); +} + #[test] fn pinned_folders_survive_a_restart() { let dir = TempFolder::new("restart"); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 761a0a44d7..d7a6a406eb 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -1706,8 +1706,8 @@ milestone = "M3" status = "shipped" commands = ["file.pin_folder"] tools = [] -evidence = ["crates/ui-egui/tests/it/ui.rs::home_shows_welcome_and_tools", "crates/ui-egui/tests/it/folders.rs::a_pinned_folder_lists_its_newest_pdfs_on_home", "crates/ui-egui/tests/it/folders.rs::clicking_a_pinned_pdf_opens_it_and_adds_it_to_recent", "crates/ui-egui/tests/it/folders.rs::pinned_folders_survive_a_restart"] -notes = "Desktop: folders pinned to Home (File ▸ Pin folder to Home…, up to 8) list their newest PDFs, read on a worker thread and refreshed every few seconds while Home shows; a folder that can't be read (moved, offline synced drive) stays pinned and says so. Pinned folders are local paths, so a cloud drive works through its synced folder (iCloud Drive, OneDrive, Dropbox). Not yet: subfolders, search within a pinned folder, the web build." +evidence = ["crates/ui-egui/tests/it/ui.rs::home_shows_welcome_and_tools", "crates/ui-egui/tests/it/folders.rs::a_pinned_folder_lists_its_newest_pdfs_on_home", "crates/ui-egui/tests/it/folders.rs::clicking_a_pinned_pdf_opens_it_and_adds_it_to_recent", "crates/ui-egui/tests/it/folders.rs::pinned_folders_survive_a_restart", "crates/ui-egui/tests/it/folders.rs::a_pinned_folder_collapses_to_its_heading_and_stays_collapsed_after_a_restart", "crates/ui-egui/tests/it/control.rs::control_collapses_and_expands_a_pinned_folder"] +notes = "Desktop: folders pinned to Home (File ▸ Pin folder to Home…, up to 8) list their newest PDFs, read on a worker thread and refreshed every few seconds while Home shows; a folder that can't be read (moved, offline synced drive) stays pinned and says so. Each folder collapses to its heading, and stays collapsed across restarts. Pinned folders are Home-view settings, not a document's, so the engine tools have none; the control channel folds them (`ui.set folder-collapsed =on|off`) and `ui.state` lists them. Pinned folders are local paths, so a cloud drive works through its synced folder (iCloud Drive, OneDrive, Dropbox). Not yet: subfolders, search within a pinned folder, the web build." [[feature]] id = "view.all-tools"