From 35cdbb1ca8ecaad69702335c984a4debcf612432 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Mon, 12 Oct 2026 10:25:17 -0400 Subject: [PATCH 1/2] LR-PREF-GENERAL: the Settings window is one size on every tab, and nothing in it runs off the side General was 970 pt wide and the other tabs about 600, because a row of controls never wrapped and the widest row set the window's width; the window was also as tall as each tab's content, so it moved up and down between tabs and the tabs moved under the pointer. In a 900 pt wide app window the row labels and the Close button of General were off screen at the left, and a long library path made the window wider than the screen. - settings::window draws the tab bar, which stays put, above the page of the open tab, which scrolls: 560 pt wide and up to 640 pt tall whatever the page holds, less when the app window is smaller. Each page keeps a scroll position of its own, and one that goes on below shows its scroll handle without the pointer on it. It is still drawn in the scroll area every dialog has, which has nothing to scroll; in a window with under 120 pt for the page (far below the desktop minimum) that area scrolls tabs and page together, as before. - A row's controls wrap in the room beside the label, further lines starting under the first control, and a label longer than its column wraps in it (Forget unchanged photos pushed its choices 8 pt out of line). - Paths are one line, shortened from the front after either kind of separator, with all of the path in a tooltip only when it was cut: Library > Location, the folder Auto Import watches (it wrapped over several lines), the smart previews folder (it lost its end, and had two tooltips) and a monitor profile's file. row(), choices() and TABS keep their signatures, the match on the tab is where it was and the shared dialog code changes by one call, so the open pull requests that add tabs and rows still merge. Below 640 pt of app window a row holding a 240 pt field can still be wider than the page. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/dialogs.rs | 4 +- crates/ui-egui/src/panels/settings.rs | 178 +++++++++-- crates/ui-egui/src/tests_dialog_settings.rs | 314 ++++++++++++++++++++ docs/whats-new.md | 11 + 5 files changed, 487 insertions(+), 22 deletions(-) create mode 100644 crates/ui-egui/src/tests_dialog_settings.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 9a387c77..c43538a7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -62,6 +62,8 @@ mod tests_date_picker; #[cfg(test)] mod tests_dialog_fit; #[cfg(test)] +mod tests_dialog_settings; +#[cfg(test)] mod tests_dialogs; #[cfg(test)] mod tests_filmstrip; diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 72528fca..92fa69e9 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -962,7 +962,9 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { } Dialog::Merge { opts } => crate::merge::body(app, ui, opts), Dialog::Import { opts } => crate::import::body(app, ui, opts), - Dialog::Settings { tab } => crate::panels::settings::body(app, ui, tab), + // (Settings is never taller than `body_max`, so the scroll area round it never + // scrolls: its tab bar stays put and its pages scroll by themselves) + Dialog::Settings { tab } => crate::panels::settings::window(app, ui, tab, body_max), Dialog::SamModel { error, .. } => sam_model_body(app, ui, error.as_deref()), Dialog::KeywordTag { editing, diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 3c7261e2..fcd6c1e3 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -33,21 +33,10 @@ const CACHE_SIZES: [u32; 5] = [512, 1024, 2048, 4096, 8192]; const LABEL_W: f32 = 150.0; -/// The dialog body for `tab` (the tab bar switches `tab`). -pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String) { +/// The page of `tab` (the tab bar in [`window`] switches `tab`). +pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &str) { let t = Tokens::get(ui.ctx()); - ui.set_width((ui.ctx().content_rect().width() - 64.0).clamp(240.0, 560.0)); - ui.horizontal_wrapped(|ui| { - ui.spacing_mut().item_spacing.x = 4.0; - for (id, label) in TABS { - if crate::widgets::text_button(ui, &format!("settingsTab-{id}"), label, tab == id).clicked() { - *tab = id.to_string(); - } - } - }); - ui.separator(); - ui.add_space(2.0); - match tab.as_str() { + match tab { "import" => import_tab(app, ui, &t), "performance" => performance_tab(app, ui, &t), "display" => display_tab(app, ui, &t), @@ -58,6 +47,95 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String) { } } +/// The widest the Settings window's content gets, and the tallest its pages get: the window is +/// one size on every tab, so it stays where it is as the tabs are gone through. +const PAGE_SIZE: egui::Vec2 = egui::vec2(560.0, 640.0); + +/// The least room in which a page scrolls by itself. In a window with less (far below the +/// desktop minimum), the tabs and the page scroll together in the dialog's own scroll area. +const PAGE_MIN_HEIGHT: f32 = 120.0; + +/// The Settings window's content: the tab bar, which stays put, above the page of `tab` +/// ([`body`]), which scrolls when it is taller than the room for it, with a scroll handle that +/// shows without the pointer on it. `max_height` is the room the app window leaves for both: the +/// two together never take more, so the scroll area every dialog is drawn in has nothing to +/// scroll (but for a window with less than [`PAGE_MIN_HEIGHT`] for the page, where that area +/// scrolls the two together). Every page gets the same room whatever it holds, and each keeps a +/// scroll position of its own. Nothing here is wider than the window from 640 pt of app window +/// up: a row's controls wrap ([`row`]) and paths are shortened ([`path_label`]). +pub fn window(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String, max_height: f32) { + ui.set_width((ui.ctx().content_rect().width() - 64.0).clamp(240.0, PAGE_SIZE.x)); + let top = ui.cursor().top(); + ui.horizontal_wrapped(|ui| { + ui.spacing_mut().item_spacing = egui::vec2(4.0, 4.0); + for (id, label) in TABS { + if crate::widgets::text_button(ui, &format!("settingsTab-{id}"), label, tab == id).clicked() { + *tab = id.to_string(); + } + } + }); + ui.separator(); + // (a point less than the room left, so rounding never makes the dialog's own scroll area scroll) + let room = max_height - (ui.cursor().top() - top) - 1.0; + if room < PAGE_MIN_HEIGHT { + body(app, ui, tab.as_str()); + register(ui.ctx(), "settings:page", ui.min_rect()); + return; + } + let height = room.min(PAGE_SIZE.y); + // A page that goes on below shows its scroll handle all the time: without it, a page a line + // or two too long looks cut off, not scrollable. + ui.spacing_mut().scroll.dormant_handle_opacity = 0.6; + let page = egui::ScrollArea::vertical() + .id_salt(("settings-page", tab.as_str())) + .max_height(height) + .min_scrolled_height(height) + .auto_shrink([false, false]) + .show(ui, |ui| { + ui.add_space(2.0); + body(app, ui, tab.as_str()); + // (what the page took: wider than the room for it would mean a control is cut off) + register(ui.ctx(), "settings:page", ui.min_rect()); + }); + register(ui.ctx(), "settings:pageRoom", page.inner_rect); +} + +/// `path` shortened to fit `max` (as `width` measures it) by dropping leading folders, after +/// either kind of separator, so that its end stays (`…/Pictures/Library`, +/// `…\Pictures\Library`): the end says which folder it is. A last folder too long by itself +/// loses its end ([`crate::widgets::elide_head`]). +fn elide_path(path: &str, max: f32, width: impl Fn(&str) -> f32) -> String { + if width(path) <= max { + return path.to_string(); + } + // (a separator at the very end is not where a name starts: `…/` alone would say nothing) + let end = path.trim_end_matches(['/', '\\']).len(); + let mut rest = path; + while let Some(i) = rest.find(['/', '\\']).filter(|i| path.len() - rest.len() + i + 1 < end) { + let separator = if rest.get(i..).is_some_and(|r| r.starts_with('\\')) { '\\' } else { '/' }; + rest = rest.get(i + 1..).unwrap_or(""); + let shorter = format!("…{separator}{rest}"); + if width(&shorter) <= max { + return shorter; + } + } + let name = rest.trim_end_matches(['/', '\\']); + // (nothing but separators: never an empty label) + if name.is_empty() { "…".to_string() } else { crate::widgets::elide_head(name, max, width) } +} + +/// A folder or file path on one line in the room left in its row (`text:{id}`): shortened from +/// the front when it is too long ([`elide_path`]), with all of it in a tooltip then. +fn path_label(ui: &mut egui::Ui, id: &str, path: &str, font: egui::FontId, color: egui::Color32) { + let shown = elide_path(path, ui.available_width(), |s| ui.painter().layout_no_wrap(s.to_string(), font.clone(), color).size().x); + // (`truncate` is the last resort, for a room narrower than "…") + let r = ui.add(egui::Label::new(RichText::new(&shown).font(font).color(color)).truncate().show_tooltip_when_elided(false)); + register(ui.ctx(), format!("text:{id}"), r.rect); + if shown != path { + r.on_hover_text(path); + } +} + pub(super) fn heading(ui: &mut egui::Ui, t: &Tokens, text: &str) { ui.add_space(4.0); ui.label(RichText::new(crate::i18n::tr(text)).font(t.semibold(12.5)).color(t.text)); @@ -67,9 +145,17 @@ fn row(ui: &mut egui::Ui, t: &Tokens, label: &str, add: impl FnOnce(&mut egui ui.horizontal(|ui| { ui.allocate_ui_with_layout(egui::vec2(LABEL_W, 24.0), egui::Layout::left_to_right(egui::Align::Center), |ui| { ui.set_min_width(LABEL_W); - ui.label(RichText::new(crate::i18n::tr(label)).color(t.text_label)); + // (a label longer than its column wraps in it; it does not push the controls along) + ui.add(egui::Label::new(RichText::new(crate::i18n::tr(label)).color(t.text_label)).wrap()); }); - add(ui) + // The controls wrap in the room beside the label, each further line starting under the + // first control, so a row with many choices grows down and never widens the window. + let wrapped = egui::Layout::left_to_right(egui::Align::Center).with_main_wrap(true); + ui.allocate_ui_with_layout(egui::vec2(ui.available_width(), 24.0), wrapped, |ui| { + ui.spacing_mut().item_spacing.y = 4.0; + add(ui) + }) + .inner }) .inner } @@ -112,7 +198,7 @@ fn general_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { None => crate::i18n::tr("In memory — nothing is saved").to_string(), }; row(ui, t, crate::i18n::tr("Location"), |ui| { - ui.label(RichText::new(location).color(t.text)); + path_label(ui, "settingsLocation", &location, egui::TextStyle::Body.resolve(ui.style()), t.text); }); row(ui, t, "", |ui| { let can = app.services.pick_folder.is_some(); @@ -363,7 +449,10 @@ fn import_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { heading(ui, t, crate::i18n::tr("Auto Import")); hint(ui, t, crate::i18n::tr("Photos that arrive in this folder (tethering, a scanner, a sync app) are added as soon as they're complete.")); row(ui, t, crate::i18n::tr("Watched folder"), |ui| { - ui.label(RichText::new(d.auto_folder.clone().unwrap_or_else(|| crate::i18n::tr("Off").into())).color(t.text)); + match &d.auto_folder { + Some(folder) => path_label(ui, "settingsAutoFolder", folder, egui::TextStyle::Body.resolve(ui.style()), t.text), + None => drop(ui.label(RichText::new(crate::i18n::tr("Off")).color(t.text))), + } let can = app.services.pick_folder.is_some(); let r = ui.add_enabled(can, egui::Button::new(crate::i18n::tr("Choose…"))); register(ui.ctx(), "button:settingsAutoFolder", r.rect); @@ -493,8 +582,10 @@ fn smart_previews(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { let available = loc["available"].as_bool().unwrap_or(true); let custom = loc["custom"].as_bool().unwrap_or(false); row(ui, t, crate::i18n::tr("Folder"), |ui| { - let text = format!("{path}{}", if custom { "" } else { " (library default)" }); - ui.add(egui::Label::new(RichText::new(text.clone()).color(if available { t.text } else { t.reject })).truncate()).on_hover_text(text); + path_label(ui, "settingsSmartFolder", &path, egui::TextStyle::Body.resolve(ui.style()), if available { t.text } else { t.reject }); + if !custom { + ui.label(RichText::new("(library default)").color(t.text)); + } }); let (count, bytes) = (loc["count"].as_u64().unwrap_or(0), loc["bytes"].as_u64().unwrap_or(0)); if available { @@ -610,7 +701,7 @@ fn display_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { let info = d.describe(); if !screen { row(ui, t, crate::i18n::tr("File"), |ui| { - ui.label(RichText::new(&d.path).size(11.0).color(t.text_dim)); + path_label(ui, "settingsDisplayFile", &d.path, egui::FontId::proportional(11.0), t.text_dim); }); } row(ui, t, crate::i18n::tr("Type"), |ui| { @@ -1123,3 +1214,48 @@ pub fn display_profile(app: &mut LightcraftApp, p: &Value) -> Result json!({"path": null, "description": "sRGB (no display profile)"}), }) } + +#[cfg(test)] +mod path_tests { + use super::elide_path; + + /// Width = characters, so a limit of 20 is "20 characters". + fn fit(path: &str, max: usize) -> String { + elide_path(path, max as f32, |s| s.chars().count() as f32) + } + + #[test] + fn a_path_that_fits_is_left_alone() { + assert_eq!(fit("/Users/me/Pictures", 18), "/Users/me/Pictures"); + assert_eq!(fit("", 5), ""); + } + + #[test] + fn leading_folders_go_first_and_the_end_stays() { + assert_eq!(fit("/Users/me/Pictures/Library", 20), "…/Pictures/Library"); + assert_eq!(fit("/Users/me/Pictures/Library", 10), "…/Library"); + } + + /// Windows paths: `Path::display` gives backslashes there. + #[test] + fn a_path_with_backslashes_keeps_its_end_too() { + assert_eq!(fit(r"C:\Users\me\Pictures\Library", 20), r"…\Pictures\Library"); + assert_eq!(fit(r"C:\Users\me/Pictures\Library", 10), r"…\Library"); + } + + #[test] + fn a_last_folder_too_long_by_itself_loses_its_end() { + assert_eq!(fit("/Users/me/a-very-long-folder-name", 8), "a-very-…"); + assert_eq!(fit("no-separator-at-all", 8), "no-sepa…"); + assert_eq!(fit("/Users/Фотографии-из-отпуска", 8), "Фотогра…"); + } + + #[test] + fn a_separator_at_the_end_is_not_where_the_name_starts() { + assert_eq!(fit("/Users/me/Pictures/Library/", 11), "…/Library/"); + assert_eq!(fit("/Users/me/a-very-long-folder-name/", 8), "a-very-…"); + assert_eq!(fit(r"C:\Users\me\Library\", 11), r"…\Library\"); + assert_eq!(fit("///", 2), "…"); + assert_eq!(fit(r"\\server\share\folder", 15), r"…\share\folder"); + } +} diff --git a/crates/ui-egui/src/tests_dialog_settings.rs b/crates/ui-egui/src/tests_dialog_settings.rs new file mode 100644 index 00000000..b30770c1 --- /dev/null +++ b/crates/ui-egui/src/tests_dialog_settings.rs @@ -0,0 +1,314 @@ +//! The Settings window is one size in one place whatever tab is open, and lies inside the app +//! window: a row with more choices than fit wraps, a label longer than its column wraps, a long +//! folder path is cut short (its tooltip has all of it), and the tabs stay put while a page scrolls. +//! +//! Before, a wide row widened the whole window: General was 970 points wide and the other tabs +//! about 600, the window moved up and down between tabs, and in a 900 point wide app window the +//! row labels and the Close button of General were cut off at the left. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::panels::settings::TABS; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +/// The app window sizes tried: roomy, the desktop minimum, and smaller than that. +const SIZES: [[f32; 2]; 3] = [[1600.0, 1000.0], [900.0, 560.0], [640.0, 480.0]]; + +fn headless(session: lightcraft_engine::Session, size: [f32; 2]) -> Headless { + let mut h = Headless::new(LightcraftApp::new(session, Services::default()), size, 1.0); + // let the first renders finish (a render still running at exit can crash in GPU teardown) + h.settle(T); + h +} + +fn demo(size: [f32; 2]) -> Headless { + headless(lightcraft_engine::Session::with_demo(), size) +} + +/// Given Settings open on `tab`, drawn until its layout has settled. +fn show(h: &mut Headless, tab: &str) { + let r = h.request("engine.execute", json!({"command": "app.settings", "params": {"tab": tab}}), T); + assert_eq!(r["ok"], true, "app.settings {tab}: {r}"); + h.settle(T); + for _ in 0..4 { + h.step(); + } +} + +/// A folder under the system's temporary one, removed again when the test ends, passed or not. +struct TempFolder(std::path::PathBuf); + +impl TempFolder { + fn new(tag: &str) -> Self { + let dir = std::env::temp_dir().join(format!("lc-ui-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + Self(dir) + } +} + +impl Drop for TempFolder { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } +} + +fn rect(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("no widget {id}")) +} + +/// The controls the Settings window registers on the tab that is open (and its tab buttons). +fn controls(h: &Headless) -> Vec<(String, egui::Rect)> { + let named = |id: &str| id.split_once(':').is_some_and(|(_, name)| name.starts_with("settings") || name.starts_with("theme")); + h.app.widgets.iter().filter(|(id, _)| named(id)).cloned().collect() +} + +fn same(a: egui::Rect, b: egui::Rect) -> bool { + (a.min - b.min).abs().max_elem() < 0.5 && (a.max - b.max).abs().max_elem() < 0.5 +} + +#[test] +fn the_window_keeps_its_size_and_place_from_tab_to_tab() { + for size in SIZES { + let mut h = demo(size); + // Given the first tab, when every other tab is opened in turn + show(&mut h, "general"); + let first = rect(&h, "dialog:window"); + for (tab, _) in TABS { + show(&mut h, tab); + // Then the window has not moved or changed size + let now = rect(&h, "dialog:window"); + assert!(same(first, now), "{size:?}: Settings is {now:?} on {tab}, and was {first:?} on general"); + } + // And going back to the first tab finds it as it was + show(&mut h, "general"); + assert!(same(first, rect(&h, "dialog:window")), "{size:?}: general again"); + } +} + +#[test] +fn every_tab_keeps_its_controls_inside_the_window() { + for size in SIZES { + let mut h = demo(size); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, h.size); + for (tab, _) in TABS { + // When a tab is opened + show(&mut h, tab); + // Then the Settings window lies inside the app window + let window = rect(&h, "dialog:window"); + assert!(screen.contains_rect(window), "{size:?} {tab}: Settings {window:?} runs off the {screen:?} window"); + // And no control reaches past its left or right edge + // (the page is everything the tab drew, whether or not it has a name of its own; a + // control past the room for it would be clipped there, inside the window's margin) + let (page, room) = (rect(&h, "settings:page"), rect(&h, "settings:pageRoom")); + assert!( + page.left() >= room.left() - 0.5 && page.right() <= room.right() + 0.5, + "{size:?} {tab}: the page {page:?} is wider than the room for it {room:?}" + ); + assert!(window.contains_rect(room), "{size:?} {tab}: the page's room {room:?} is inside Settings {window:?}"); + for (id, r) in controls(&h) { + let within = if id.contains("settingsTab-") { window } else { room }; + assert!(r.left() >= within.left() - 0.5 && r.right() <= within.right() + 0.5, "{size:?} {tab}: {id} {r:?} is outside {within:?}"); + } + } + } +} + +#[test] +fn a_row_of_many_choices_wraps_under_itself() { + // Given an app window too narrow for the languages in one line + let mut h = demo([900.0, 560.0]); + show(&mut h, "general"); + let languages: Vec = (0..crate::i18n::Locale::ALL.len()).map(|i| rect(&h, &format!("button:settingsLanguage-{i}"))).collect(); + // Then they take more than one line + let first = languages[0]; + assert!(languages.iter().any(|r| r.top() > first.bottom()), "the languages wrap: {languages:?}"); + // And each further line starts under the first choice, not under the row's label + for r in languages.iter().filter(|r| r.top() > first.bottom()) { + assert!(r.left() >= first.left() - 0.5, "{r:?} starts left of the first language {first:?}"); + } + assert!( + languages.iter().filter(|r| r.top() > first.bottom()).any(|r| (r.left() - first.left()).abs() < 0.5), + "a wrapped line starts at the first choice" + ); +} + +#[test] +fn a_label_longer_than_its_column_leaves_the_controls_in_line() { + // Given Performance, whose "Forget unchanged photos" label is wider than the label column + let mut h = demo([1600.0, 1000.0]); + show(&mut h, "performance"); + // Then its choices start where the choices of a short label do + let (short, long) = (rect(&h, "button:settingsPreview-0"), rect(&h, "button:settingsForgetLocal-0")); + assert!((short.left() - long.left()).abs() < 0.5, "Preview size starts at {} and Forget unchanged photos at {}", short.left(), long.left()); +} + +#[test] +fn a_long_library_folder_is_cut_short_and_its_tooltip_has_all_of_it() { + // Given a library in a folder with a very long path + let base = TempFolder::new("settings-window"); + let dir = + base.0.join("photographs-from-2019-to-2026-kept-on-the-second-external-drive-that-nobody-wants-to-type-twice").join("the-master-library"); + std::fs::create_dir_all(&dir).unwrap(); + let mut session = lightcraft_engine::Session::new().with_fs(); + session.open_library(&dir, true).unwrap(); + let path = dir.display().to_string(); + + let mut h = headless(session, [1600.0, 1000.0]); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, h.size); + // When Settings opens on General, which shows where the library is + show(&mut h, "general"); + let general = rect(&h, "dialog:window"); + show(&mut h, "display"); + // Then the window is as wide as on a tab without a path + assert!(same(general, rect(&h, "dialog:window")), "General is {general:?} with a long path, Display is {:?}", rect(&h, "dialog:window")); + show(&mut h, "general"); + assert!(screen.contains_rect(general), "Settings {general:?} runs off the {screen:?} window"); + // And the path is cut short, inside the window + let location = rect(&h, "text:settingsLocation"); + let room = rect(&h, "settings:pageRoom"); + assert!(location.right() <= room.right() + 0.5, "the path {location:?} stays inside the room for the page {room:?}"); + let texts = h.painted_text(); + assert!(!texts.iter().any(|text| text.contains(&path)), "the whole path is not drawn in the row"); + // And what is left of it is its end, which says which library this is + let name = "the-master-library"; + assert!(path.ends_with(name)); + assert!( + texts.iter().any(|text| text.starts_with('…') && text.ends_with(name)), + "the row keeps the end of the path: {:?}", + texts.iter().filter(|t| t.contains("library")).collect::>() + ); + // And resting the pointer on it shows the whole path + assert_eq!(h.request("ui.hoverWidget", json!({"id": "text:settingsLocation"}), T)["ok"], true); + assert!(h.step_until(T, |h| h.painted_text().iter().any(|text| text.contains(&path))), "the tooltip has the whole path"); + + // And the same holds for the folder Auto Import watches, which stays on one line + let watched = h.app.run("library.autoImport", json!({"folder": path})); + assert!(watched.is_ok(), "{watched:?}"); + show(&mut h, "import"); + let (folder, room) = (rect(&h, "text:settingsAutoFolder"), rect(&h, "settings:pageRoom")); + assert!(folder.height() < 24.0 && folder.right() <= room.right(), "the watched folder {folder:?} is one line inside {room:?}"); + assert!(h.painted_text().iter().any(|text| text.starts_with('…') && text.ends_with(name)), "and keeps its end"); + drop(h); +} + +#[test] +fn a_short_path_is_shown_whole_and_has_no_tooltip() { + // Given the in-memory library, whose Location is a short sentence + let mut h = demo([1600.0, 1000.0]); + show(&mut h, "general"); + assert!(h.painted_text().iter().any(|text| text == "In memory — nothing is saved"), "it is all there"); + let before = h.painted_text().len(); + // When the pointer rests on it + assert_eq!(h.request("ui.hoverWidget", json!({"id": "text:settingsLocation"}), T)["ok"], true); + let _ = h.step_until(Duration::from_secs(2), |h| h.painted_text().len() > before); + // Then nothing more is drawn: there is nothing a tooltip could add + assert_eq!(h.painted_text().len(), before, "no tooltip for a path that is all there"); +} + +/// How many pixels of `rect` are not `colour` (one pixel per point here). +fn other_than(image: &egui::ColorImage, rect: egui::Rect, colour: egui::Color32) -> usize { + let [w, h] = image.size; + let (x0, x1) = (rect.left().round().max(0.0) as usize, (rect.right().round().max(0.0) as usize).min(w)); + let (y0, y1) = (rect.top().round().max(0.0) as usize, (rect.bottom().round().max(0.0) as usize).min(h)); + (y0..y1).flat_map(|y| (x0..x1).map(move |x| (x, y))).filter(|(x, y)| image.pixels[y * w + x] != colour).count() +} + +#[test] +fn a_page_that_goes_on_below_shows_a_scroll_handle_without_the_pointer_on_it() { + let mut h = demo([900.0, 560.0]); + let chrome = crate::theme::Tokens::get(&h.view.ctx).chrome; + // (the pointer is nowhere near the window) + h.request("ui.move", json!({"x": 5.0, "y": 5.0}), T); + let handle = |h: &Headless| { + let room = rect(h, "settings:pageRoom"); + other_than(&h.paint(), egui::Rect::from_min_max(egui::pos2(room.right() - 6.0, room.top()), room.max), chrome) + }; + // Given Import, which is taller than the room a 560 pt tall app window has for it + show(&mut h, "import"); + assert!(rect(&h, "settings:page").height() > rect(&h, "settings:pageRoom").height()); + // Then a handle is drawn at the page's right edge + assert!(handle(&h) > 40, "Import shows a scroll handle ({} px)", handle(&h)); + // And Display, which fits, has none + show(&mut h, "display"); + assert!(rect(&h, "settings:page").height() <= rect(&h, "settings:pageRoom").height()); + assert_eq!(handle(&h), 0, "Display has nothing to scroll"); +} + +#[test] +fn a_window_too_short_for_a_page_of_its_own_scrolls_tabs_and_page_together() { + // Given an app window far below the desktop minimum + let mut h = demo([900.0, 220.0]); + let screen = egui::Rect::from_min_size(egui::Pos2::ZERO, h.size); + show(&mut h, "import"); + let window = rect(&h, "dialog:window"); + assert!(screen.contains_rect(window), "Settings {window:?} is inside the {screen:?} window"); + let (tab, page) = (rect(&h, "button:settingsTab-import"), rect(&h, "check:settings.perCamera")); + // When it is scrolled + h.request("ui.move", json!({"x": window.center().x, "y": window.center().y}), T); + for _ in 0..3 { + h.request("ui.scroll", json!({"dy": -120.0}), T); + h.step(); + } + h.settle(T); + // Then the tabs and the page move as one, so every setting can still be reached + let (tab_now, page_now) = (rect(&h, "button:settingsTab-import"), rect(&h, "check:settings.perCamera")); + assert!(page_now.top() < page.top() - 50.0, "the page scrolled: {page:?} -> {page_now:?}"); + assert!(((tab.top() - tab_now.top()) - (page.top() - page_now.top())).abs() < 0.5, "tabs and page moved together"); + assert!(same(window, rect(&h, "dialog:window")), "the window did not move"); +} + +#[test] +fn the_tabs_stay_put_while_a_page_scrolls() { + // Given a page taller than the window has room for + let mut h = demo([900.0, 560.0]); + show(&mut h, "import"); + let window = rect(&h, "dialog:window"); + let tabs: Vec = TABS.iter().map(|(id, _)| rect(&h, &format!("button:settingsTab-{id}"))).collect(); + let page_before = rect(&h, "check:settings.perCamera"); + // When the page is scrolled + h.request("ui.move", json!({"x": window.center().x, "y": window.center().y}), T); + for _ in 0..3 { + h.request("ui.scroll", json!({"dy": -120.0}), T); + h.step(); + } + h.settle(T); + // Then the page moved + let page_after = rect(&h, "check:settings.perCamera"); + assert!(page_after.top() < page_before.top() - 50.0, "the page scrolled: {page_before:?} -> {page_after:?}"); + // And the tabs and the window did not + for ((id, _), before) in TABS.iter().zip(tabs) { + assert!(same(before, rect(&h, &format!("button:settingsTab-{id}"))), "tab {id} moved"); + } + assert!(same(window, rect(&h, "dialog:window")), "the window moved"); +} + +#[test] +fn each_tab_remembers_how_far_it_was_scrolled() { + let mut h = demo([900.0, 560.0]); + show(&mut h, "interface"); + let fresh = rect(&h, "button:settingsAppearance-0"); + // Given Import scrolled down + show(&mut h, "import"); + let window = rect(&h, "dialog:window"); + let top = rect(&h, "check:settings.perCamera").top(); + h.request("ui.move", json!({"x": window.center().x, "y": window.center().y}), T); + for _ in 0..3 { + h.request("ui.scroll", json!({"dy": -120.0}), T); + h.step(); + } + h.settle(T); + let scrolled = rect(&h, "check:settings.perCamera").top(); + assert!(scrolled < top - 50.0); + // When another tab is opened, it starts at its top + show(&mut h, "interface"); + let mode = rect(&h, "button:settingsAppearance-0"); + assert!(same(fresh, mode), "Interface is at its top, as when first opened: {fresh:?} -> {mode:?}"); + // And Import is where it was left + show(&mut h, "import"); + assert!((rect(&h, "check:settings.perCamera").top() - scrolled).abs() < 0.5, "Import is where it was left"); +} diff --git a/docs/whats-new.md b/docs/whats-new.md index fb8b285a..ca59b871 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -22,6 +22,17 @@ scene) now develop from their sensor data with every editing tool, like the camera's other raws. Photos imported earlier switch over with Photo ▸ Reload from Disk. +### Settings window +- Settings is one size on every tab and stays where it is as you go through them; before, each tab made its own + size and the tabs moved under the pointer. The tabs stay in place while a long page scrolls, a page that goes on + below shows a scroll handle without the pointer on it, and each page remembers where it was scrolled to. +- Nothing in Settings runs off the side any more. A row with more choices than fit (the languages on General) wraps + onto further lines and a long label wraps in its column, where both used to widen the window: in a small + LightCraft window the labels and the Close button of General ended up off screen. +- A long folder (General ▸ Library ▸ Location, the folder Auto Import watches, the smart previews folder, a monitor + profile's file) is shortened to its last folders ("…/Pictures/Library") on one line; rest the pointer on it for + the whole path. + ### Switching libraries - File ▸ New Library… asks for a name, then where to put it, and opens the new library (issue #902). - File ▸ Open Recent Library lists the last 8 libraries; ones that are gone (moved, drive disconnected) are greyed From 99e58473b20f9fe51a031633bcbe43eba25bb711 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Mon, 12 Oct 2026 12:16:15 -0400 Subject: [PATCH 2/2] LR-PREF-GENERAL: the long-path test scrolls to the watched folder before reading it The row is at the foot of Import. With more sections above it (another pull request adds one) it is below the fold of the 640 pt page, and a row that is not drawn has no text to read: the test failed for the page's length, not for the path. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/tests_dialog_settings.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/ui-egui/src/tests_dialog_settings.rs b/crates/ui-egui/src/tests_dialog_settings.rs index b30770c1..ed1ee4e9 100644 --- a/crates/ui-egui/src/tests_dialog_settings.rs +++ b/crates/ui-egui/src/tests_dialog_settings.rs @@ -190,7 +190,16 @@ fn a_long_library_folder_is_cut_short_and_its_tooltip_has_all_of_it() { let watched = h.app.run("library.autoImport", json!({"folder": path})); assert!(watched.is_ok(), "{watched:?}"); show(&mut h, "import"); + // (the row is at the foot of the page: scrolled to, so that it is drawn however long the page is) + let window = rect(&h, "dialog:window"); + h.request("ui.move", json!({"x": window.center().x, "y": window.center().y}), T); + for _ in 0..6 { + h.request("ui.scroll", json!({"dy": -400.0}), T); + h.step(); + } + h.settle(T); let (folder, room) = (rect(&h, "text:settingsAutoFolder"), rect(&h, "settings:pageRoom")); + assert!(room.intersects(folder), "the watched folder {folder:?} is in view in {room:?}"); assert!(folder.height() < 24.0 && folder.right() <= room.right(), "the watched folder {folder:?} is one line inside {room:?}"); assert!(h.painted_text().iter().any(|text| text.starts_with('…') && text.ends_with(name)), "and keeps its end"); drop(h);