From cb8b797cd44040fe889b146e1e05b329902c9690 Mon Sep 17 00:00:00 2001 From: Atmik Shetty Date: Tue, 13 Oct 2026 13:16:08 +0530 Subject: [PATCH] KEY-SLIDER-RESET: a slider's name and thumb say a double-click resets it (fixes #1436) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A double-click on a slider's name or thumb already reset it, but nothing said so: only the value had a tooltip. Resting the pointer on the name or on the thumb now shows "Exposure — double-click to reset it", the wording (and translations) the curve channels already use. The rest of the track, which is clicked and dragged, gets no tooltip, and the value keeps "Click to type a value". The first click of a double-click on the thumb moved the value to where the pointer landed, a few points off its centre: one history step, then the reset another. A click on the thumb now leaves the value alone, so the reset is one undo step, as from the name. A disabled slider's name no longer resets it. No per-slider reset button: Lightroom has none, and the double-click it documents is now shown where it works. Tests: tests_slider_reset.rs (name and thumb reset Exposure and Contrast in one undo step, the hints, a click on the thumb keeps the value, typing a value still works) and widgets/tests_slider_reset.rs (a disabled slider's name). --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/tests_slider_reset.rs | 151 ++++++++++++++++++ crates/ui-egui/src/widgets.rs | 29 +++- .../ui-egui/src/widgets/tests_slider_reset.rs | 36 +++++ docs/parity-checklist.md | 5 +- docs/whats-new.md | 6 + 6 files changed, 223 insertions(+), 6 deletions(-) create mode 100644 crates/ui-egui/src/tests_slider_reset.rs create mode 100644 crates/ui-egui/src/widgets/tests_slider_reset.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 8248ee5a1..4dd6effda 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -110,6 +110,8 @@ mod tests_quit_unsaved; #[cfg(test)] mod tests_scroll; #[cfg(test)] +mod tests_slider_reset; +#[cfg(test)] mod tests_switch_library; #[cfg(test)] mod tests_sync; 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 a6af3dc7a..9a9f86c36 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -176,9 +176,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. Click the value to -/// type one: Return or clicking away applies it, Esc keeps the old one. +/// Double-click the label or thumb to reset (hovering either says so). Shift-drag = fine +/// adjustment. 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()); let w = ui.available_width(); @@ -214,7 +217,12 @@ 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 }; + 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); @@ -241,6 +249,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. @@ -313,6 +327,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); @@ -391,7 +408,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(); @@ -848,6 +865,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 76d91ba3e..ba1babb61 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-11 · **Change:** minor (LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); 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 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (KEY-SLIDER-RESET: a slider's name and thumb say a double-click resets it, one undo step (#1436); LR-MERGE-HDR: merges of raws without colour matrices keep their colour (#965); 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 The measured, row-by-row checklist: one row per Lightroom feature, menu item and shortcut, what we have and what is missing. It answers *does the feature exist?* (feature breadth). Whether it is good @@ -666,7 +666,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 | | 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` | | @@ -789,6 +789,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 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 b5c2ac68c..86e626fe0 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -2,6 +2,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