From f50372d80887d1db6aad8c05fdcdc992e35dbb06 Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Wed, 14 Oct 2026 16:55:38 -0400 Subject: [PATCH 1/2] Dialogs: the confirm button is on the right, filled when it is the next step A design critique of the first run found the import review's "Import N Photos" a plain button at the bottom left beside Cancel, though the welcome before it had taught that blue means go. - Every dialog's footer is right-aligned: the confirm on the right, Cancel to its left, added in that order so the keyboard goes through them as the eye does. The row spans the width the window gives its content this frame, so it reaches the window's edge without widening it. - The confirm is filled with the accent when it makes or brings in something (`confirm_style`); Delete and Remove, and an informational dialog's Close, stay plain. Which dialogs only show something is one list (`only_shows`). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/dialogs.rs | 78 ++++++++++++++++++---- crates/ui-egui/src/tests_dialog_primary.rs | 74 ++++++++++++++++++++ docs/whats-new.md | 5 ++ 4 files changed, 145 insertions(+), 14 deletions(-) create mode 100644 crates/ui-egui/src/tests_dialog_primary.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 37fb86df3..544724671 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -63,6 +63,8 @@ mod tests_date_picker; #[cfg(test)] mod tests_dialog_fit; #[cfg(test)] +mod tests_dialog_primary; +#[cfg(test)] mod tests_dialogs; #[cfg(test)] mod tests_error_kinds; diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index a79261fdc..c91266a26 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -1194,15 +1194,17 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { Dialog::Shortcuts => crate::panels::keymap::body(app, ui, &t), }); ui.add_space(4.0); - ui.horizontal(|ui| { + // the confirm on the right with Cancel to its left (macOS and GNOME's order), filled with + // the accent when it is the dialog's primary action. The row spans the width the window + // gives its content this frame (its default width, or its content's if wider), so it + // reaches the edge without widening the window. + let content_w = ui.available_width().max(ui.min_rect().width()).max(1.0); + let row_h = ui.spacing().interact_size.y; + ui.allocate_ui_with_layout(egui::vec2(content_w, row_h), egui::Layout::left_to_right(egui::Align::Center), |ui| { // a model file LightCraft cannot use has nothing to confirm, nor has a dialog that // only shows something (one Close, no Cancel / OK) let unusable_model = matches!(&dlg, Dialog::DenoiseModel { info, .. } | Dialog::FaceModel { info, .. } if info["kind"] == "unsupported"); - let informational = unusable_model - || matches!( - dlg, - Dialog::About | Dialog::Shortcuts | Dialog::Settings { .. } | Dialog::SystemInfo { .. } | Dialog::AllMetadata { .. } | Dialog::WhatsNew - ); + let informational = unusable_model || only_shows(&dlg); let sam = &app.session.segmenter; let (sam_installed, sam_running, sam_failed) = (sam.installed(), sam.download_status().running, sam.download_status().error.is_some()); // no download location in this build: nothing to offer but the manual install @@ -1212,13 +1214,6 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { Dialog::SamModel { .. } => "Not Now", _ => "Cancel", }; - if !informational { - let r = ui.button(crate::i18n::tr(cancel)); - crate::widgets::register(ui.ctx(), "button:dialogCancel", r.rect); - if r.clicked() { - close = true; - } - } let add_label; let ok = match &dlg { Dialog::Import { opts } => { @@ -1258,7 +1253,37 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { ) // smart-album rules that can't mean anything wait to be fixed && !matches!(&dlg, Dialog::SmartRules { id, rules, .. } if !rules.check_for(&app.session.catalog, id.map(lightcraft_catalog::AlbumId)).is_empty()); - let r = (!ok.is_empty()).then(|| ui.add_enabled(can_confirm, egui::Button::new(crate::i18n::tr(ok)))); + let primary = !informational && can_confirm && confirm_style(&dlg) == ConfirmStyle::Primary; + // (Cancel first, then the confirm: the keyboard goes through them as the eye does) + let width_of = |label: &str| { + let text = egui::WidgetText::from(crate::i18n::tr(label)) + .into_galley(ui, Some(egui::TextWrapMode::Extend), f32::INFINITY, egui::TextStyle::Button) + .size() + .x; + (text + 2.0 * ui.spacing().button_padding.x).max(ui.spacing().interact_size.x) + }; + let gap = ui.spacing().item_spacing.x; + let mut buttons_w = 0.0; + if !informational { + buttons_w += width_of(cancel) + gap; + } + if !ok.is_empty() { + buttons_w += width_of(ok); + } + ui.add_space((ui.available_width() - buttons_w).max(0.0)); + if !informational { + let r = ui.button(crate::i18n::tr(cancel)); + crate::widgets::register(ui.ctx(), "button:dialogCancel", r.rect); + if r.clicked() { + close = true; + } + } + let button = if primary { + egui::Button::new(egui::RichText::new(crate::i18n::tr(ok)).color(egui::Color32::WHITE)).fill(t.accent) + } else { + egui::Button::new(crate::i18n::tr(ok)) + }; + let r = (!ok.is_empty()).then(|| ui.add_enabled(can_confirm, button)); if let Some(r) = &r { crate::widgets::register(ui.ctx(), "button:dialogOk", r.rect); } @@ -1975,3 +2000,28 @@ mod watermark_position_tests { } } } + +/// How a dialog's confirm button looks: [`ConfirmStyle::Primary`] (the accent's fill) when it +/// makes or brings in something, plain when it deletes or removes (never the eye-catching +/// default) or when the dialog only shows something (its Close). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ConfirmStyle { + Primary, + Plain, +} + +pub fn confirm_style(dlg: &Dialog) -> ConfirmStyle { + match dlg { + Dialog::ConfirmDelete { .. } | Dialog::DeleteKeyword { .. } | Dialog::RemoveFolder { .. } => ConfirmStyle::Plain, + _ if only_shows(dlg) => ConfirmStyle::Plain, + _ => ConfirmStyle::Primary, + } +} + +/// A dialog that only shows something: one Close, nothing to confirm or cancel. +fn only_shows(dlg: &Dialog) -> bool { + matches!( + dlg, + Dialog::About | Dialog::Shortcuts | Dialog::Settings { .. } | Dialog::SystemInfo { .. } | Dialog::AllMetadata { .. } | Dialog::WhatsNew + ) +} diff --git a/crates/ui-egui/src/tests_dialog_primary.rs b/crates/ui-egui/src/tests_dialog_primary.rs new file mode 100644 index 000000000..29078d009 --- /dev/null +++ b/crates/ui-egui/src/tests_dialog_primary.rs @@ -0,0 +1,74 @@ +//! A dialog's confirm button reads as its primary action: on the right, Cancel to its left, and +//! filled with the accent when it makes or brings in something (Import, Merge, Save…). A +//! destructive confirm (Delete, Remove) stays plain: never the eye-catching default. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::panels::dialogs::{ConfirmStyle, confirm_style}; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); + +fn rect(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("{id} not on screen")) +} + +fn png(dir: &std::path::Path, name: &str) -> String { + let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[90, 120, 160, 255]; 64] }; + let bytes = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap(); + let p = dir.join(name); + std::fs::write(&p, bytes).unwrap(); + p.to_string_lossy().to_string() +} + +#[test] +fn the_import_review_puts_its_confirm_on_the_right_as_the_primary_action() { + // Given the import review of two photos + let dir = std::env::temp_dir().join(format!("lc-ui-dialog-primary-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let paths = [png(&dir, "a.png"), png(&dir, "b.png")]; + let app = LightcraftApp::new(lightcraft_engine::Session::new().with_fs(), Services::default()); + let mut h = Headless::new(app, [1400.0, 1000.0], 1.0); + h.step(); + h.app.run("file.addPhotos", json!({"paths": paths})).unwrap(); + h.step_until(T, |h| h.app.widgets.iter().any(|(w, _)| w == "button:dialogOk")); + h.step(); + // Then "Import 2 Photos" is right of Cancel, at the dialog's right edge + let (ok, cancel, window) = (rect(&h, "button:dialogOk"), rect(&h, "button:dialogCancel"), rect(&h, "dialog:window")); + assert!(ok.left() > cancel.right(), "confirm {ok:?} right of cancel {cancel:?}"); + assert!(window.right() - ok.right() < 40.0, "confirm {ok:?} at the right edge of {window:?}"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn constructive_confirms_are_primary_and_destructive_ones_plain() { + use crate::state::Dialog; + assert_eq!(confirm_style(&Dialog::AutoStack { gap: 60.0 }), ConfirmStyle::Primary); + assert_eq!(confirm_style(&Dialog::ConfirmDelete { count: 3 }), ConfirmStyle::Plain); + // a dialog that only shows something has nothing to confirm: its Close is plain + assert_eq!(confirm_style(&Dialog::About), ConfirmStyle::Plain); +} + +#[test] +fn a_dialog_wider_than_its_content_still_ends_its_footer_at_its_edge() { + // Auto-Stack's title makes it wider than its slider and note: the buttons go to the edge, and + // the window doesn't grow for them + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services::default()); + let mut h = Headless::new(app, [1400.0, 1000.0], 1.0); + h.step(); + h.app.run("dialog.autoStack", json!({})).unwrap(); + for _ in 0..4 { + h.step(); + } + let (ok, window) = (rect(&h, "button:dialogOk"), rect(&h, "dialog:window")); + assert!(window.right() - ok.right() < 24.0, "confirm {ok:?} at the right edge of {window:?}"); + let width = window.width(); + for _ in 0..4 { + h.step(); + } + assert_eq!(rect(&h, "dialog:window").width(), width, "the footer doesn't widen the window"); +} diff --git a/docs/whats-new.md b/docs/whats-new.md index 1bf44c3f4..d8611d7a6 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,11 @@ ## 2026-10-14 +### A dialog's confirm button says it's the next step +- In dialogs, the confirm button is on the right with Cancel to its left, and filled with the accent when it makes or + brings in something (Import N Photos, Merge, Save, Create, Download…). Delete and Remove stay plain, so a destructive + choice is never the eye-catching default. + ### Hard-edged Heal and Clone spots - A spot with Feather at 0 no longer draws black pixels exactly on its radius. This also fixes spots at 0% Opacity changing pixels along their edge. The hard edge now leaves pixels on the radius unchanged. From 81a9569301426aafe844e17056a88ba4154df207 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 15 Oct 2026 00:42:56 -0400 Subject: [PATCH 2/2] Batch 36: rustfmt after merging #1802 Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/dialogs.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 8e73e62f6..44509cd06 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -2251,7 +2251,9 @@ pub enum ConfirmStyle { pub fn confirm_style(dlg: &Dialog) -> ConfirmStyle { match dlg { - Dialog::ConfirmDelete { .. } | Dialog::DeleteKeyword { .. } | Dialog::DeleteEffects { .. } | Dialog::RemoveFolder { .. } => ConfirmStyle::Plain, + Dialog::ConfirmDelete { .. } | Dialog::DeleteKeyword { .. } | Dialog::DeleteEffects { .. } | Dialog::RemoveFolder { .. } => { + ConfirmStyle::Plain + } _ if only_shows(dlg) => ConfirmStyle::Plain, _ => ConfirmStyle::Primary, } @@ -2261,7 +2263,12 @@ pub fn confirm_style(dlg: &Dialog) -> ConfirmStyle { fn only_shows(dlg: &Dialog) -> bool { matches!( dlg, - Dialog::About | Dialog::Shortcuts | Dialog::Settings { .. } | Dialog::SystemInfo { .. } | Dialog::AllMetadata { .. } | Dialog::WhatsNew + Dialog::About + | Dialog::Shortcuts + | Dialog::Settings { .. } + | Dialog::SystemInfo { .. } + | Dialog::AllMetadata { .. } + | Dialog::WhatsNew | Dialog::ManageEffects { .. } ) }