diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index a14332c82..258a804b7 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -798,7 +798,7 @@ fn local_rows(app: &mut LightcraftApp, ui: &mut egui::Ui, now: &LocalNow) { for (i, (name, path)) in local.places.iter().enumerate() { let transient = local.browse_root.as_deref() == Some(path.as_str()); let reveal = local.owner == Some(i); - folder_tree(app, ui, name, path, 0.0, current, reveal, transient); + folder_tree(app, ui, path, name, path, 0.0, current, reveal, transient); } if app.services.pick_folder.is_some() && row(app, ui, "local:browse", Icon::Plus, crate::i18n::tr("Browse Folder…"), None, false, 0.0).clicked() { @@ -990,10 +990,14 @@ pub(crate) fn fs_cached(ui: &egui::Ui, kind: &'static /// subfolders. In the tree that holds the folder being browsed (`reveal`), the folders on the /// way down to it open whenever the browsed folder changes, so it shows highlighted in place. /// `transient`: a top-level row listed for this session only (it offers Keep in Local). +/// `place`: the top-level folder this tree hangs from. A folder can be listed in more than one +/// tree (Pictures on its own and inside Home): each row opens and closes by itself, and their +/// triangles have ids of their own, else the copy drawn last takes the other's clicks (#1093). #[allow(clippy::too_many_arguments)] fn folder_tree( app: &mut LightcraftApp, ui: &mut egui::Ui, + place: &str, name: &str, path: &str, indent: f32, @@ -1001,13 +1005,13 @@ fn folder_tree( reveal: bool, transient: bool, ) { - let open_id = egui::Id::new(("folder-open", path.to_string())); + let open_id = egui::Id::new(("folder-open", place.to_string(), path.to_string())); let sel = current.is_some_and(|c| same_folder(c, path)); let on_the_way = !sel && current.is_some_and(|c| lightcraft_catalog::query::folder_within(c, path)); let mut open = tree_open(ui, open_id, false); if reveal && on_the_way { // opened once per browsed folder: collapsing it again afterwards sticks - let revealed_id = egui::Id::new(("folder-revealed", path.to_string())); + let revealed_id = egui::Id::new(("folder-revealed", place.to_string(), path.to_string())); let target = current.map(str::to_string); if ui.data(|d| d.get_temp::>(revealed_id)) != Some(target.clone()) { open = true; @@ -1070,7 +1074,7 @@ fn folder_tree( }); if open && indent < 12.0 * 8.0 { for (n, p) in subfolders(ui, path) { - folder_tree(app, ui, &n, &p, indent + 12.0, current, reveal, false); + folder_tree(app, ui, place, &n, &p, indent + 12.0, current, reveal, false); } } } diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 8024c62c5..7c7acac4c 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -896,6 +896,48 @@ fn local_folder_tree_expands_and_browses() { h.settle(SETTLE); } +/// A folder listed twice in Local (Pictures as a place of its own and again inside Home) opens +/// and closes on each row by itself: both copies of its triangle used to share one egui id, so +/// the copy drawn later took the clicks of the one drawn first (issue #1093). +#[test] +fn local_folder_listed_twice_opens_and_closes_each_row() { + let base = std::env::temp_dir().join(format!("lc-ui-twice-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let inner = base.join("Inner"); + std::fs::create_dir_all(inner.join("Sub")).unwrap(); + let mut h = detail("panel.edit"); + exec(&mut h, "view.leftPanel", json!({"show": true})); + h.hide_home_above(&base); + let (base_s, inner_s) = (base.to_string_lossy().to_string(), inner.to_string_lossy().to_string()); + let sub = inner.join("Sub").to_string_lossy().to_string(); + exec(&mut h, "local.addRoot", json!({"path": base_s})); + exec(&mut h, "local.addRoot", json!({"path": inner_s})); + h.settle(SETTLE); + let r = h.request("ui.clickWidget", json!({"id": format!("folderToggle:{base_s}")}), T); + assert_eq!(r["ok"], true, "{r}"); + let toggle = format!("folderToggle:{inner_s}"); + h.step_until(SETTLE, |h| h.app.widgets.iter().filter(|(w, _)| *w == toggle).count() == 2); + let rows = |h: &Headless, id: &str| h.app.widgets.iter().filter(|(w, _)| w == id).count(); + assert_eq!(rows(&h, &toggle), 2, "Inner is listed inside the base folder and as a place of its own"); + let sub_row = format!("source:local:{sub}"); + assert_eq!(rows(&h, &sub_row), 0); + // each copy's triangle opens that copy only, and closes it again + for copy in 0..2 { + let at = h.app.widgets.iter().filter(|(w, _)| *w == toggle).nth(copy).map(|(_, r)| r.center()).unwrap(); + let r = h.request("ui.click", json!({"x": at.x, "y": at.y}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step_until(SETTLE, |h| h.app.widgets.iter().any(|(w, _)| *w == sub_row)); + assert_eq!(rows(&h, &sub_row), 1, "copy {copy} opens, the other stays closed"); + let at = h.app.widgets.iter().filter(|(w, _)| *w == toggle).nth(copy).map(|(_, r)| r.center()).unwrap(); + let r = h.request("ui.click", json!({"x": at.x, "y": at.y}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert_eq!(rows(&h, &sub_row), 0, "copy {copy} closes again"); + } + let _ = std::fs::remove_dir_all(&base); + h.settle(SETTLE); +} + #[test] fn slideshow_advances_pauses_and_ends() { let mut h = detail("panel.edit");