UI: don't reveal a hidden Local folder inside Home's tree
Hiding a kept Local folder beneath Home while browsing it (Remove from Local, local.hide) expanded Home down to that folder and pushed the "Show hidden locations" row out of view. local_places picked the innermost listed folder containing the browsed one as the tree to reveal; once the kept row was hidden that became Home, so folder_tree opened Home, AppData, Local, Temp, ... to show the hidden folder again. A tree no longer opens on the way down through a hidden folder: the owner is chosen only among places whose path to the browsed folder passes no hidden one. Hiding Home itself still lets Pictures reveal a folder browsed inside it. local_location_can_be_hidden_and_restored again hides a browsed temporary folder (beneath Home on Windows), kept as Browse Folder... does, while it is still browsed, and checks the restore row is inside the viewport before clicking it. It failed before this fix. New unit test: hidden_folder_is_not_revealed_in_an_outer_tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
dbc05bedc7
commit
c7f5664ce2
|
|
@ -826,38 +826,55 @@ mod tests {
|
|||
}
|
||||
|
||||
/// Local: Remove from Local hides a sidebar location (nothing on disk changes) and the
|
||||
/// "Show hidden locations" row puts it back.
|
||||
/// "Show hidden locations" row puts it back. The folder is browsed and kept the way Browse
|
||||
/// Folder… does it, in the temporary folder — on Windows beneath Home, whose tree must not
|
||||
/// open down to the hidden folder and push the restore row out of view.
|
||||
#[test]
|
||||
fn local_location_can_be_hidden_and_restored() {
|
||||
let mut h = demo([1300.0, 900.0]);
|
||||
let size = [1300.0, 900.0];
|
||||
let mut h = demo(size);
|
||||
let t = Duration::from_secs(10);
|
||||
let dir = std::env::temp_dir().join(format!("lc-ui-hide-{}", std::process::id()));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let path = dir.to_string_lossy().to_string();
|
||||
let ids = |h: &mut Headless| -> Vec<String> {
|
||||
let rects = |h: &mut Headless| -> Vec<(String, Vec<f64>)> {
|
||||
let w = h.request("ui.widgets", json!({"filter": "source:local:"}), t);
|
||||
w["result"].as_array().unwrap().iter().filter_map(|x| x["id"].as_str().map(String::from)).collect()
|
||||
w["result"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|x| (x["id"].as_str().unwrap().to_string(), x["rect"].as_array().unwrap().iter().filter_map(|v| v.as_f64()).collect()))
|
||||
.collect()
|
||||
};
|
||||
let ids = |h: &mut Headless| -> Vec<String> { rects(h).into_iter().map(|(id, _)| id).collect() };
|
||||
h.request("ui.set", json!({"leftPanel": true}), t);
|
||||
// Keep the fixture as a root rather than depending on its location
|
||||
// relative to the machine's built-in Home folder.
|
||||
h.request("engine.execute", json!({"command": "local.addRoot", "params": {"path": path}}), t);
|
||||
h.request("engine.execute", json!({"command": "library.browse", "params": {"path": path}}), t);
|
||||
// Browse Folder… browses the picked folder and keeps it in Local within one frame; a
|
||||
// request runs frames, so keep it first (no frame sees it browsed but not yet kept)
|
||||
let r = h.request("engine.execute", json!({"command": "local.addRoot", "params": {"path": path}}), t);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
let r = h.request("engine.execute", json!({"command": "library.browse", "params": {"path": path}}), t);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.settle(SETTLE);
|
||||
assert!(ids(&mut h).contains(&format!("source:local:{path}")));
|
||||
assert!(h.wait_for_widget(&format!("source:local:{path}"), t), "the browsed folder is listed: {:?}", ids(&mut h));
|
||||
assert!(!ids(&mut h).iter().any(|i| i == "source:local:restoreHidden"));
|
||||
// Test sidebar visibility independently of browsing: hiding a folder
|
||||
// beneath Home otherwise expands the user's entire parent tree and
|
||||
// pushes the restore button outside the viewport.
|
||||
assert_eq!(h.request("ui.clickWidget", json!({"id": "source:all"}), t)["ok"], true);
|
||||
h.settle(SETTLE);
|
||||
// hide it while it is still being browsed
|
||||
let r = h.request("engine.execute", json!({"command": "local.hide", "params": {"path": path}}), t);
|
||||
assert_eq!(r["ok"], true, "{r}");
|
||||
h.settle(SETTLE);
|
||||
let after = ids(&mut h);
|
||||
assert!(!after.contains(&format!("source:local:{path}")), "{after:?}");
|
||||
assert!(after.iter().any(|i| i == "source:local:restoreHidden"), "{after:?}");
|
||||
// give any listing a reveal would wait for time to arrive
|
||||
for _ in 0..20 {
|
||||
h.step();
|
||||
std::thread::sleep(Duration::from_millis(5));
|
||||
}
|
||||
h.settle(SETTLE);
|
||||
let after = rects(&mut h);
|
||||
let after_ids: Vec<&str> = after.iter().map(|(id, _)| id.as_str()).collect();
|
||||
assert!(!after_ids.contains(&format!("source:local:{path}").as_str()), "{after_ids:?}");
|
||||
let restore = after.iter().find(|(id, _)| id == "source:local:restoreHidden").map(|(_, r)| r.clone());
|
||||
let restore = restore.unwrap_or_else(|| panic!("no restore row: {after_ids:?}"));
|
||||
assert!(restore[1] >= 0.0 && restore[1] + restore[3] <= f64::from(size[1]), "the restore row is in view: {restore:?} {after_ids:?}");
|
||||
assert!(h.app.session.browse.is_some(), "hiding does not stop browsing");
|
||||
assert!(dir.is_dir(), "the folder itself is untouched");
|
||||
assert_eq!(h.request("ui.clickWidget", json!({"id": "source:local:restoreHidden"}), t)["ok"], true);
|
||||
h.settle(SETTLE);
|
||||
|
|
|
|||
|
|
@ -254,7 +254,7 @@ pub(crate) struct LocalPlaces {
|
|||
/// (label, path) of each top-level folder, in order.
|
||||
pub places: Vec<(String, String)>,
|
||||
/// The top-level folder the browsed folder lies in (the innermost one): its tree opens on
|
||||
/// the way down to it.
|
||||
/// the way down to it. None when that way passes through a hidden folder.
|
||||
pub owner: Option<usize>,
|
||||
/// A folder listed only for this session because the browsed folder is in no saved
|
||||
/// location (browsed from a breadcrumb, the CLI…); it stays while browsing below it.
|
||||
|
|
@ -299,7 +299,15 @@ pub(crate) fn local_places(
|
|||
}
|
||||
}
|
||||
if let Some(c) = browsing {
|
||||
out.owner = places.iter().enumerate().filter(|(_, (_, p))| folder_within(c, p)).max_by_key(|(_, (_, p))| folder_key(p).len()).map(|(i, _)| i);
|
||||
// A tree never opens on the way down through a hidden folder: hiding a kept folder
|
||||
// beneath Home would otherwise reveal it again inside Home's (possibly huge) tree.
|
||||
let through_hidden = |p: &str| hidden.iter().any(|h| folder_within(c, h) && folder_within(h, p));
|
||||
out.owner = places
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(_, (_, p))| folder_within(c, p) && !through_hidden(p))
|
||||
.max_by_key(|(_, (_, p))| folder_key(p).len())
|
||||
.map(|(i, _)| i);
|
||||
}
|
||||
out.places = places;
|
||||
out
|
||||
|
|
@ -803,4 +811,22 @@ mod tests {
|
|||
let l = local_places(Vec::new(), &[], Some("/t/base"), None, &["/t/base/".into()]);
|
||||
assert!(l.places.is_empty() && l.browse_root.is_none());
|
||||
}
|
||||
|
||||
/// A hidden folder inside a listed one (a kept folder beneath Home) is not revealed in that
|
||||
/// one's tree while it is browsed, nor are the folders below it; hiding Home itself still
|
||||
/// lets Pictures open down to a folder browsed inside it.
|
||||
#[test]
|
||||
fn hidden_folder_is_not_revealed_in_an_outer_tree() {
|
||||
let builtin = || vec![("Pictures".to_string(), "/home/example/Pictures".to_string()), ("Home".to_string(), "/home/example".to_string())];
|
||||
let kept = ["/home/example/AppData/Temp/lc".to_string()];
|
||||
let l = local_places(builtin(), &kept, Some("/home/example/AppData/Temp/lc"), None, &[]);
|
||||
assert_eq!(l.owner, Some(2), "shown as its own kept row");
|
||||
for browsing in ["/home/example/AppData/Temp/lc", "/home/example/AppData/Temp/lc/Day 1"] {
|
||||
let l = local_places(builtin(), &kept, Some(browsing), None, &["/home/example/AppData/Temp/lc/".into()]);
|
||||
assert_eq!(names(&l.places), ["Pictures", "Home"], "{browsing}");
|
||||
assert_eq!((l.owner, l.browse_root.as_deref()), (None, None), "Home does not open down to {browsing}");
|
||||
}
|
||||
let l = local_places(builtin(), &[], Some("/home/example/Pictures/Trip"), None, &["/home/example".into()]);
|
||||
assert_eq!((names(&l.places), l.owner), (vec!["Pictures"], Some(0)));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue