Merge PR #1101

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<T: Clone + Send + 'static>(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::<Option<String>>(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);
         }
     }
 }

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");