fix: a folder listed twice in Local can't be collapsed (#1093)

Local lists a folder once per place it is in: Pictures as a place of its
own and again inside Home. Each Local row keyed its open state and its
triangle's egui id by the folder's path alone, so once Home was open the
two Pictures triangles shared one id: the copy drawn last took the clicks
of the copy drawn first, and the first Pictures could not be collapsed.
The open state was shared too, so both copies opened and closed together.

The tree now keys both by the top-level place it hangs from as well, so
every row opens and closes by itself. Regression test: two Local places,
one inside the other, and a click on each copy's triangle.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Matthew James Sivak
2026-10-11 21:17:12 +02:00
co-authored by Claude Opus 5.5
parent 76b5377868
commit ed737afda0
2 changed files with 50 additions and 4 deletions
+8 -4
View File
@@ -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);
}
}
}
+42
View File
@@ -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");