diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index edaf1ec63..b9e94bed3 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -81,6 +81,8 @@ mod tests_dialog_fit; #[cfg(test)] mod tests_dialog_primary; #[cfg(test)] +mod tests_dialog_tab; +#[cfg(test)] mod tests_dialogs; #[cfg(test)] mod tests_edit_layout; diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 894ad2bd5..c24f6da7c 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -2244,13 +2244,15 @@ fn person_rename_plan(cat: &lightcraft_catalog::Catalog, from: &str, to: &str) - PersonRenamePlan { faces, needs_merge: needs_merge && from_key != to_key, block } } -/// A dialog's one text field: it keeps the focus (except while its own menu has it) and opens -/// with its text selected, so typing replaces it. True when Return confirms the dialog; Esc -/// cancels the dialog (`show`). Taking the focus back with a bare `TextEdit` every frame hid the -/// focus Return gives up, and Return did nothing. +/// A dialog's one text field: it takes the focus when the dialog opens, and opens with its text +/// selected, so typing replaces it. It takes the focus back only while nothing else has it: taking +/// it back every frame would keep Tab from ever reaching the dialog's buttons and check boxes. +/// True when Return confirms the dialog; Esc cancels the dialog (`show`). fn dialog_field(ui: &mut egui::Ui, widget: &str, text: &mut String, hint: &str) -> bool { let r = crate::text_field::TextField::singleline(widget, text).hint(crate::i18n::tr(hint)).width(f32::INFINITY).select_on_focus(true).show(ui); - if !r.editing { + // (and takes it back when nothing has it: after a click on the dialog's background or a check box, + // or when Return gave it up on a refusal; a control Tab reached keeps it) + if just_opened(ui, widget) || (!r.editing && r.ending.is_none() && ui.memory(|m| m.focused().is_none())) { r.response.request_focus(); } r.ending == Some(crate::text_field::Ending::Return) diff --git a/crates/ui-egui/src/tests_dialog_tab.rs b/crates/ui-egui/src/tests_dialog_tab.rs new file mode 100644 index 000000000..5ffb99580 --- /dev/null +++ b/crates/ui-egui/src/tests_dialog_tab.rs @@ -0,0 +1,153 @@ +//! A dialog's name field takes the keyboard focus once, when the dialog opens: Tab then carries it +//! on to the dialog's other controls (a check box, Cancel, OK) and Shift+Tab back, instead of the +//! field taking it back every frame. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::Dialog; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +fn demo_with_selection() -> Headless { + let mut session = lightcraft_engine::Session::with_demo(); + session.selection.ids = session.catalog.photos().take(2).map(|p| p.id).collect(); + let mut h = Headless::new(LightcraftApp::new(session, Services::default()), [1400.0, 900.0], 1.0); + h.settle(T); + h +} + +/// The registered widget (`field:…`, `check:…`, `button:…`) that has the keyboard focus. +fn focused(h: &Headless) -> Option { + let id = h.view.ctx.memory(|m| m.focused())?; + let rect = h.view.ctx.read_response(id)?.rect; + h.app.widgets.iter().find(|(_, r)| *r == rect).map(|(w, _)| w.clone()) +} + +fn key(h: &mut Headless, params: serde_json::Value) { + let r = h.request("ui.key", params, T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); +} + +/// Open `dialog`, then Tab through it; the focus order, starting with the field it opens on. +fn tab_order(dialog: Dialog) -> Vec { + let mut h = demo_with_selection(); + h.app.ui.dialog = Some(dialog); + for _ in 0..4 { + h.step(); + } + let mut order = vec![focused(&h).unwrap_or_default()]; + for _ in 0..4 { + key(&mut h, json!({"key": "Tab"})); + order.push(focused(&h).unwrap_or_default()); + } + order +} + +#[test] +fn new_album_opens_on_its_name_and_tab_reaches_the_check_box_cancel_and_ok() { + let order = tab_order(Dialog::NewAlbum { name: "Trip".into(), folder: false, parent: None, include_selected: true }); + assert_eq!(order[..4], ["field:albumName", "check:albumIncludeSelected", "button:dialogCancel", "button:dialogOk"], "{order:?}"); +} + +#[test] +fn new_smart_album_opens_on_its_name_and_tab_reaches_cancel_and_ok() { + let order = tab_order(Dialog::NewSmartAlbum { name: "Rejected".into(), parent: None }); + let (cancel, ok) = (order.iter().position(|w| w == "button:dialogCancel"), order.iter().position(|w| w == "button:dialogOk")); + assert_eq!(order[0], "field:smartAlbumName", "{order:?}"); + assert!(cancel.is_some() && ok.is_some() && cancel < ok, "Cancel then OK are reached by Tab: {order:?}"); +} + +#[test] +fn save_metadata_preset_opens_on_its_name_and_tab_reaches_cancel_and_ok() { + let prompt = Dialog::TextPrompt { + title: "Save Metadata Preset".into(), + hint: "Preset name".into(), + value: String::new(), + command: "metadata.savePreset".into(), + params: json!({}), + key: "name".into(), + }; + let order = tab_order(prompt); + assert_eq!(order[..3], ["field:prompt", "button:dialogCancel", "button:dialogOk"], "{order:?}"); +} + +#[test] +fn shift_tab_goes_back_from_ok_to_the_name() { + let mut h = demo_with_selection(); + h.app.ui.dialog = Some(Dialog::NewAlbum { name: "Trip".into(), folder: false, parent: None, include_selected: true }); + for _ in 0..4 { + h.step(); + } + for _ in 0..3 { + key(&mut h, json!({"key": "Tab"})); + } + assert_eq!(focused(&h).as_deref(), Some("button:dialogOk")); + for _ in 0..3 { + key(&mut h, json!({"key": "Tab", "shift": true})); + } + assert_eq!(focused(&h).as_deref(), Some("field:albumName")); +} + +#[test] +fn the_name_still_takes_typing_and_enter_after_opening() { + let mut h = demo_with_selection(); + h.app.ui.dialog = Some(Dialog::NewAlbum { name: String::new(), folder: false, parent: None, include_selected: false }); + for _ in 0..4 { + h.step(); + } + let r = h.request("ui.text", json!({"text": "Holiday"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + key(&mut h, json!({"key": "Enter"})); + h.settle(T); + assert!(h.app.ui.dialog.is_none(), "Enter confirmed the dialog"); + assert!(h.app.session.catalog.albums().any(|a| a.name == "Holiday"), "the typed name made the album"); +} + +#[test] +fn escape_still_cancels_after_tabbing_away_from_the_name() { + let mut h = demo_with_selection(); + h.app.ui.dialog = Some(Dialog::NewAlbum { name: "Trip".into(), folder: false, parent: None, include_selected: true }); + for _ in 0..4 { + h.step(); + } + key(&mut h, json!({"key": "Tab"})); + key(&mut h, json!({"key": "Escape"})); + assert!(h.app.ui.dialog.is_none(), "Esc closed the dialog"); +} + +#[test] +fn a_click_on_a_check_box_leaves_return_confirming_the_name() { + let mut h = demo_with_selection(); + h.app.ui.dialog = Some(Dialog::NewAlbum { name: "Trip".into(), folder: false, parent: None, include_selected: true }); + for _ in 0..4 { + h.step(); + } + let r = h.request("ui.clickWidget", json!({"id": "check:albumIncludeSelected"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); + key(&mut h, json!({"key": "Enter"})); + h.settle(T); + assert!(h.app.ui.dialog.is_none(), "Enter confirmed the dialog after the click"); + assert!(h.app.session.catalog.albums().any(|a| a.name == "Trip")); +} + +#[test] +fn a_refused_return_leaves_the_name_focused_for_the_correction() { + let mut h = demo_with_selection(); + h.app.ui.dialog = Some(Dialog::NewAlbum { name: String::new(), folder: false, parent: None, include_selected: false }); + for _ in 0..4 { + h.step(); + } + key(&mut h, json!({"key": "Enter"})); + h.step(); + assert!(h.app.ui.dialog.is_some(), "an empty name is refused and the dialog stays"); + assert_eq!(focused(&h).as_deref(), Some("field:albumName"), "the field has the focus again"); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index cf01f3444..8cec4b9ac 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -479,7 +479,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-BEHAV-TOOLTIPS | Tooltips with shortcuts | P0 | ✅ | `crates/ui-egui/src/panels/bottombar.rs` | | | LR-BEHAV-TEXTFIELD | Text fields behave alike | P1 | 🟡 | `crates/ui-egui/src/text_field.rs`, `crates/ui-egui/src/panels/topbar.rs`, `crates/ui-egui/src/widgets.rs`, `crates/ui-egui/src/panels/right.rs` (`meta_field`), `crates/ui-egui/src/panels/dialogs.rs`, `crates/ui-egui/src/panels/unnamed.rs` | one shared field: right-click Cut / Copy / Paste / Select All (no Paste in the browser), Esc gives back the text from before the edit, Return / leaving / Esc reported to the caller, optional select-on-focus; agents use `ui.clipboard` (MCP `clipboard`) and `ui.inspect` → `copied`. Used by Search Photos, slider values, the Info panel's metadata fields (multi-line captions), the one-field dialogs (New / Rename Album, New Smart Album, Rename / Merge Keywords, text prompts) and the unnamed faces' name field so far; the other text fields (most dialogs, import, settings, masking, keywords …) still use egui's own and have no menu | | LR-BEHAV-HEADLESS | Headless UI snapshots | P2 | ✅ | `apps/lightcraft-cli/src/main.rs`, `crates/ui-egui/src/headless.rs`, `docs/control-protocol.md` | `snapshot` rasterizes the UI and defaults photo development to the CPU before the first frame, without adapter discovery; scaled dimensions are validated before allocation, and UI zoom preserves the requested physical viewport; JSON-lines control scripts are supported | -| LR-BEHAV-ACCESS | Accessibility | P2 | 🟡 | `crates/ui-egui/src/widgets.rs`, `crates/ui-egui/src/text_field.rs`, `crates/ui-egui/src/panels/grid.rs`, `crates/ui-egui/tests/it/accessibility.rs` | AccessKit (VoiceOver / Narrator / AT-SPI): sliders announce control, value, range and step and take Increment / Decrement, buttons / icon buttons / dropdowns / section headers / sources their labels and state, text fields their caption, grid thumbnails file name, rating, flag and label. `tests/it/accessibility.rs` fails on an unnamed control, a slider without range or an untitled dialog in every view, Develop tool and 13 dialogs, and checks that Crop and Remove sliders are reached with Tab and that Tab walks the right panels top to bottom. Canvas tools by keyboard (`crates/ui-egui/src/canvas_keys.rs`, `crates/ui-egui/src/tests_canvas_keys.rs`): crop corners, edges and box, spot targets and sources, a New spot point (Return adds), radial / linear gradient handles and mask pins take the focus, are announced with their position and move with the arrows (Shift: further). Brush strokes and red-eye ellipses are pointer-only; not tried with a real screen reader ([audit](ui-parity.md#accessibility-audit)) | +| LR-BEHAV-ACCESS | Accessibility | P2 | 🟡 | `crates/ui-egui/src/widgets.rs`, `crates/ui-egui/src/text_field.rs`, `crates/ui-egui/src/panels/grid.rs`, `crates/ui-egui/tests/it/accessibility.rs` | AccessKit (VoiceOver / Narrator / AT-SPI): sliders announce control, value, range and step and take Increment / Decrement, buttons / icon buttons / dropdowns / section headers / sources their labels and state, text fields their caption, grid thumbnails file name, rating, flag and label. `tests/it/accessibility.rs` fails on an unnamed control, a slider without range or an untitled dialog in every view, Develop tool and 13 dialogs, and checks that Crop and Remove sliders are reached with Tab and that Tab walks the right panels top to bottom. Canvas tools by keyboard (`crates/ui-egui/src/canvas_keys.rs`, `crates/ui-egui/src/tests_canvas_keys.rs`): crop corners, edges and box, spot targets and sources, a New spot point (Return adds), radial / linear gradient handles and mask pins take the focus, are announced with their position and move with the arrows (Shift: further). Dialogs with one name field (New Album, New Smart Album, Save Metadata Preset, the renames) take the focus once when they open, so Tab and Shift+Tab reach their check box, Cancel and OK, and Return and Esc still confirm and cancel (`crates/ui-egui/src/tests_dialog_tab.rs`). Brush strokes and red-eye ellipses are pointer-only; not tried with a real screen reader ([audit](ui-parity.md#accessibility-audit)) | | LR-BEHAV-LOCALE | Language options | P2 | ✅ | `cmd:app.language.english`, `cmd:app.language.simplifiedChinese`, `cmd:app.language.traditionalChinese`, `cmd:app.language.japanese`, `cmd:app.language.portuguese`, `cmd:app.language.french`, `cmd:app.language.spanish`, `cmd:app.language.german`, `cmd:app.language.russian`, `cmd:app.language.ukrainian`, `cmd:app.language.georgian` | Edit > Language lists every language in the table and the active one is checked; shared with Settings > General (docs/localization.md) | | LR-BEHAV-LOCALIZE | Localisation | P2 | 🟡 | `crates/ui-egui/src/i18n.rs`, `crates/ui-egui/locales/`, `docs/localization.md`, `docs/localization-zh-hans.md`, `docs/localization-zh-hant.md`, `docs/localization-ja.md`, `docs/localization-pt-br.md`, `docs/localization-es.md`, `docs/localization-de.md`, `docs/localization-ru.md`, `docs/localization-ka.md` | One table entry per language (code, endonym, ISO 15924 script, catalog): English, Simplified Chinese, Traditional Chinese (Taiwan), Japanese, Brazilian Portuguese, Spanish, German, Russian, Ukrainian and Georgian UI with a persisted language preference and craft-fonts regular/bold faces per script (a variable face, as for Arabic, is drawn heavier for bold from its one file); menus, edit, crop/masks, settings, import/export and primary progress messages. Adding a language registers the locale and its language command, plus two catalogs. Stock presets/profiles, history steps, library headings, date headings and capture times are localised (user-named items stay verbatim); the Ukrainian catalog covers every existing catalog key, command/control label and rule-editor label; the German catalog reports newly added gaps; the other catalogs fall back to English for messages they lack. Technical errors remain English; release notes are translated in German and Ukrainian; craft-fonts faces for other scripts (Thai, Arabic, …) are UI fallbacks in every language, for file, folder and album names (#736); Traditional Chinese borrows the Simplified Chinese face (no TC face in craft-fonts yet), and the web build embeds the Japanese, Thai and Georgian faces in WASM while loading the Simplified Chinese face from a separate font file. | | LR-BEHAV-LEARN | Tutorials | OOS | 🚫 | | | @@ -928,3 +928,4 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-19 | minor | LRC-LIB-LIBFOLDERS: the sidebar trees indent by a narrower step (14 / 12 / 10 / 8 pt) when the deepest open row would not fit the panel, instead of cutting its name or scrolling sideways at once (issue #2298); status unchanged (✅) | | 2026-10-15 | patch | LR-EXP-DIM: Width & Height (`fit: widthHeight`) limits the width and the height as the photo stands, beside Dimensions (W × H, either orientation), as in Lightroom (#1620); status unchanged (✅) | | 2026-10-22 | minor | MENU-BAR: on macOS the native menu gives a key back to the window while a text field has focus or a shortcut is being recorded (muda's `set_accelerator(None)` does nothing there, so the menu item is now swapped for one without a key, and the key is put back on that item afterwards); status unchanged (✅) | +| 2026-10-23 | patch | LR-BEHAV-ACCESS: the name field of New Album, New Smart Album, Rename and the other one-field dialogs takes the keyboard focus once when the dialog opens instead of every frame, so Tab and Shift+Tab reach the check box, Cancel and OK; status unchanged (🟡) | diff --git a/docs/whats-new.md b/docs/whats-new.md index 999d5d22f..252a834a5 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -15,6 +15,14 @@ key could still run the menu command instead of reaching the field or the editor. The keys come back on the menu items when the field loses focus or recording ends; the menu's order, checkmarks and enabled state are unchanged. +## 2026-10-23 + +### 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). + Before, the field took the focus back every frame, so only Return and Esc got you out of it. Return still + confirms and Esc still cancels. + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges