From 3962d90f2f3a8444847131b901332cdc49a5b028 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 1 Oct 2026 23:07:11 -0400 Subject: [PATCH] =?UTF-8?q?MENU-FILE-ADDFOLDER=20+=20KEY-QUIT=20+=20KEY-ER?= =?UTF-8?q?ASE:=20Add=20Folder,=20Quit=20everywhere,=20hold=20=E2=8C=A5=20?= =?UTF-8?q?to=20erase?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - File → Add Folder… (file.addFolder {path?}): folder picker → import review, subfolders included. - File → Quit LightCraft (⌘Q / Ctrl+Q) off macOS; the native macOS app menu keeps its own. - Holding ⌥ while painting a brush mask flips Add/Erase for that stroke. - Control channel: injected pointer/key events now announce their modifiers to egui (ModifiersChanged, held from button down to the frame after up), so modifier gestures work and are testable through ui.pointer / ui.click. Co-Authored-By: Claude Opus 5.5 --- apps/lightcraft/src/native_menu.rs | 7 ++++-- crates/ui-egui/src/headless.rs | 35 +++++++++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 29 ++++++++++++++++++++++++ crates/ui-egui/src/menubar.rs | 2 ++ crates/ui-egui/src/menus.rs | 19 +++++++++++++++- crates/ui-egui/src/panels/detail.rs | 4 +++- crates/ui-egui/src/state.rs | 4 ++++ crates/ui-egui/src/tests_masking.rs | 13 +++++++++++ docs/parity.md | 14 ++++++------ 9 files changed, 116 insertions(+), 11 deletions(-) diff --git a/apps/lightcraft/src/native_menu.rs b/apps/lightcraft/src/native_menu.rs index 4286de77..fbcd4987 100644 --- a/apps/lightcraft/src/native_menu.rs +++ b/apps/lightcraft/src/native_menu.rs @@ -224,8 +224,11 @@ impl NativeMenu { ]); } // About and Settings live in the app menu on macOS - let nodes: Vec = - nodes.iter().filter(|n| !matches!(n, MenuNode::Item { id, .. } if id == "app.about" || id == SETTINGS)).cloned().collect(); + let nodes: Vec = nodes + .iter() + .filter(|n| !matches!(n, MenuNode::Item { id, .. } if id == "app.about" || id == SETTINGS || id == QUIT)) + .cloned() + .collect(); let nodes = tidy_separators(nodes); self.append_nodes(&sub, &nodes); if title == "Window" { diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 8d9c94fb..1c242515 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -591,6 +591,41 @@ mod tests { assert!(last["files"][0]["width"].as_u64().is_some_and(|w| w <= 64)); } + /// ⌘Q (File → Quit LightCraft) closes the window. + #[test] + fn cmd_q_quits() { + let mut h = demo([900.0, 600.0]); + assert!(!h.quit_requested()); + h.request("ui.key", json!({"key": "Q", "cmd": true}), Duration::from_secs(10)); + h.step(); + h.step(); + assert!(h.quit_requested()); + } + + /// File → Add Folder… opens the import review for a folder (searched recursively). + #[test] + fn add_folder_opens_the_import_review() { + let mut h = demo([1200.0, 900.0]); + let t = Duration::from_secs(10); + let dir = std::env::temp_dir().join(format!("lc-addfolder-{}", std::process::id())); + let sub = dir.join("day 2"); + std::fs::create_dir_all(&sub).unwrap(); + let img = lightcraft_raster::Rgba8::from_fn(24, 16, |x, y| [(x * 9) as u8, (y * 12) as u8, 80, 255]); + let png = lightcraft_engine::export::encode_image( + &img, + &lightcraft_engine::export::ExportOptions { format: lightcraft_engine::export::ExportFormat::Png, ..Default::default() }, + ) + .unwrap(); + std::fs::write(dir.join("a.png"), &png).unwrap(); + std::fs::write(sub.join("b.png"), &png).unwrap(); + let r = h.request("engine.execute", json!({"command": "file.addFolder", "params": {"path": dir.to_string_lossy()}}), t); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + let Some(crate::state::Dialog::Import { opts }) = &h.app.ui.dialog else { panic!("no import review: {:?}", h.app.ui.dialog) }; + assert_eq!(opts.candidates.len(), 2, "both files, the subfolder's too"); + let _ = std::fs::remove_dir_all(&dir); + } + /// ⇧⌘V opens Paste Selected Settings (prefilled with the copied groups); unchecking a group /// leaves it alone. ⌘F puts typing into the search field. #[test] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 1136526c..53a465ca 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -97,6 +97,10 @@ pub struct LightcraftApp { shadow: Option, /// Synthetic input events (from the control channel) injected one step per frame. pub synthetic: Vec, + /// Modifiers announced for synthetic input (held from a button down to its release). + synthetic_mods: egui::Modifiers, + /// Clear `synthetic_mods` on the next frame. + synthetic_mods_release: bool, styled: bool, fonts_ready: bool, last_time: f64, @@ -148,6 +152,8 @@ impl LightcraftApp { screenshot_token: 0, shadow: None, synthetic: vec![], + synthetic_mods: egui::Modifiers::NONE, + synthetic_mods_release: false, styled: false, fonts_ready: false, last_time: 0.0, @@ -383,6 +389,11 @@ impl LightcraftApp { /// Inject synthetic events (pointer events one per frame; key sequences up to the release). pub fn raw_input_hook(&mut self, raw: &mut egui::RawInput) { + // the frame after a synthetic release / key: modifiers back up + if std::mem::take(&mut self.synthetic_mods_release) && self.synthetic_mods != egui::Modifiers::NONE { + self.synthetic_mods = egui::Modifiers::NONE; + raw.events.push(egui::Event::ModifiersChanged(egui::Modifiers::NONE)); + } if self.synthetic.is_empty() { return; } @@ -393,6 +404,21 @@ impl LightcraftApp { if let Some(egui::Event::PointerMoved(p) | egui::Event::PointerButton { pos: p, .. }) = self.synthetic.first() { raw.events.push(egui::Event::PointerMoved(*p)); } + // egui's `input.modifiers` follow `ModifiersChanged` events, not the modifiers carried by + // pointer/key events: announce the injected events' modifiers (held from button down to + // the frame after up), so ⌥-drag, ⇧-click … work through the control channel + let carried = self.synthetic[..n].iter().find_map(|e| match e { + egui::Event::PointerButton { pressed, modifiers, .. } => Some((*modifiers, !pressed)), + egui::Event::Key { modifiers, .. } => Some((*modifiers, true)), + _ => None, + }); + if let Some((m, ends)) = carried { + if m != self.synthetic_mods { + self.synthetic_mods = m; + raw.events.push(egui::Event::ModifiersChanged(m)); + } + self.synthetic_mods_release = ends; + } raw.events.extend(self.synthetic.drain(..n)); } @@ -404,6 +430,9 @@ impl LightcraftApp { return; } let t0 = now_ms(); + if std::mem::take(&mut self.ui.quit) { + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + } // panels set it again this frame while the pointer rests on a preset or profile self.hover_preview = None; if self.ui.fullscreen { diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 89dbf49a..ff50418b 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -53,6 +53,7 @@ const LAYOUT: &[(&str, &[&str])] = &[ "File", &[ "file.addPhotos", + "file.addFolder", "---", "app.openLibrary", "---", @@ -69,6 +70,7 @@ const LAYOUT: &[(&str, &[&str])] = &[ "---", "library.toggleAutoWriteXmp", "---", + "app.quit", ], ), ( diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 670e409a..c364dd43 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -86,6 +86,8 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("dialog.mergePanorama", "Panorama…", Some("Ctrl+M"), "Photo>Photo Merge"), ("dialog.mergeHdrPanorama", "HDR Panorama…", None, "Photo>Photo Merge"), ("file.addPhotos", "Add Photos…", Some("Cmd+Shift+I"), "File"), + ("file.addFolder", "Add Folder…", None, "File"), + ("app.quit", "Quit LightCraft", Some("Cmd+Q"), "File"), ("file.importPresets", "Import Presets…", None, "File"), ("file.exportPresets", "Export Presets…", None, "File"), ("app.settings", "Settings…", Some("Cmd+,"), "Edit"), @@ -569,6 +571,21 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { + app.ui.quit = true; + Ok(Value::Null) + } + "file.addFolder" => { + // a folder (searched recursively) into the import review + let path = match p.get("path").and_then(Value::as_str) { + Some(x) => Some(x.to_string()), + None => app.services.pick_folder.as_mut().and_then(|f| f()), + }; + match path { + Some(path) => crate::import::open(app, vec![path]), + None => Ok(Value::Null), + } + } "file.importPresets" => { let paths = match p.get("paths").and_then(Value::as_array) { Some(a) => a.iter().filter_map(Value::as_str).map(str::to_string).collect(), @@ -656,7 +673,7 @@ pub fn ui_enabled(app: &LightcraftApp, id: &str) -> bool { "file.exportPresets" => app.session.presets.iter().any(|p| !p.builtin), "view.compare" => app.session.catalog.len() > 1, "view.fullScreenPreview" | "view.infoOverlay" | "view.navigator" => app.session.active().is_some() || app.ui.fullscreen, - "app.openLibrary" => app.services.pick_folder.is_some(), + "app.openLibrary" | "file.addFolder" => app.services.pick_folder.is_some(), "compare.swap" | "compare.makeSelect" => app.ui.view == ViewMode::Compare, s if s.starts_with("dialog.merge") => app.session.targets(&serde_json::json!({})).len() >= 2 && app.merge.final_task.is_none(), _ => true, diff --git a/crates/ui-egui/src/panels/detail.rs b/crates/ui-egui/src/panels/detail.rs index 420482ad..5ecfbb77 100644 --- a/crates/ui-egui/src/panels/detail.rs +++ b/crates/ui-egui/src/panels/detail.rs @@ -857,10 +857,12 @@ fn mask_overlay(app: &mut LightcraftApp, ui: &mut egui::Ui, resp: &egui::Respons && let Some(Gesture::Brush { points }) = app.gesture.take() { let pts: Vec<[f64; 2]> = points.iter().map(|q| [q.x, q.y]).collect(); + // holding ⌥ (Alt) paints with the other mode: Erase while adding, Add while erasing + let erase = app.ui.brush_erase != resp.ctx.input(|i| i.modifiers.alt); let _ = app.run( "mask.brushStroke", json!({"points": pts, "size": app.ui.brush_size, "feather": app.ui.brush_feather, "flow": app.ui.brush_flow, - "erase": app.ui.brush_erase, "autoMask": app.ui.brush_auto_mask}), + "erase": erase, "autoMask": app.ui.brush_auto_mask}), ); } return; diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index 8d76acc6..91e9c98c 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -205,6 +205,9 @@ pub struct UiState { /// A mask being renamed in the Masks list: its id and the edited name. #[serde(skip)] pub renaming_mask: Option<(u32, String)>, + /// Close the window on the next frame (File → Quit). + #[serde(skip)] + pub quit: bool, /// Selected curve channel in the Curve flyout. pub curve_channel: String, /// Selected mixer mode: "hue" | "saturation" | "luminance" | "all". @@ -394,6 +397,7 @@ impl Default for UiState { search: String::new(), focus_search: false, renaming_mask: None, + quit: false, curve_channel: "parametric".into(), mixer_mode: "hue".into(), grading_mode: "3way".into(), diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index e3cc3938..695bb19f 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -104,6 +104,19 @@ fn brush_strokes_carry_auto_mask() { let d = develop(&h); let MaskShape::Brush { strokes } = &d.masks[0].components[0].shape else { panic!("brush") }; assert!(strokes[0].auto_mask && strokes[0].points.len() >= 2, "{strokes:?}"); + assert!(!strokes[0].erase); + // holding ⌥ paints an erase stroke without switching the brush to Erase + let r = h.request( + "ui.pointer", + json!({"events": [{"kind": "down", "x": 0.35, "y": 0.5}, {"kind": "drag", "x": 0.45, "y": 0.5}, {"kind": "up", "x": 0.45, "y": 0.5}], "alt": true}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + let d = develop(&h); + let MaskShape::Brush { strokes } = &d.masks[0].components[0].shape else { panic!("brush") }; + assert_eq!(strokes.len(), 2, "{strokes:?}"); + assert!(strokes[1].erase, "⌥ erases"); + assert!(!h.app.ui.brush_erase, "the brush mode is unchanged"); h.settle(SETTLE); } diff --git a/docs/parity.md b/docs/parity.md index 10ef6b93..e314ea34 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -45,10 +45,10 @@ and prints the summary; `cargo xtask parity --write` refreshes the summary table | V. Preferences (PREF) | 4 | 1 | 3 | 3 | 1/1 (100%) | 3/4 (75%) | | W. Cloud & AI infrastructure (CLOUD / AI) | 0 | 1 | 1 | 2 | — | — | | X. Cross-cutting behaviours (BEHAV) | 11 | 2 | 4 | 1 | 8/8 (100%) | 3/5 (60%) | -| Y. Menus | 59 | 13 | 13 | 8 | 44/47 (94%) | 13/23 (57%) | -| Z. Keyboard shortcuts (desktop) | 56 | 13 | 11 | 1 | 47/52 (90%) | 9/23 (39%) | +| Y. Menus | 61 | 11 | 13 | 8 | 46/47 (98%) | 13/23 (57%) | +| Z. Keyboard shortcuts (desktop) | 58 | 11 | 11 | 1 | 49/52 (94%) | 9/23 (39%) | | Lightroom Classic extras | 4 | 20 | 55 | 9 | — | 3/21 (14%) | -| **Total** | 269 | 75 | 156 | 37 | 187/198 (94%) | 72/144 (50%) | +| **Total** | 273 | 71 | 156 | 37 | 191/198 (96%) | 72/144 (50%) | ## Top gaps @@ -501,9 +501,9 @@ visible menu bar yet: the menu model is only exposed through the control channel | MENU-APP-SYNC | Sync status / pause | OOS | 🚫 | | | | MENU-APP-SIGNOUT | Sign out | OOS | 🚫 | | | | MENU-APP-HIDE | Hide / hide others / show all | P1 | 🟡 | | platform window defaults (unverified) | -| MENU-APP-QUIT | Quit | P0 | 🟡 | | platform window defaults (unverified) | +| MENU-APP-QUIT | Quit | P0 | ✅ | `cmd:app.quit`, `apps/lightcraft/src/native_menu.rs` | macOS: app menu (native); elsewhere: File → Quit LightCraft | | MENU-FILE-ADDPHOTOS | Add Photos… | P0 | ✅ | `cmd:file.addPhotos` | | -| MENU-FILE-ADDFOLDER | Add Folder… | P0 | 🟡 | `cmd:library.import` (folders, recursive) | no folder picker entry | +| MENU-FILE-ADDFOLDER | Add Folder… | P0 | ✅ | `cmd:file.addFolder`, `cmd:library.importPreview` | folder picker (desktop) → the import review, subfolders included; `path` param for agents | | MENU-FILE-MIGRATE | Migrate photos | OOS | 🚫 | | | | MENU-FILE-NEWALBUM | New Album… | P0 | ✅ | `cmd:dialog.newAlbum` | | | MENU-FILE-NEWFOLDER | New Folder… | P0 | ✅ | `cmd:dialog.newFolder` | ⇧⌘N | @@ -631,7 +631,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-STACK | Group / ungroup stack — ⌘G / ⇧⌘G | P1 | ✅ | `cmd:stack.group`, `cmd:stack.ungroup` | also S expand/collapse, ⇧S top of stack | | KEY-GUIDEDUPRIGHT | Guided Upright — ⇧G | P1 | 🟡 | `cmd:geometry.upright` | button in the crop panel; ⇧G = Square Grid | | KEY-HIDE | Hide / hide others — ⌘H / ⌥⌘H | P1 | 🟡 | | platform default (unverified) | -| KEY-QUIT | Quit — ⌘Q | P0 | 🟡 | | platform default (unverified) | +| KEY-QUIT | Quit — ⌘Q | P0 | ✅ | `cmd:app.quit` | ⌘Q (Ctrl+Q off macOS) | | KEY-CREATEVERSION | Create version — ⇧M | P1 | ✅ | `cmd:version.create` | secondary binding (primary ⌘⇧S) | | KEY-CLOSEWINDOW | Close window — ⌘W | P1 | 🟡 | | platform default (unverified) | | KEY-DELETE | Delete photo — ⌫ | P0 | ✅ | `cmd:photo.delete` | | @@ -651,7 +651,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-RATING | Ratings — 0…5 | P0 | ✅ | `crates/ui-egui/src/shortcuts.rs` → `cmd:photo.rate` | | | KEY-LABELS | Labels — 6…9 | P1 | ✅ | `crates/ui-egui/src/shortcuts.rs` → `cmd:photo.label` | | | KEY-MASKING | Masking — M | P0 | ✅ | `cmd:panel.masking` | | -| KEY-ERASE | Erase while held — ⌥ | P0 | 🟡 | `crates/ui-egui/src/panels/masking.rs` | Add/Erase toggle buttons; hold-to-erase unverified | +| KEY-ERASE | Erase while held — ⌥ | P0 | ✅ | `crates/ui-egui/src/panels/detail.rs`, `crates/ui-egui/src/panels/masking.rs` | ⌥ while painting flips Add ↔ Erase for that stroke; Add/Erase buttons too | | KEY-RATEADVANCE | Rate and advance — ⇧0…5 | P1 | ✅ | `crates/ui-egui/src/shortcuts.rs` → `cmd:photo.rate` (`advance`) | | | KEY-FLAGADVANCE | Flag and advance — ⇧Z / ⇧X / ⇧U | P1 | 🟡 | `cmd:photo.flag` (`advance`) | ⇧X and ⇧U bound; pick-and-advance has no key (⇧P = presets) | | KEY-NEXTPREV | Next / previous — → / ← | P0 | ✅ | `cmd:library.next`, `cmd:library.previous` | |