diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6b5033e0..34a38a2d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -41,6 +41,8 @@ mod tests_activity; #[cfg(test)] mod tests_album_picker; #[cfg(test)] +mod tests_copy_paste_dialogs; +#[cfg(test)] mod tests_crop_rotate; #[cfg(test)] mod tests_curve; diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index aa287c95..99275e4b 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -77,6 +77,16 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { }); let mut close = false; let mut confirm = false; + // Return presses OK in a dialog without a text field of its own (Lightroom's default button); + // a widget that has the keyboard focus (a text field, a button or checkbox reached with Tab) + // takes Return itself. Consumed before the dialog is drawn, so nothing else sees it too. + if return_confirms(&dlg) + && ctx.memory(|m| m.focused().is_none()) + && !egui::Popup::is_any_open(ctx) + && ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Enter)) + { + confirm = true; + } let title: String = match &dlg { Dialog::TextPrompt { title, .. } => title.as_str(), Dialog::NewAlbum { folder: true, .. } => "Create Folder", @@ -1556,6 +1566,12 @@ fn choices(ui: &mut egui::Ui, label: &str, id: &str, option }); } +/// Dialogs that Return confirms with nothing focused (those with text fields confirm from the +/// field: `dialog_field`). +fn return_confirms(dlg: &Dialog) -> bool { + matches!(dlg, Dialog::CopySettings { .. } | Dialog::PasteSettings { .. }) +} + /// 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 diff --git a/crates/ui-egui/src/tests_copy_paste_dialogs.rs b/crates/ui-egui/src/tests_copy_paste_dialogs.rs new file mode 100644 index 00000000..5a8bf4ad --- /dev/null +++ b/crates/ui-egui/src/tests_copy_paste_dialogs.rs @@ -0,0 +1,91 @@ +//! Return presses OK in Copy Settings (⇧⌘C) and Paste Selected Settings (⇧⌘V); Esc cancels (#735). + +use std::time::Duration; + +use lightcraft_catalog::PhotoId; +use serde_json::{Value, json}; + +use crate::LightcraftApp; +use crate::headless::Headless; +use crate::state::Dialog; + +const SETTLE: Duration = Duration::from_secs(120); +const T: Duration = Duration::from_secs(10); + +/// The demo library with an edited first photo selected; returns the first two photos' ids. +fn edited() -> (Headless, [u64; 2]) { + let services = crate::Services { png: None, ..Default::default() }; + let mut app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + app.ui.view = crate::state::ViewMode::PhotoGrid; + let mut h = Headless::new(app, [1200.0, 900.0], 1.0); + let ids: Vec = h.app.session.visible_cloned().iter().take(2).map(|p| p.0).collect(); + let ids = [ids[0], ids[1]]; + exec(&mut h, "library.select", json!({"ids": [ids[0]]})); + exec(&mut h, "develop.set", json!({"values": {"light.exposure": 1.0, "color.vibrance": 30}})); + (h, ids) +} + +fn exec(h: &mut Headless, command: &str, params: Value) { + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{command}: {r}"); +} + +fn key(h: &mut Headless, k: Value) { + let r = h.request("ui.key", k.clone(), T); + assert_eq!(r["ok"], true, "{k}: {r}"); + h.settle(SETTLE); +} + +fn copy_dialog(h: &mut Headless) { + key(h, json!({"key": "C", "cmd": true, "shift": true})); + assert!(matches!(h.app.ui.dialog, Some(Dialog::CopySettings { .. })), "dialog not open: {:?}", h.app.ui.dialog); +} + +/// Copies light and colour from the first photo, selects the second and opens ⇧⌘V. +fn paste_dialog(h: &mut Headless, ids: [u64; 2]) { + exec(h, "develop.copy", json!({"groups": ["light", "color"]})); + exec(h, "library.select", json!({"ids": [ids[1]]})); + key(h, json!({"key": "V", "cmd": true, "shift": true})); + assert!(matches!(h.app.ui.dialog, Some(Dialog::PasteSettings { .. })), "dialog not open: {:?}", h.app.ui.dialog); +} + +fn exposure(h: &Headless, id: u64) -> f64 { + h.app.session.develop_of(PhotoId(id)).map(|d| d.light.exposure).unwrap_or_default() +} + +#[test] +fn return_copies_the_chosen_settings() { + let (mut h, _) = edited(); + copy_dialog(&mut h); + assert!(h.app.session.clipboard.is_none()); + key(&mut h, json!({"key": "Enter"})); + assert!(h.app.ui.dialog.is_none(), "Return closes the dialog"); + assert!(h.app.session.clipboard.is_some(), "Return copies the settings"); +} + +#[test] +fn escape_closes_copy_settings_without_copying() { + let (mut h, _) = edited(); + copy_dialog(&mut h); + key(&mut h, json!({"key": "Escape"})); + assert!(h.app.ui.dialog.is_none(), "Esc closes the dialog"); + assert!(h.app.session.clipboard.is_none(), "Esc copies nothing"); +} + +#[test] +fn return_pastes_into_the_selected_photos() { + let (mut h, ids) = edited(); + paste_dialog(&mut h, ids); + key(&mut h, json!({"key": "Enter"})); + assert!(h.app.ui.dialog.is_none(), "Return closes the dialog"); + assert_eq!(exposure(&h, ids[1]), 1.0, "Return pastes the settings"); +} + +#[test] +fn escape_closes_paste_settings_without_pasting() { + let (mut h, ids) = edited(); + paste_dialog(&mut h, ids); + key(&mut h, json!({"key": "Escape"})); + assert!(h.app.ui.dialog.is_none(), "Esc closes the dialog"); + assert_eq!(exposure(&h, ids[1]), 0.0, "Esc pastes nothing"); +} diff --git a/docs/parity.md b/docs/parity.md index d0bdc35c..f34ca3cd 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -572,8 +572,8 @@ visible menu bar yet: the menu model is only exposed through the control channel | MENU-EDIT-UNDO | Undo | P0 | ✅ | `cmd:edit.undo` | label does not name the step | | MENU-EDIT-REDO | Redo | P0 | ✅ | `cmd:edit.redo` | | | MENU-EDIT-COPYPASTE | Copy / paste (edit settings) | P0 | ✅ | `cmd:develop.copy`, `cmd:develop.paste` | | -| MENU-EDIT-CHOOSECOPY | Choose Edit Settings to Copy… | P0 | ✅ | `cmd:dialog.copySettings` | | -| MENU-EDIT-PASTESELECTED | Paste Selected Settings | P0 | ✅ | `cmd:dialog.pasteSettings`, `cmd:develop.paste` (`groups`) | checklist prefilled with the copied groups; only copied groups can be pasted | +| MENU-EDIT-CHOOSECOPY | Choose Edit Settings to Copy… | P0 | ✅ | `cmd:dialog.copySettings` | Return copies, Esc cancels | +| MENU-EDIT-PASTESELECTED | Paste Selected Settings | P0 | ✅ | `cmd:dialog.pasteSettings`, `cmd:develop.paste` (`groups`) | checklist prefilled with the copied groups; only copied groups can be pasted; Return pastes, Esc cancels | | MENU-EDIT-SELECTALL | Select All | P0 | ✅ | `cmd:library.selectAll` | | | MENU-EDIT-SELECTNONE | Select None | P0 | ✅ | `cmd:library.selectNone` | | | MENU-EDIT-SELECTBY | Select by flag / rating | P1 | ✅ | `cmd:library.selectBy` | Edit → Select by: picks, rejects, unflagged, ★…★★★★★ and higher, unrated, colour labels; `add` extends the selection | diff --git a/docs/whats-new.md b/docs/whats-new.md index fb1fd316..c9c34b8a 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -157,6 +157,8 @@ - Shortcuts are editable: Help ▸ Keyboard Shortcuts (⌘/) lists every command with a search box; click a shortcut and press the new keys (Esc cancels), × removes it, ↺ restores the original, Reset All undoes every change. A key that belonged to another command moves to the new one. Menus show the new keys; agents use `app.setShortcut`. +- Return presses OK in Choose Edit Settings to Copy (⇧⌘C) and Paste Selected Settings (⇧⌘V), and Esc cancels them + (issue #735). ### RAW decoding - Olympus ORF raws get the starting look fitted to the camera's own JPEG too, instead of opening flat and grey (the