From a2a1505953673eae380571aff2c112f3d682bd31 Mon Sep 17 00:00:00 2001 From: Ketbome Date: Thu, 15 Oct 2026 13:24:52 -0300 Subject: [PATCH 1/2] LR-BEHAV-DIALOGS: optionally close a dialog by clicking outside it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings → Dialogs → "Close a dialog by clicking outside it" (off by default, so a stray click never throws away a dialog's choices). When on, a click pressed and released on the dimmed backdrop closes the dialog like Esc and doesn't reach the panels below. A drag that ends outside (a slider pulled past the edge), a click that only closes a dropdown, and the click that opened the dialog keep it open. The import review never closes this way. The press and release are read from the raw pointer events: egui clears the press origin on release, and a scripted click is a single frame. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/locales/es.json | 2 ++ crates/ui-egui/src/panels/dialogs.rs | 31 +++++++++++++++++++++++++++ crates/ui-egui/src/panels/settings.rs | 2 ++ crates/ui-egui/src/state.rs | 4 ++++ crates/ui-egui/src/tests_dialogs.rs | 31 +++++++++++++++++++++++++++ docs/parity-checklist.md | 2 +- 6 files changed, 71 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/locales/es.json b/crates/ui-egui/locales/es.json index 77ba4c08e..6c6816273 100644 --- a/crates/ui-egui/locales/es.json +++ b/crates/ui-egui/locales/es.json @@ -292,6 +292,8 @@ "Another library was opened since the photos were dropped: nothing was moved": "Se abrió otra biblioteca después de soltar las fotos: no se movió nada", "Settings → Confirm before moving photos to another folder on disk turns it back on": "Ajustes → Confirmar antes de mover fotos a otra carpeta del disco lo vuelve a activar", "Confirm before moving photos to another folder on disk": "Confirmar antes de mover fotos a otra carpeta del disco", + "Dialogs": "Diálogos", + "Close a dialog by clicking outside it": "Cerrar un diálogo al hacer clic fuera de él", "Constrain Crop": "Restringir recorte", "Contact Sheet PDF": "PDF de hoja de contactos", "Continue": "Continuar", diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 5562e94d1..c15e126bf 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -1434,6 +1434,12 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { if settings && content_w > widest + 0.5 { ctx.data_mut(|d| d.insert_temp(widest_id, content_w)); } + if let Some(w) = &shown { + // (the import review keeps its hand-picked photos however the click lands) + if app.ui.settings.close_dialogs_outside && !import && clicked_backdrop(ctx, w.response.rect) { + close = true; + } + } if let Some(w) = shown { ctx.memory_mut(|m| m.set_modal_layer(w.response.layer_id)); if let Some(body_h) = body_h { @@ -1512,6 +1518,31 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { app.ui.dialog = if close { None } else { Some(dlg) }; } +/// A click on the dimmed backdrop around the dialog: it closes the dialog like Esc. Pressed and +/// released outside the window, so a slider dragged past its edge doesn't close it; a press while a +/// popup is open (a dropdown, a date picker's calendar) only closes the popup. Only a press made +/// while the dialog is shown counts, so the click that opened it never closes it. (Read from the +/// raw events: egui forgets the press origin on release, and a scripted click is one frame.) +fn clicked_backdrop(ctx: &egui::Context, window: egui::Rect) -> bool { + let press_id = egui::Id::new("dialog-backdrop-press"); + let (press, release, clicked) = ctx.input(|i| { + let at = |down: bool| { + i.events.iter().rev().find_map(|e| match e { + egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, .. } if *pressed == down => Some(*pos), + _ => None, + }) + }; + (at(true), at(false), i.pointer.primary_clicked()) + }); + if let Some(p) = press { + let outside = !window.contains(p) && !egui::Popup::is_any_open(ctx); + ctx.data_mut(|d| d.insert_temp(press_id, outside)); + } + let Some(release) = release else { return false }; + let pressed_outside = ctx.data_mut(|d| d.remove_temp::(press_id)).unwrap_or(false); + clicked && pressed_outside && !window.contains(release) +} + /// Room kept free above and below a dialog (#781). const DIALOG_SCREEN_MARGIN: f32 = 12.0; /// A dialog's title bar, frame and button row before the first frame measures them. diff --git a/crates/ui-egui/src/panels/settings.rs b/crates/ui-egui/src/panels/settings.rs index 682e2acb2..9e6426782 100644 --- a/crates/ui-egui/src/panels/settings.rs +++ b/crates/ui-egui/src/panels/settings.rs @@ -150,6 +150,8 @@ fn general_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { check(ui, "settings.autoAdvance", &mut app.ui.auto_advance, "Auto Advance: move to the next photo after rating or flagging"); check(ui, "settings.confirmDelete", &mut app.ui.settings.confirm_delete, "Confirm before moving photos to Recently Deleted"); check(ui, "settings.askBeforeMove", &mut app.ui.settings.ask_before_move, "Confirm before moving photos to another folder on disk"); + heading(ui, t, crate::i18n::tr("Dialogs")); + 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) diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index f8c8cc0d4..4d623a1b1 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -346,6 +346,9 @@ pub struct AppSettings { /// Ask before photos dropped on a Folders row move there on disk (the dialog's "Don't ask /// again" turns it off). pub ask_before_move: bool, + /// A click on the dimmed backdrop around a dialog closes it, like Esc (off: a stray click + /// never throws away a dialog's choices). + pub close_dialogs_outside: bool, /// GPU rendering allowed (`app.gpu`). pub gpu: bool, /// Largest long edge (pixels) the user lets the loupe render at; 0 = Automatic (the size it is @@ -401,6 +404,7 @@ impl Default for AppSettings { startup_view: StartupView::Last, confirm_delete: true, ask_before_move: true, + close_dialogs_outside: false, gpu: true, preview_limit: 0, external_editor: String::new(), diff --git a/crates/ui-egui/src/tests_dialogs.rs b/crates/ui-egui/src/tests_dialogs.rs index 6581640ec..f08eb62b3 100644 --- a/crates/ui-egui/src/tests_dialogs.rs +++ b/crates/ui-egui/src/tests_dialogs.rs @@ -85,6 +85,37 @@ fn modal_dialogs_block_background_shortcuts_and_keep_escape() { assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, 5, "closing restores shortcuts"); } +/// Settings → Close a dialog by clicking outside it: a click on the backdrop closes the dialog +/// like Esc, without reaching the panels below; a click inside it, or a drag that ends outside, keeps it. +#[test] +fn a_click_outside_closes_the_dialog_when_settings_allow_it() { + let mut h = demo([1400.0, 1000.0], json!({"view": "detail", "right": "edit"})); + let active = h.app.session.active().unwrap(); + let rating = h.app.session.catalog.photo(active).unwrap().rating; + h.app.run("dialog.copySettings", json!({})).unwrap(); + h.step(); + // off by default: the background click is blocked and the dialog stays + let r = h.request("ui.clickWidget", json!({"id": "star:bottom:5"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_some(), "a stray click keeps the dialog unless Settings say otherwise"); + h.app.ui.settings.close_dialogs_outside = true; + let r = h.request("ui.clickWidget", json!({"id": "copyGroup:none"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_some(), "a click inside the dialog keeps it"); + let window = h.app.widgets.iter().find(|(id, _)| id == "dialog:window").map(|(_, r)| *r).unwrap(); + let r = h.request("ui.drag", json!({"x": window.center().x, "y": window.center().y, "toX": 10.0, "toY": 10.0}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_some(), "a drag that ends outside the dialog keeps it"); + let r = h.request("ui.clickWidget", json!({"id": "star:bottom:5"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_none(), "a click on the backdrop closes the dialog"); + assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, rating, "the click doesn't reach the panel below"); +} + #[test] fn modal_dialogs_close_background_menus() { let mut h = demo([1400.0, 1000.0], json!({"view": "detail", "right": "edit"})); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 95dcd2aef..5c1732789 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -468,7 +468,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-BEHAV-RENDER-FIDELITY | Rendering matches Lightroom | P1 | 🟡 | `crates/pipeline/src/lib.rs`, `crates/pipeline/src/tone.rs` | every slider exists and works, but the character of the result (tone curve shape, highlight recovery, texture / clarity / dehaze, noise reduction, sharpening, default look) is tuned by eye; there is no systematic side-by-side comparison against Lightroom on the same CC0 raws. Needs a fidelity suite: Lightroom reference renders kept only in the local `plan/` (never committed), compared per slider and preset with a perceptual metric. Tuning that changes how existing settings render ships as a new process version (LRC-DEV-PROCESS, `docs/process-versions.md`) | | LR-BEHAV-DRAGDROP | Drag and drop | P1 | ✅ | `crates/ui-egui/src/lib.rs`, `crates/ui-egui/src/panels/grid.rs` (`drag_feedback`), `crates/ui-egui/src/panels/left.rs` (`drop_target`, `album_drag_over`, `top_level_drop_target`, `drag_auto_scroll`) | files → app (import); grid photos → an album row (adds the selection, with a count badge while dragging); sidebar album / folder rows → a folder (moves it; its name follows the pointer); a drag of albums or grid photos near the top or bottom edge of the sidebar scrolls it (faster nearer the edge, only while the pointer is over the sidebar) | | LR-BEHAV-TOAST | Toast notifications | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` | | -| LR-BEHAV-DIALOGS | Dialogs fit the window | P1 | ✅ | `crates/ui-egui/src/panels/dialogs.rs` (`dialog_body`, `list_height`), `crates/ui-egui/src/tests_dialog_fit.rs` | a dialog taller than the window (Export on a 1366×768 screen, issue #781) never runs off it: its options scroll while the title bar and the Cancel / OK row stay on screen; lists that scroll on their own (What's New, All Metadata, shortcuts, smart-album rules, credits) get shorter first; dialogs that fit look as before; Import keeps its own resizable, scrolling window | +| LR-BEHAV-DIALOGS | Dialogs fit the window | P1 | ✅ | `crates/ui-egui/src/panels/dialogs.rs` (`dialog_body`, `list_height`), `crates/ui-egui/src/tests_dialog_fit.rs`, `crates/ui-egui/src/tests_dialogs.rs` (`a_click_outside_closes_the_dialog_when_settings_allow_it`) | a dialog taller than the window (Export on a 1366×768 screen, issue #781) never runs off it: its options scroll while the title bar and the Cancel / OK row stay on screen; lists that scroll on their own (What's New, All Metadata, shortcuts, smart-album rules, credits) get shorter first; dialogs that fit look as before; Import keeps its own resizable, scrolling window; Settings → Dialogs → Close a dialog by clicking outside it (off by default, so a stray click never throws away a dialog's choices): a click pressed and released on the dimmed backdrop closes the dialog like Esc and doesn't reach the panels below, while a drag that ends outside (a slider pulled past the edge) or a click that closes a dropdown keeps it; the import review never closes this way (`clicked_backdrop`) | | LR-BEHAV-SIDEBAR-COLLAPSE | Collapsible left-sidebar sections | P2 | ✅ | `crates/ui-egui/src/panels/left.rs` (`section`, `section_header`), `crates/ui-egui/src/sidebar.rs` (`SidebarLayout`), `crates/ui-egui/src/tests_panels.rs` | click the Albums, Local, By Date, Folders or Keywords header (or drive `ui.clickWidget` `sidebarSection:`) to fold or unfold the section; a chevron before the title shows the state; the choice is kept across restarts (`sidebar` in the UI state; the `collapsedSidebar` list of older settings files is still read). the click itself is UI-only, like the By Date / Keywords disclosure triangles, and `cmd:view.sidebarSection` `{section, collapsed}` folds and unfolds for agents; photos dropped on a folded Albums section have no target | | LR-BEHAV-SIDEBAR-SECTIONS | Left-sidebar sections share one frame and one arrangement | P2 | ✅ | `crates/ui-egui/src/sidebar.rs` (`SidebarSection`, `SidebarLayout`), `crates/ui-egui/src/panels/left.rs` (`section`, `section_header`, `tree_toggle`), `crates/ui-egui/src/tests_panels.rs` | Albums, Local, By Date, Folders and Keywords are drawn by one `section` frame (the same gap above, header, fold and accessibility name) from one arrangement, `sidebar` in the UI state: a list of `{id, hidden, collapsed}` top to bottom, also settable with `ui.set`. Whatever is read, every section is in it exactly once (unknown and repeated entries are dropped, the rest follow in the usual order), so a damaged or newer settings file never loses the other settings. Rows that open (album folders, Local folders, dates, library folders, keywords) share `tree_toggle`: the same triangle, the same remembered open state, announced as Expand / Collapse. A section with nothing to list (Folders before anything is imported, Keywords before any is added) stays, open, with a one-line note in place of rows (`sidebarEmpty:`) instead of disappearing. `ui.set` reads `sidebar` strictly (`SidebarLayout::parse`): an unknown or repeated section, or a flag that is not true / false, is refused and changes nothing; sections left out follow in the usual order, shown and open; the older `collapsedSidebar` list still folds and unfolds. A section may have one button at the end of its header, declared in one place (`header_button`: an icon, a tooltip, and either an action or a menu): the frame draws it at the visible edge in the header and its pinned copy, keeps its click from folding the section and leaves it room beside a folded section's count. Albums has + (`icon:albumNew`: Create Album / Smart Album / Folder…), Keywords has + (`icon:keywordNew`: Create Keyword Tag, for a keyword inside the default parent, not inside what the Keyword List has picked; once made, Keywords is shown and opened down to it). My Photos (All Photos, Recently Added, Picks, Missing Photos, Recently Deleted) is not a section: it stays on top | | LR-BEHAV-SIDEBAR-HIDE | Show and hide left-sidebar sections | P2 | ✅ | `cmd:view.sidebarSection`, `cmd:view.sidebarReset`, `crates/ui-egui/src/sidebar.rs`, `crates/ui-egui/src/panels/left.rs` (`sections_menu`), `crates/ui-egui/src/menubar.rs`, `crates/ui-egui/src/tests_panels.rs` | View ▸ Sidebar Sections lists Albums, Local, By Date, Folders and Keywords in the sidebar's order, each checked while shown; unchecking one takes its header and rows off the sidebar, checking it puts it back where and how it was (still folded if it was). The same list opens with a right-click on any section header (with Hide This Section first; widgets `sidebarMenu:hide`, `sidebarMenu:section:`, `sidebarMenu:reset`) and on the My Photos title (`sidebarTitle`), which is always there, so hidden sections can always come back. Reset Sidebar Sections (enabled once something changed) shows and opens every section in the usual order. `view.sidebarSection` `{section, show?, collapsed?}` flips shown / hidden when neither flag is given and refuses an unknown section or a flag that is not true / false. Hiding the section of what the grid shows (an album, a keyword filter) leaves the grid as it is; an album made while Albums is hidden or folded brings Albums back, open. Kept with the UI state (`sidebar`). My Photos itself can't be hidden | From 4a6c43ce06f0c33d6f27c19f918325242f83ef7f Mon Sep 17 00:00:00 2001 From: Ketbome Date: Tue, 20 Oct 2026 13:07:20 -0300 Subject: [PATCH 2/2] LR-BEHAV-DIALOGS: the click that opens a dialog never closes it A quick click on a slow frame presses and releases in the frame where the panels open a dialog, and the backdrop check read that press as one on the backdrop, closing the dialog at once. Input now counts only once the dialog was already shown the frame before; a pending press is dropped otherwise. The test opens Copy Settings from the bottom-bar gear with press and release in one frame and checks it stays open, then that the next backdrop click closes it. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/dialogs.rs | 19 +++++++++++++++++-- crates/ui-egui/src/tests_dialogs.rs | 12 ++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index c15e126bf..a53da854c 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -1515,16 +1515,31 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { if app.ui.dialog.as_ref().is_some_and(|now| *now != at_start) { return; } + if !close { + let frame = ctx.cumulative_frame_nr(); + ctx.data_mut(|d| d.insert_temp(egui::Id::new(BACKDROP_SHOWN), frame)); + } app.ui.dialog = if close { None } else { Some(dlg) }; } +/// The last frame a dialog was drawn and stayed open, for [`clicked_backdrop`]. +const BACKDROP_SHOWN: &str = "dialog-backdrop-shown"; + /// A click on the dimmed backdrop around the dialog: it closes the dialog like Esc. Pressed and /// released outside the window, so a slider dragged past its edge doesn't close it; a press while a /// popup is open (a dropdown, a date picker's calendar) only closes the popup. Only a press made -/// while the dialog is shown counts, so the click that opened it never closes it. (Read from the -/// raw events: egui forgets the press origin on release, and a scripted click is one frame.) +/// while the dialog is shown counts: in the frame a dialog appears, its input is the click that +/// opened it (a quick click on a slow frame presses and releases in that one frame), so it never +/// closes it. (Read from the raw events: egui forgets the press origin on release.) fn clicked_backdrop(ctx: &egui::Context, window: egui::Rect) -> bool { let press_id = egui::Id::new("dialog-backdrop-press"); + let frame = ctx.cumulative_frame_nr(); + // the same frame too: a second pass of it gets no input + let shown = ctx.data(|d| d.get_temp::(egui::Id::new(BACKDROP_SHOWN))).is_some_and(|f| f + 1 >= frame); + if !shown { + ctx.data_mut(|d| d.remove_temp::(press_id)); + return false; + } let (press, release, clicked) = ctx.input(|i| { let at = |down: bool| { i.events.iter().rev().find_map(|e| match e { diff --git a/crates/ui-egui/src/tests_dialogs.rs b/crates/ui-egui/src/tests_dialogs.rs index f08eb62b3..aefe4863b 100644 --- a/crates/ui-egui/src/tests_dialogs.rs +++ b/crates/ui-egui/src/tests_dialogs.rs @@ -114,6 +114,18 @@ fn a_click_outside_closes_the_dialog_when_settings_allow_it() { h.step(); assert!(h.app.ui.dialog.is_none(), "a click on the backdrop closes the dialog"); assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, rating, "the click doesn't reach the panel below"); + // the click that opens a dialog lands outside its window; a quick one on a slow frame arrives + // pressed and released in that one frame + let gear = h.app.widgets.iter().find(|(id, _)| id == "icon:copyGear").map(|(_, r)| r.center()).unwrap(); + let press = |pressed| egui::Event::PointerButton { pos: gear, button: egui::PointerButton::Primary, pressed, modifiers: egui::Modifiers::NONE }; + h.events.extend([egui::Event::PointerMoved(gear), press(true), press(false)]); + h.step(); + h.step(); + assert!(h.app.ui.dialog.is_some(), "the click that opens a dialog doesn't close it"); + let r = h.request("ui.clickWidget", json!({"id": "star:bottom:5"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_none(), "the next click on the backdrop does"); } #[test]