diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d4520bfa8..d268ec6ec 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -135,6 +135,8 @@ mod tests_sam_setup; #[cfg(test)] mod tests_scroll; #[cfg(test)] +mod tests_slider_reset; +#[cfg(test)] mod tests_slideshow; #[cfg(test)] mod tests_solo_sections; diff --git a/crates/ui-egui/src/tests_slider_reset.rs b/crates/ui-egui/src/tests_slider_reset.rs new file mode 100644 index 000000000..6d0357fcb --- /dev/null +++ b/crates/ui-egui/src/tests_slider_reset.rs @@ -0,0 +1,151 @@ +//! Headless tests of resetting a slider (issue #1436): double-clicking its name or its thumb puts it +//! back to its default in one undo step, as in Lightroom, and resting the pointer on either says so. + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); +/// The sliders the issue names, each set away from its default. +const EDITED: [(&str, f64); 2] = [("light.exposure", 1.25), ("light.contrast", 40.0)]; + +/// The demo library in Detail with the Edit panel's Light section open. +fn edit_panel() -> Headless { + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), Services { png: None, ..Default::default() }); + let mut h = Headless::new(app, [1400.0, 900.0], 1.0); + let id = h.app.session.visible_cloned()[0].0; + assert_eq!(h.request("engine.execute", json!({"command": "library.select", "params": {"ids": [id]}}), T)["ok"], true); + assert_eq!(h.request("ui.set", json!({"view": "detail", "right": "edit"}), T)["ok"], true); + h.app.ui.open_sections = vec!["light".to_string()]; + h.settle(SETTLE); + h +} + +fn widget(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("no widget {id}")) +} + +fn value(h: &Headless, control: &str) -> f64 { + let id = h.app.session.selection.active.unwrap(); + lightcraft_develop::controls::get(&h.app.session.develop_of(id).unwrap_or_default(), control).unwrap() +} + +fn default(control: &str) -> f64 { + lightcraft_develop::controls::find(control).unwrap().default +} + +fn ask(h: &mut Headless, method: &str, params: serde_json::Value) { + let r = h.request(method, params.clone(), T); + assert_eq!(r["ok"], true, "{method} {params}: {r}"); + h.settle(SETTLE); +} + +fn set(h: &mut Headless, control: &str, value: f64) { + ask(h, "engine.execute", json!({"command": "develop.set", "params": {"control": control, "value": value}})); +} + +/// The thumb's place along the track (0..1), plus `dx` points: a hand lands near its centre, not on it. +fn thumb_fx(h: &Headless, control: &str, dx: f32) -> f32 { + let spec = lightcraft_develop::controls::find(control).unwrap(); + let track = widget(h, &format!("slider:{control}")); + spec.position(value(h, control)) as f32 + dx / track.width() +} + +/// A double-click on a widget: the control channel plays each press and release in a frame of its +/// own, so the slider sees the first click before the second, as with a hand on a mouse. +fn double_click(h: &mut Headless, id: &str, fx: f32) { + ask(h, "ui.clickWidget", json!({"id": id, "count": 2, "fx": fx})); +} + +#[test] +fn double_clicking_a_sliders_name_resets_it_in_one_step() { + let mut h = edit_panel(); + for (control, edited) in EDITED { + set(&mut h, control, edited); + let undo = h.app.session.undo.len(); + double_click(&mut h, &format!("sliderLabel:{control}"), 0.1); + assert_eq!(value(&h, control), default(control), "{control} is back at its default"); + assert_eq!(h.app.session.undo.len() - undo, 1, "{control}: one undo step"); + ask(&mut h, "engine.execute", json!({"command": "edit.undo"})); + assert_eq!(value(&h, control), edited, "{control}: undo brings the edit back"); + } +} + +/// The first click of the double-click lands a few points off the thumb's centre: it leaves the value +/// alone (it used to move it there, a history step of its own before the reset). +#[test] +fn double_clicking_a_sliders_thumb_resets_it_in_one_step() { + let mut h = edit_panel(); + for (control, edited) in EDITED { + set(&mut h, control, edited); + let undo = h.app.session.undo.len(); + let fx = thumb_fx(&h, control, 4.0); + double_click(&mut h, &format!("slider:{control}"), fx); + assert_eq!(value(&h, control), default(control), "{control} is back at its default"); + assert_eq!(h.app.session.undo.len() - undo, 1, "{control}: one undo step"); + ask(&mut h, "engine.execute", json!({"command": "edit.undo"})); + assert_eq!(value(&h, control), edited, "{control}: undo brings the edit back"); + } +} + +/// A click on the thumb leaves the value alone; a click elsewhere on the track still moves it there. +#[test] +fn a_click_on_the_thumb_keeps_the_value() { + let mut h = edit_panel(); + set(&mut h, "light.exposure", 1.25); + let fx = thumb_fx(&h, "light.exposure", -4.0); + ask(&mut h, "ui.clickWidget", json!({"id": "slider:light.exposure", "fx": fx})); + assert_eq!(value(&h, "light.exposure"), 1.25); + ask(&mut h, "ui.clickWidget", json!({"id": "slider:light.exposure", "fx": 0.25})); + assert!((value(&h, "light.exposure") - -2.5).abs() < 0.02, "{}", value(&h, "light.exposure")); +} + +#[test] +fn a_sliders_name_and_thumb_say_a_double_click_resets_it() { + let mut h = edit_panel(); + set(&mut h, "light.exposure", 1.25); + let hint = "Exposure — double-click to reset it"; + let shown = |h: &Headless| h.painted_text().iter().any(|t| t == hint); + let thumb = thumb_fx(&h, "light.exposure", 0.0); + for (what, id, fx) in [("name", "sliderLabel:light.exposure", 0.1), ("thumb", "slider:light.exposure", thumb)] { + // somewhere else first, so the hint of the step before goes + ask(&mut h, "ui.hoverWidget", json!({"id": "sliderLabel:light.contrast", "fx": 0.9})); + assert!(!shown(&h), "{what}: no hint left over"); + ask(&mut h, "ui.hoverWidget", json!({"id": id, "fx": fx})); + // tooltips wait for the pointer to rest + assert!( + h.step_until(Duration::from_secs(5), shown), + "{what}: {:?}", + h.painted_text().iter().filter(|t| t.contains("reset")).collect::>() + ); + } + // the rest of the track is for dragging and clicking: no hint there + ask(&mut h, "ui.hoverWidget", json!({"id": "slider:light.exposure", "fx": 0.1})); + for _ in 0..90 { + h.step(); + } + assert!(!shown(&h), "no hint away from the thumb"); + // the value keeps its own hint + ask(&mut h, "ui.hoverWidget", json!({"id": "sliderValue:light.exposure"})); + assert!(h.step_until(Duration::from_secs(5), |h| h.painted_text().iter().any(|t| t == "Click to type a value"))); +} + +/// Typing a value (KEY-SLIDER-TYPE, #1362) still works next to the reset. +#[test] +fn a_typed_value_still_applies_after_a_reset() { + let mut h = edit_panel(); + set(&mut h, "light.exposure", 1.25); + double_click(&mut h, "sliderLabel:light.exposure", 0.1); + assert_eq!(value(&h, "light.exposure"), 0.0); + let undo = h.app.session.undo.len(); + ask(&mut h, "ui.clickWidget", json!({"id": "sliderValue:light.exposure"})); + for (method, params) in [("ui.text", json!({"text": "0.5"})), ("ui.key", json!({"key": "Enter"}))] { + ask(&mut h, method, params); + } + assert_eq!(value(&h, "light.exposure"), 0.5); + assert_eq!(h.app.session.undo.len() - undo, 1, "one undo step"); +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 67fa4d394..870e6b107 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -218,9 +218,12 @@ pub(crate) fn shown_value(spec: &ControlSpec, v: f64) -> String { if shown == "+0" || shown == "-0" { "0".to_string() } else { shown } } +/// The radius of a slider's hollow ring thumb. +const THUMB_RING: f32 = 7.0; + /// A Lightroom slider row (label + value above a track with a hollow ring thumb). -/// Double-click the label or thumb to reset. Shift-drag = fine adjustment. Scroll sideways over -/// it to nudge it. Click the value to type one: Return or clicking away applies it, Esc keeps the +/// Double-click the label or thumb to reset (hovering either says so). Shift-drag = fine +/// adjustment. Scroll sideways over it to nudge it. Click the value to type one: Return or clicking away applies it, Esc keeps the /// old one. pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_override: Option<&str>) -> SliderOut { let t = Tokens::get(ui.ctx()); @@ -257,8 +260,13 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ node.add_action(egui::accesskit::Action::Decrement); } }); - let label_resp = ui.interact(label_rect, id.with("label"), Sense::click()); + // the name: double-click it to reset, as the thumb (a disabled slider's name is only text) + let label_resp = ui.interact(label_rect, id.with("label"), if enabled { Sense::click() } else { Sense::hover() }); + // say how to reset (issue #1436: the double-click was hard to discover), as the curve channels do + let reset_hint = crate::i18n::tr_format!("{name} — double-click to reset it", name = label_text); + let label_resp = if enabled { label_resp.on_hover_text(reset_hint.as_str()) } else { label_resp }; crate::widgets::announce(&label_resp, crate::i18n::tr_format!("Reset {name}", name = label_text.clone())); + register(ui.ctx(), format!("sliderLabel:{}", spec.id), label_rect); register(ui.ctx(), format!("slider:{}", spec.id), track_rect); // the value: click it to type an exact one (over the label, so it takes the click) let value_rect = Rect::from_min_max(pos2(label_rect.right() - 64.0, label_rect.top()), label_rect.max); @@ -286,6 +294,12 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ // value ↔ track position: linear, except for the Kelvin temperature (`ControlSpec::position`) let to_x = |v: f64| track_rect.left() + spec.position(v) as f32 * track_rect.width(); let from_x = |x: f32| spec.value_at(f64::from((x - track_rect.left()) / track_rect.width())); + // the thumb: a click there doesn't move the value, and resting on it says how to reset (the rest + // of the track, which is clicked and dragged, gets no tooltip) + let on_thumb = |p: egui::Pos2| (p.x - to_x(value)).abs() <= THUMB_RING + 1.0; + if enabled && !resp.dragged() && resp.hover_pos().is_some_and(on_thumb) { + resp.clone().on_hover_text(reset_hint.as_str()); + } let mut v = value; if let Some((mut text, frames, pressed)) = typing.take() { // Keep the owned opening release focused without restarting its draft or selection. @@ -358,6 +372,9 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ } } else if resp.clicked() && let Some(p) = resp.interact_pointer_pos() + // a click on the thumb leaves it there, so the first click of a double-click on it isn't a + // history step of its own before the reset + && !on_thumb(p) { let step = spec.step.max(1e-9); let nv = ((from_x(p.x) / step).round() * step).clamp(spec.min, spec.max); @@ -458,7 +475,7 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ ui.ctx().set_cursor_icon(egui::CursorIcon::Text); } } - let ring = 7.0; + let ring = THUMB_RING; let tx = to_x(v); paint_track(ui, track_rect, &spec.track, &t, tx, ring); let p = ui.painter(); @@ -930,6 +947,10 @@ pub(crate) fn elide_head(text: &str, max: f32, width: impl Fn(&str) -> f32) -> S #[path = "widgets/tests_numeric_entry.rs"] mod numeric_entry_tests; +#[cfg(test)] +#[path = "widgets/tests_slider_reset.rs"] +mod slider_reset_tests; + fn focused_slider_key() -> egui::Id { egui::Id::new("lightcraft-focused-slider") } diff --git a/crates/ui-egui/src/widgets/tests_slider_reset.rs b/crates/ui-egui/src/widgets/tests_slider_reset.rs new file mode 100644 index 000000000..091768ec7 --- /dev/null +++ b/crates/ui-egui/src/widgets/tests_slider_reset.rs @@ -0,0 +1,36 @@ +//! Real egui pointer frames on a disabled slider: its name doesn't reset it (issue #1436). +use super::{SliderOut, slider, take_registry}; +use crate::headless::HeadlessView; +use egui::{Event, Modifiers, PointerButton, Rect, vec2}; +use lightcraft_develop::controls; + +fn frame(view: &mut HeadlessView, time: f64, enabled: bool, events: Vec) -> (SliderOut, Rect) { + let spec = controls::find("light.exposure").unwrap(); + let mut out = SliderOut::default(); + view.run(HeadlessView::raw_input(vec2(330.0, 100.0), 1.0, time, events), |ui| { + out = slider(ui, spec, 1.25, enabled, None); + }); + let name = take_registry(&view.ctx).into_iter().find(|(id, _)| id == "sliderLabel:light.exposure").unwrap().1; + (out, name) +} + +#[test] +fn a_disabled_sliders_name_ignores_a_double_click() { + for enabled in [true, false] { + let mut view = HeadlessView::new(); + let mut time = 0.0; + let (_, name) = frame(&mut view, time, enabled, vec![]); + let pos = name.left_center() + vec2(8.0, 0.0); + let mut reset = false; + for pressed in [None, Some(true), Some(false), Some(true), Some(false), None] { + time += 0.02; + let events = match pressed { + None => vec![Event::PointerMoved(pos)], + Some(pressed) => vec![Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }], + }; + let (out, _) = frame(&mut view, time, enabled, events); + reset |= out.reset || out.value.is_some(); + } + assert_eq!(reset, enabled, "enabled: {enabled}"); + } +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index ee05a4f55..ff90e3ec3 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -1,6 +1,6 @@ # Lightroom parity checklist -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (catalog import: persistent result, offline Local promotion and library-switch dismissal; integration batch 35: KEYC-SELECTONLYACTIVE, Select Only Active Photo ⌘⇧D; LR-BEHAV-AUTOSAVE, damaged snapshots repaired on open; LR-EDIT-OPTICS-PROFILE, Nikon Z 6II / Z 8 / Z f in-file distortion; LRC-LIB-CATALOG-IMPORT, WAL-only catalogs open; LR-EXP-DIM, enlargements stop at 512 MP; LR-IMP-CAMERA-COVERAGE, Canon EOS RP CR3 verified; model folder retry) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (KEY-SLIDER-RESET: a slider's name and thumb say a double-click resets it, one undo step (#1436); catalog import: persistent result, offline Local promotion and library-switch dismissal; integration batch 35: KEYC-SELECTONLYACTIVE, Select Only Active Photo ⌘⇧D; LR-BEHAV-AUTOSAVE, damaged snapshots repaired on open; LR-EDIT-OPTICS-PROFILE, Nikon Z 6II / Z 8 / Z f in-file distortion; LRC-LIB-CATALOG-IMPORT, WAL-only catalogs open; LR-EXP-DIM, enlargements stop at 512 MP; LR-IMP-CAMERA-COVERAGE, Canon EOS RP CR3 verified; model folder retry) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-14 · **Change:** minor (integration batch 35: KEYC-SELECTONLYACTIVE, Select Only Active Photo ⌘⇧D; LR-BEHAV-AUTOSAVE, damaged snapshots repaired on open; LR-EDIT-OPTICS-PROFILE, Nikon Z 6II / Z 8 / Z f in-file distortion; LRC-LIB-CATALOG-IMPORT, WAL-only catalogs open; LR-EXP-DIM, enlargements stop at 512 MP; LR-IMP-CAMERA-COVERAGE, Canon EOS RP CR3 verified; model folder retry; LR-IMP-FORMATS: preview-only raws rechecked after an update, #662) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 > **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (LR-VIEW-ZOOM: full-size slider-drag drafts on the GPU, timed with their frame, issue #330; rapid slider entry; LR-PROF-CAMERACOLOR: tone curve from black-and-white camera JPEGs; LR-VIEW-ZOOM, LR-VIEW-COMPARE: Compare and Reference zoom windows; LR-IMP-FORMATS, LR-IMP-CAMERA-COVERAGE: EOS R5 Mark II CR3 decodes; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md; filesystem history retry/Undo/Redo evidence; renamed from `parity.md`; Top gaps moved to gaps.md, shortcut notes to ui-parity.md) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 @@ -673,7 +673,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | KEY-CROP-OVERLAYORIENT | Crop overlay orientation — ⇧O | P1 | ✅ | `cmd:view.cropOverlayOrientation` | while cropping: O cycles the overlay, ⇧O its orientation | | KEY-CROP-RESET | Reset crop — ⌥⌘R | P0 | ✅ | `cmd:crop.reset` | | | KEY-STRAIGHTEN | Straighten while held — ⌘ drag | P1 | ✅ | `cmd:crop.straighten`, `crates/ui-egui/src/panels/detail.rs` (`straighten_overlay`) | ⌘-drag in Crop draws a level line; the crop tool stays active | -| KEY-SLIDER-RESET | Reset slider — double-click | P0 | ✅ | `crates/ui-egui/src/widgets.rs` | | +| KEY-SLIDER-RESET | Reset slider — double-click | P0 | ✅ | `crates/ui-egui/src/widgets.rs`, `crates/ui-egui/src/tests_slider_reset.rs` | double-click a slider's name or thumb: back to its default as one undo step (a click on the thumb no longer moves it first, which made it two); resting the pointer on either says so (“Exposure — double-click to reset it”, as the curve channels), the value keeps “Click to type a value” (issue #1436); a disabled slider's name doesn't reset it. No per-slider reset button (Lightroom has none); no ⌥ panel headings turning into Reset buttons yet (`cmd:develop.resetSection` is command-only) | | KEY-SLIDER-NUDGE | Nudge slider — ↑/↓ | P1 | ✅ | `crates/ui-egui/src/widgets.rs` (`nudged`), `cmd:develop.adjust` | ↑/↓ over any slider: ≈ 1/200 of its range (exposure 0.05, most sliders 1, temperature 50 K); ⇧ ×5; one undo step each. Scrolling sideways over any slider (trackpad swipe, tilting wheel, ⇧+wheel) nudges it the same way, one wheel notch a step, without selecting it first; only a mostly sideways movement counts, so scrolling the panel leaves sliders alone (`crates/ui-egui/src/tests_masking.rs`, `sideways_scrolling_adjusts_the_slider_under_the_pointer`) | | KEY-SLIDER-TYPE | Type a slider value | P1 | ✅ | `crates/ui-egui/src/widgets.rs` (`typed_value`), `crates/ui-egui/src/text_field.rs`, `crates/ui-egui/src/widgets/tests_numeric_entry.rs` (`opening_release_honors_return_and_escape_immediately`), `crates/ui-egui/src/tests_keymap.rs` (`opening_numeric_release_keeps_keys_in_the_editor`) | click the value next to any slider's name and type one (`1.5`, `+0,25`, `-20`, `5600 K`): Return or clicking away applies it on the control's steps and range as one undo step, Esc keeps the old value; issue #322; Return/Escape take effect on the opening release, retaining the stored value on Escape; typing stays in the editor without triggering global shortcuts | | KEY-SHORTCUTS | Shortcut list — ⌘/ | P1 | ✅ | `cmd:app.shortcuts` | | @@ -817,6 +817,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-13 | minor | KEYC-RATING: backtick cycles the flag (`photo.cycleFlag`), ⌘↑ / ⌘↓ raise / lower it (`photo.increaseFlag`, `photo.decreaseFlag`); status unchanged (🟡) | | 2026-10-13 | patch | LR-MASK-LUMRANGE / LR-MASK-COLORRANGE: range drags keep the grabbed endpoint; colour samples use the render's enabled sections and HDR/SDR settings; regression coverage, status unchanged | | 2026-10-13 | minor | LR-IMP-DEVICE: Settings → Import → Cameras and Cards can open the import review when a camera or card is connected; status unchanged (✅) | +| 2026-10-13 | minor | KEY-SLIDER-RESET: hovering a slider's name or thumb says a double-click resets it; a double-click on the thumb is one undo step, not two; a disabled slider's name doesn't reset it (issue #1436); status unchanged (✅) | | 2026-10-11 | minor | LR-MERGE-HDR: HDR and panorama merges of raws without colour matrices (CR2, NEF, ARW…) write the colour they open with as DNG tags (issue #965); status unchanged | | 2026-10-12 | minor | LR-META-XMP: Read Metadata from File no longer auto-writes the sidecar it has just read, which added `lc:settings` and hid another app's later `crs:` edits (#934); status unchanged (✅) | | 2026-10-12 | patch | LR-IMP-CAMERA-COVERAGE: CR2 white level from the active area, not the saturated masked rows above it (issue #1272); status unchanged (🟡) | diff --git a/docs/whats-new.md b/docs/whats-new.md index 89a3b9672..27cf0ed6b 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -283,6 +283,12 @@ ## 2026-10-13 +### Resetting a slider +- Resting the pointer on a slider's name or its thumb now says that a double-click resets it ("Exposure — double-click + to reset it"), in every language (issue #1436). +- A double-click on the thumb is a single step in History and Undo. Its first click no longer nudges the value to + where the pointer landed, and a single click on the thumb leaves the value alone. + ### Slider drags on large raws - Dragging a slider no longer re-renders the edited photo's filmstrip thumbnail on every frame; it follows once you let go. On a 64 MP raw the first of those renders decoded the whole file again, which held up the loupe's