Merge PR #1802 (rebased)

# Conflicts:
#	docs/whats-new.md

crates/ui-egui/src/lib.rs

@@ -73,6 +73,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_empty_library;

crates/ui-egui/src/panels/dialogs.rs

@@ -1292,17 +1292,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 row_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(row_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::LightroomResult { first: None, .. })
-                    || matches!(
-                        dlg,
-                        Dialog::About | Dialog::Shortcuts | Dialog::Settings { .. } | Dialog::SystemInfo { .. } | Dialog::AllMetadata { .. } | Dialog::WhatsNew
-                            | Dialog::ManageEffects { .. }
-                    );
+                let informational = unusable_model || matches!(dlg, Dialog::LightroomResult { first: None, .. }) || 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
@@ -1313,13 +1313,6 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
                     Dialog::LightroomResult { .. } => "Close",
                     _ => "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 } => {
@@ -1371,7 +1364,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);
                 }
@@ -2216,3 +2239,36 @@ 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::DeleteEffects { .. } | 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
+            | Dialog::ManageEffects { .. }
+    )
+}

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");
+}

docs/whats-new.md

@@ -130,6 +130,11 @@
   script, the control channel or MCP is now refused (issue #1506). It used to act on the selected photos instead,
   or on the part of the list it could read, so a mistyped id could rate, move or delete the wrong photos.
 
+### 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.