From 71be070a2fde1fb7ca3db80253eb612ad8756295 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 23 Oct 2026 07:29:16 -0400 Subject: [PATCH] LR-PREF-GENERAL: Esc in a Settings text box gives the text back and stays in Settings --- crates/ui-egui/src/dialogs/settings.rs | 15 +- .../ui-egui/src/dialogs/settings/general.rs | 22 +-- crates/ui-egui/src/dialogs/settings/import.rs | 22 ++- .../ui-egui/src/dialogs/settings/widgets.rs | 10 ++ crates/ui-egui/src/tests.rs | 1 + .../ui-egui/src/tests/dialog_settings_text.rs | 146 ++++++++++++++++++ docs/whats-new.md | 6 + 7 files changed, 200 insertions(+), 22 deletions(-) create mode 100644 crates/ui-egui/src/tests/dialog_settings_text.rs diff --git a/crates/ui-egui/src/dialogs/settings.rs b/crates/ui-egui/src/dialogs/settings.rs index 7b264c811..271b6e7c9 100644 --- a/crates/ui-egui/src/dialogs/settings.rs +++ b/crates/ui-egui/src/dialogs/settings.rs @@ -106,11 +106,16 @@ const PREVIEW_CHROME: f32 = 82.0; 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(); + // The page drawn is the one that was open when the frame began: a tab clicked now opens on + // the next frame, so a text box on the page being left is drawn once more and saves what was + // typed in it (the click took the focus from it). + let shown = tab.clone(); 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() { + if crate::widgets::text_button(ui, &format!("settingsTab-{id}"), label, shown == *id).clicked() { *tab = id.to_string(); + ui.ctx().request_repaint(); } } }); @@ -118,7 +123,7 @@ pub fn window(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String, max_ // (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()); + body(app, ui, shown.as_str()); register(ui, "settings:page", ui.min_rect()); return; } @@ -129,7 +134,7 @@ pub fn window(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String, max_ // (at least the least tall slide); where that leaves less than [`PAGE_MIN_HEIGHT`] for the // page, it scrolls with the page instead (as in the short window above). let mut pinned = false; - if tab == "slideshow" { + if shown == "slideshow" { let slide_h = (height * PINNED_PREVIEW_SHARE - PREVIEW_CHROME).max(crate::panels::slide_layout::PREVIEW_MIN_H); if height - (slide_h + PREVIEW_CHROME) >= PAGE_MIN_HEIGHT { let before = ui.cursor().top(); @@ -144,7 +149,7 @@ pub fn window(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String, max_ // 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())) + .id_salt(("settings-page", shown.as_str())) .max_height(height) .min_scrolled_height(height) .auto_shrink([false, false]) @@ -153,7 +158,7 @@ pub fn window(app: &mut LightcraftApp, ui: &mut egui::Ui, tab: &mut String, max_ if pinned { slideshow_tab(app, ui, &Tokens::get(ui.ctx()), false); } else { - body(app, ui, tab.as_str()); + body(app, ui, shown.as_str()); } // (what the page took: wider than the room for it would mean a control is cut off) register(ui, "settings:page", ui.min_rect()); diff --git a/crates/ui-egui/src/dialogs/settings/general.rs b/crates/ui-egui/src/dialogs/settings/general.rs index 85866b5fa..c4fb8b754 100644 --- a/crates/ui-egui/src/dialogs/settings/general.rs +++ b/crates/ui-egui/src/dialogs/settings/general.rs @@ -1,4 +1,4 @@ -use super::widgets::{check, choices, heading, hint, path_label, row}; +use super::widgets::{check, choices, field, heading, hint, path_label, row}; use super::{LightcraftApp, RichText, StartupView, Tokens, json, register}; // ------------------------------------------------------------------------------------- General @@ -77,10 +77,12 @@ pub(super) fn general_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens check(ui, "settings.closeDialogsOutside", &mut app.ui.settings.close_dialogs_outside, "Close a dialog by clicking outside it"); heading(ui, t, crate::i18n::tr("External Editing")); row(ui, t, crate::i18n::tr("PhotoCraft"), |ui| { - crate::text_field::TextField::singleline("field:photocraftPath", &mut app.ui.settings.photocraft_path) - .hint(crate::i18n::tr("Found automatically")) - .width(220.0) - .show(ui); + field( + ui, + crate::text_field::TextField::singleline("field:photocraftPath", &mut app.ui.settings.photocraft_path) + .hint(crate::i18n::tr("Found automatically")) + .width(220.0), + ); }); edit_file_rows(ui, t, "photocraftFile", &mut app.ui.settings.photocraft_file); hint( @@ -91,10 +93,12 @@ pub(super) fn general_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens ), ); row(ui, t, crate::i18n::tr("Other editor"), |ui| { - crate::text_field::TextField::singleline("field:externalEditor", &mut app.ui.settings.external_editor) - .hint(crate::i18n::tr("System default")) - .width(220.0) - .show(ui); + field( + ui, + crate::text_field::TextField::singleline("field:externalEditor", &mut app.ui.settings.external_editor) + .hint(crate::i18n::tr("System default")) + .width(220.0), + ); }); edit_file_rows(ui, t, "externalFile", &mut app.ui.settings.external_file); hint(ui, t, crate::i18n::tr("Edit in Other Editor (⌥⌘E) opens the copy in this application (an app name on macOS, a program path elsewhere).")); diff --git a/crates/ui-egui/src/dialogs/settings/import.rs b/crates/ui-egui/src/dialogs/settings/import.rs index 97f34d6ab..685ac1073 100644 --- a/crates/ui-egui/src/dialogs/settings/import.rs +++ b/crates/ui-egui/src/dialogs/settings/import.rs @@ -1,5 +1,5 @@ use super::general::{cameras, preset_combo}; -use super::widgets::{button_width, check, choices, heading, hint, path_label, row}; +use super::widgets::{button_width, check, choices, field, heading, hint, path_label, row}; use super::{LightcraftApp, RichText, Tokens, json, register}; pub(super) fn import_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { @@ -79,12 +79,14 @@ pub(super) fn import_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) row(ui, t, label, |ui| { let id = egui::Id::unique(("settingsMeta", key)); let mut text: String = ui.data(|m| m.get_temp(id)).unwrap_or(value.clone()); - let r = ui.add(egui::TextEdit::singleline(&mut text).hint_text(hint_text).desired_width(240.0)); - register(ui, format!("field:settings.{key}"), r.rect); - if r.lost_focus() && text.trim() != value { + // (the project's text field: Esc gives the text back and stays in Settings, and with + // the text back the line below has nothing to save) + let widget = format!("field:settings.{key}"); + let shown = field(ui, crate::text_field::TextField::singleline(&widget, &mut text).hint(hint_text).width(240.0)); + if shown.committed() && text.trim() != value { let _ = app.run("library.preferences", json!({"import": {key: text.trim()}})); } - if r.has_focus() { + if shown.editing { ui.data_mut(|m| m.insert_temp(id, text)); } else { ui.data_mut(|m| m.remove::(id)); @@ -171,10 +173,14 @@ pub(super) fn import_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) } }); row(ui, t, crate::i18n::tr("Album"), |ui| { - let id = egui::Id::unique("auto-album"); + // (per library: a name left here by one library is not another's) + let id = egui::Id::unique(("auto-album", app.session.library.as_ref().map(|l| l.dir.clone()))); let mut name = ui.data(|m| m.get_temp::(id)).unwrap_or_else(|| d.auto_album.clone().unwrap_or_default()); - let r = ui.add(egui::TextEdit::singleline(&mut name).hint_text(crate::i18n::tr("None")).desired_width(180.0)); - if r.lost_focus() { + let shown = field( + ui, + crate::text_field::TextField::singleline("field:settingsAutoAlbum", &mut name).hint(crate::i18n::tr("None")).width(180.0), + ); + if shown.committed() { let _ = app.run("library.autoImport", json!({"album": name.trim()})); } ui.data_mut(|m| m.insert_temp(id, name)); diff --git a/crates/ui-egui/src/dialogs/settings/widgets.rs b/crates/ui-egui/src/dialogs/settings/widgets.rs index 7607b1251..afd66068e 100644 --- a/crates/ui-egui/src/dialogs/settings/widgets.rs +++ b/crates/ui-egui/src/dialogs/settings/widgets.rs @@ -49,6 +49,16 @@ pub(super) fn button_width(ui: &egui::Ui, text: &str) -> f32 { label_width(ui, text, egui::TextStyle::Button) + 2.0 * ui.spacing().button_padding.x } +/// A text box of the Settings window, shown: Esc in it gives its text back and is used up by +/// that, so Settings stays open (a second Esc, with nothing being edited, closes it). +pub(super) fn field(ui: &mut egui::Ui, field: crate::text_field::TextField<'_>) -> crate::text_field::TextFieldResponse { + let shown = field.show(ui); + if shown.cancelled() { + crate::widgets::take_escape(ui.ctx()); + } + shown +} + pub(crate) 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)); diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 3a102a29a..f98e03cae 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -17,6 +17,7 @@ mod date_picker; mod dialog_fit; mod dialog_primary; mod dialog_settings; +mod dialog_settings_text; mod dialog_tab; mod dialogs; mod docking_acceptance; diff --git a/crates/ui-egui/src/tests/dialog_settings_text.rs b/crates/ui-egui/src/tests/dialog_settings_text.rs new file mode 100644 index 000000000..fca63500a --- /dev/null +++ b/crates/ui-egui/src/tests/dialog_settings_text.rs @@ -0,0 +1,146 @@ +//! Esc in a text box of Settings gives the text back and leaves Settings open, and text typed in a +//! box is saved when another tab is clicked. +//! +//! Before: Esc in Copyright or Creator saved what had been typed and closed the window, and text +//! typed there was dropped when another tab was clicked. + +use std::time::Duration; + +use serde_json::{Value, json}; + +use crate::headless::Headless; +use crate::state::Dialog; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +fn demo() -> Headless { + let mut h = Headless::new(LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services::default()), [1200.0, 900.0], 1.0); + // let the first renders finish (a render still running at exit can crash in GPU teardown) + h.settle(T); + h +} + +fn steps(h: &mut Headless) { + for _ in 0..4 { + h.step(); + } +} + +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); + steps(h); +} + +fn click(h: &mut Headless, id: &str) { + let r = h.request("ui.clickWidget", json!({"id": id}), T); + assert_eq!(r["ok"], true, "click {id}: {r}"); + steps(h); +} + +fn key(h: &mut Headless, key: &str) { + assert_eq!(h.request("ui.key", json!({"key": key}), T)["ok"], true, "{key}"); + steps(h); +} + +fn says(h: &Headless, part: &str) -> bool { + h.painted_text().iter().any(|text| text.contains(part)) +} + +fn settings_open(h: &Headless, tab: &str) -> bool { + h.app.ui.dialog == Some(Dialog::Settings { tab: tab.into() }) +} + +fn copyright(h: &mut Headless) -> Value { + h.app.run("library.preferences", json!({})).unwrap()["import"]["copyright"].clone() +} + +#[test] +fn esc_in_copyright_gives_the_text_back_and_leaves_settings_open() { + let mut h = demo(); + show(&mut h, "import"); + let before = copyright(&mut h); + // Given a name typed into Copyright + click(&mut h, "field:settings.copyright"); + h.request("ui.text", json!({"text": "Example Name"}), T); + steps(&mut h); + assert!(says(&h, "Example Name"), "the typing shows"); + // When Esc is pressed + key(&mut h, "Escape"); + // Then the name is gone again, nothing was saved, and Settings is still open + assert!(!says(&h, "Example Name"), "Esc gives the text back"); + assert_eq!(copyright(&mut h), before, "nothing was saved"); + assert!(settings_open(&h, "import"), "Settings stays open: {:?}", h.app.ui.dialog); + // And a second Esc, with no edit under way, closes Settings as it always did + key(&mut h, "Escape"); + assert!(h.app.ui.dialog.is_none()); +} + +#[test] +fn return_in_copyright_saves_the_text() { + let mut h = demo(); + show(&mut h, "import"); + click(&mut h, "field:settings.copyright"); + h.request("ui.text", json!({"text": "Example Name"}), T); + steps(&mut h); + key(&mut h, "Enter"); + assert_eq!(copyright(&mut h), json!("Example Name")); + assert!(settings_open(&h, "import")); +} + +#[test] +fn text_typed_in_copyright_is_saved_when_another_tab_is_opened() { + let mut h = demo(); + show(&mut h, "import"); + click(&mut h, "field:settings.copyright"); + h.request("ui.text", json!({"text": "Example Name"}), T); + steps(&mut h); + // When another tab is clicked with the text still being typed + click(&mut h, "button:settingsTab-general"); + // Then that tab opens and the text was saved, as leaving the box any other way saves it + assert!(settings_open(&h, "general")); + assert_eq!(copyright(&mut h), json!("Example Name")); +} + +#[test] +fn esc_in_an_external_editing_box_leaves_settings_open_too() { + let mut h = demo(); + show(&mut h, "general"); + // (External Editing is far down the General page: scroll it into view) + assert_eq!(h.request("ui.hoverWidget", json!({"id": "settings:pageRoom"}), T)["ok"], true); + h.request("ui.scroll", json!({"dy": -3000.0}), T); + steps(&mut h); + let before = h.app.ui.settings.external_editor.clone(); + click(&mut h, "field:externalEditor"); + h.request("ui.text", json!({"text": "Example Editor"}), T); + steps(&mut h); + key(&mut h, "Escape"); + assert_eq!(h.app.ui.settings.external_editor, before, "Esc gives the text back"); + assert!(settings_open(&h, "general"), "and Settings stays open: {:?}", h.app.ui.dialog); +} + +#[test] +fn using_the_edit_menu_does_not_save_a_half_typed_text() { + let mut h = demo(); + show(&mut h, "import"); + let before = copyright(&mut h); + click(&mut h, "field:settings.copyright"); + h.request("ui.text", json!({"text": "Example Name"}), T); + steps(&mut h); + // When the field's Cut / Copy / Paste menu is opened, which takes the focus off the box + let r = h.request("ui.clickWidget", json!({"id": "field:settings.copyright", "button": "right"}), T); + assert_eq!(r["ok"], true, "right click: {r}"); + steps(&mut h); + assert!(h.app.widgets.iter().any(|(w, _)| w == "field:settings.copyright:copy"), "the menu is open"); + // Then the text typed so far is still there, and nothing was saved + assert!(says(&h, "Example Name"), "the typing stays in the box"); + assert_eq!(copyright(&mut h), before, "an unfinished edit is not saved"); + // And Esc closes only the menu: the edit goes on, and Return then saves it + key(&mut h, "Escape"); + assert_eq!(copyright(&mut h), before, "closing the menu does not end the edit"); + assert!(settings_open(&h, "import"), "Esc closed the menu only"); + key(&mut h, "Enter"); + assert_eq!(copyright(&mut h), json!("Example Name")); +} diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..65d63cc8e 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -21,6 +21,12 @@ ## 2026-10-23 +### Text boxes in Settings +- Esc in a text box of Settings (Copyright, Creator, the Auto Import album, the two External Editing boxes) gives + the text back and stays in Settings; in the first three it used to save what was typed, and in all of them it + closed the window. Those three now have the Cut / Copy / Paste menu too, and what is typed in Copyright or + Creator is saved when another tab is clicked, where it used to be dropped. + ### Tab leaves the name field in New Album, New Smart Album and the name prompts - In New Album, New Smart Album, Save Metadata Preset and the other dialogs with one name field, the field has the keyboard focus when the dialog opens, and Tab now moves on to the check box, Cancel and OK (Shift+Tab goes back).