diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 36cb4edbd..ff4b9647e 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -1631,6 +1631,36 @@ fn a_selected_develop_slider_accepts_wheel_nudges() { } let after = lightcraft_develop::controls::get(&develop(&h), "light.exposure").expect("exposure"); assert!(after > before, "selected exposure did not increase: {before} -> {after}"); + for _ in 0..40 { + h.step(); + } + let before_leaving = develop(&h).light.exposure; + // Leaving the row disarms wheel edits, but must not take keyboard arrows away. + h.request("ui.move", json!({"x": panel.left() - 10.0, "y": track.center().y}), T); + h.request("ui.hoverWidget", json!({"id": name}), T); + // More than a full notch: disarming dropped the wheel remainder, so this alone would edit it. + h.request("ui.scroll", json!({"dy": -240.0}), T); + for _ in 0..40 { + h.step(); + } + assert_eq!(develop(&h).light.exposure, before_leaving, "returning to a slider while scrolling must not edit it (issue #1355)"); + h.request("ui.move", json!({"x": panel.left() - 10.0, "y": track.center().y}), T); + h.request("ui.key", json!({"key": "ArrowUp"}), T); + assert!(develop(&h).light.exposure > before_leaving, "keyboard adjustment survives leaving the slider"); + + // Scroll back to the top, then explicitly select it again to re-enable wheel nudges. + h.request("ui.move", json!({"x": panel.right() - 8.0, "y": panel.center().y}), T); + h.request("ui.scroll", json!({"dy": 600.0}), T); + for _ in 0..40 { + h.step(); + } + click(&mut h, name); + let before_reselecting = develop(&h).light.exposure; + h.request("ui.scroll", json!({"dy": 120.0}), T); + for _ in 0..40 { + h.step(); + } + assert!(develop(&h).light.exposure > before_reselecting, "a new click re-enables wheel adjustment"); } /// Scrolling sideways over a slider adjusts it without selecting it first: a trackpad swipe diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 31de9daf4..e2b2b3500 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -255,16 +255,45 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ let track_rect = Rect::from_min_max(pos2(row.left() + pad_l, row.top() + 22.0), pos2(row.right() - pad_r, row.top() + 40.0)); let id = ui.id().with(spec.id); let resp = ui.interact(track_rect.expand2(vec2(8.0, 2.0)), id, if enabled { Sense::click_and_drag() } else { Sense::hover() }); - // A click selects the slider for arrow/wheel adjustments. Keep scroll-to-adjust - // opt-in so hovering an unfocused slider still scrolls the containing panel. - // Tab reaching the slider selects it as a click does: the arrow keys then adjust it + // Keyboard focus survives leaving the row; vertical wheel edits need a fresh click or drag + // in it (at most one slider is armed: `wheel_slider_key`). + let wheel_id = id.with("wheel-rem"); + let mut wheel_armed = ui.data(|d| d.get_temp::(wheel_slider_key())) == Some(id); + let pointer_in_row = ui.rect_contains_pointer(row); + // Every move this frame counts, not only where the pointer ended up: a slow frame (a slider + // drag, soft proofing) can merge leaving the row and coming back into one. + let left_row = wheel_armed + && ui.input(|i| { + i.events.iter().any(|e| match e { + egui::Event::PointerMoved(p) => !row.contains(*p), + egui::Event::PointerGone => true, + _ => false, + }) + }); + // Tab reaching the slider selects it for arrow keys, not vertical wheel edits. if enabled && resp.gained_focus() { ui.data_mut(|d| d.insert_temp(focused_slider_key(), id)); } - if enabled && (resp.clicked() || resp.drag_started()) { + let selected = enabled && (resp.clicked() || resp.drag_started()); + if selected { resp.request_focus(); ui.data_mut(|d| d.insert_temp(focused_slider_key(), id)); } + // A drag that leaves the row disarms too (it keeps dragging: that needs no wheel selection), + // and one that only starts once the pointer is outside doesn't arm it again. + if selected && pointer_in_row && !left_row { + ui.data_mut(|d| { + d.insert_temp(wheel_slider_key(), id); + d.remove::(wheel_id); + }); + wheel_armed = true; + } else if wheel_armed && (!enabled || !resp.has_focus() || !pointer_in_row || left_row) { + ui.data_mut(|d| { + d.remove::(wheel_slider_key()); + d.remove::(wheel_id); + }); + wheel_armed = false; + } // screen readers: a slider named after its control, with its value let label_text = crate::i18n::tr(label_override.unwrap_or(spec.label)).to_string(); resp.widget_info(|| egui::WidgetInfo::slider(enabled, value, label_text.clone())); @@ -464,7 +493,7 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ }) }); // Arrow keys work with either a hovered or a focused slider. - if enabled && out.value.is_none() && typing.is_none() && (ui.rect_contains_pointer(row) || resp.has_focus() || inc || dec) { + if enabled && out.value.is_none() && typing.is_none() && (pointer_in_row || resp.has_focus() || inc || dec) { let (up, down, shift) = ui.input_mut(|i| { let shift = i.modifiers.shift; let m = if shift { egui::Modifiers::SHIFT } else { egui::Modifiers::NONE }; @@ -487,7 +516,7 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ // leaves the sliders it passes alone. Right raises the value, as dragging the thumb right does. let side_id = id.with("side-rem"); let (dx, dy) = ui.input(|i| (i.smooth_scroll_delta.x, i.smooth_scroll_delta.y)); - if !enabled || typing.is_some() || out.value.is_some() || !ui.rect_contains_pointer(row) || dx.abs() <= dy.abs() { + if !enabled || typing.is_some() || out.value.is_some() || !pointer_in_row || dx.abs() <= dy.abs() { ui.data_mut(|m| m.remove::(side_id)); } else { // the vertical part would scroll the panel and move the slider out from under the pointer @@ -505,10 +534,8 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ } // Wheel nudges require an explicitly selected slider, unlike regular panel scrolling. // Consume only the vertical component so a trackpad's horizontal scroll still works. - let wheel_id = id.with("wheel-rem"); - if !resp.has_focus() { - ui.data_mut(|m| m.remove::(wheel_id)); - } else if enabled && typing.is_none() && out.value.is_none() && ui.rect_contains_pointer(row) { + // (`wheel_armed` is cleared above with its remainder whenever focus or the pointer leaves.) + if wheel_armed && enabled && typing.is_none() && out.value.is_none() && pointer_in_row { let delta = ui.input_mut(|i| { let dy = i.smooth_scroll_delta.y; if dy != 0.0 { @@ -1031,8 +1058,13 @@ fn focused_slider_key() -> egui::Id { egui::Id::new("lightcraft-focused-slider") } -/// Whether the widget with keyboard focus is a develop slider a click selected. It owns only the -/// arrow keys (and the wheel): every other shortcut still works, as in Lightroom after a drag. +/// The slider a click or drag armed for vertical wheel edits, if any. +fn wheel_slider_key() -> egui::Id { + egui::Id::new("lightcraft-wheel-slider") +} + +/// Whether the widget with keyboard focus is a develop slider selected by a click, drag or Tab. +/// It owns only the arrow keys: every other shortcut still works. Wheel selection is separate. pub fn slider_has_focus(ctx: &egui::Context) -> bool { let focused = ctx.memory(|m| m.focused()); focused.is_some() && focused == ctx.data(|d| d.get_temp::(focused_slider_key())) diff --git a/crates/ui-egui/src/widgets/tests_numeric_entry.rs b/crates/ui-egui/src/widgets/tests_numeric_entry.rs index 403ed7292..eb2e3c9b8 100644 --- a/crates/ui-egui/src/widgets/tests_numeric_entry.rs +++ b/crates/ui-egui/src/widgets/tests_numeric_entry.rs @@ -195,3 +195,134 @@ fn a_long_click_keeps_the_field_open() { h.frame(vec![Event::Text("2".into())]); assert_eq!(h.key(Key::Enter).value, Some(2.0)); } + +#[test] +fn leaving_a_slider_disarms_wheel_edits_until_it_is_selected_again() { + let mut h = Harness::new(); + let inside = h.track.center(); + let outside = egui::pos2(5.0, 80.0); + let button = |pressed| Event::PointerButton { pos: inside, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + let wheel = || Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(0.0, 120.0), + phase: egui::TouchPhase::Move, + modifiers: Modifiers::NONE, + }; + h.frame(vec![Event::PointerMoved(inside)]); + h.frame(vec![button(true)]); + h.frame(vec![button(false)]); + let selected = h.value; + h.frame(vec![wheel()]); + for _ in 0..40 { + h.frame(vec![]); + } + assert!(h.value > selected, "selecting a slider enables wheel nudges"); + h.frame(vec![Event::PointerMoved(outside)]); + let before_return = h.value; + h.frame(vec![Event::PointerMoved(inside), wheel()]); + for _ in 0..40 { + h.frame(vec![]); + } + assert_eq!(h.value, before_return, "scrolling back over a previously selected slider must not edit it"); + h.frame(vec![Event::PointerMoved(outside)]); + let before_arrow = h.value; + h.key(Key::ArrowUp); + assert!(h.value > before_arrow, "leaving must preserve keyboard arrows"); + // Sideways scrolling remains independent of vertical wheel selection (#1465). + let before_sideways = h.value; + h.frame(vec![ + Event::PointerMoved(inside), + Event::MouseWheel { unit: egui::MouseWheelUnit::Point, delta: vec2(120.0, 0.0), phase: egui::TouchPhase::Move, modifiers: Modifiers::NONE }, + ]); + for _ in 0..40 { + h.frame(vec![]); + } + let after_sideways = h.value; + assert!(after_sideways > before_sideways, "a disarmed slider still accepts sideways nudges"); + h.frame(vec![wheel()]); + for _ in 0..40 { + h.frame(vec![]); + } + assert_eq!(h.value, after_sideways, "sideways nudges do not re-arm vertical wheel edits"); + h.frame(vec![Event::PointerMoved(inside)]); + h.frame(vec![button(true)]); + h.frame(vec![button(false)]); + let selected_again = h.value; + h.frame(vec![wheel()]); + for _ in 0..40 { + h.frame(vec![]); + } + assert!(h.value > selected_again, "selecting it again restores wheel nudges"); +} + +#[test] +fn leaving_the_row_during_a_drag_keeps_dragging_but_release_disarms_the_wheel() { + let mut h = Harness::new(); + let inside = h.track.center(); + let outside = egui::pos2(h.track.right() - 20.0, 80.0); + let button = |pos, pressed| Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + h.frame(vec![Event::PointerMoved(inside)]); + h.frame(vec![button(inside, true)]); + let initial = h.value; + h.frame(vec![Event::PointerMoved(outside)]); + assert!(h.value > initial, "an owned drag keeps editing outside the row"); + h.frame(vec![button(outside, false)]); + let released = h.value; + h.frame(vec![ + Event::PointerMoved(inside), + Event::MouseWheel { unit: egui::MouseWheelUnit::Point, delta: vec2(0.0, -120.0), phase: egui::TouchPhase::Move, modifiers: Modifiers::NONE }, + ]); + for _ in 0..40 { + h.frame(vec![]); + } + assert_eq!(h.value, released, "releasing outside disarms subsequent wheel edits"); +} + +#[test] +fn a_drag_that_leaves_and_ends_back_in_the_row_disarms_the_wheel() { + let mut h = Harness::new(); + let inside = h.track.center(); + let outside = egui::pos2(h.track.right() - 20.0, 80.0); + let button = |pos, pressed| Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + h.frame(vec![Event::PointerMoved(inside)]); + h.frame(vec![button(inside, true)]); + h.frame(vec![Event::PointerMoved(outside)]); + h.frame(vec![Event::PointerMoved(inside)]); + h.frame(vec![button(inside, false)]); + let released = h.value; + h.frame(vec![Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(0.0, 120.0), + phase: egui::TouchPhase::Move, + modifiers: Modifiers::NONE, + }]); + for _ in 0..40 { + h.frame(vec![]); + } + assert_eq!(h.value, released, "the drag left the row, so the wheel needs a fresh selection"); +} + +#[test] +fn leaving_and_returning_within_one_frame_still_disarms_the_wheel() { + let wheel = || Event::MouseWheel { + unit: egui::MouseWheelUnit::Point, + delta: vec2(0.0, 120.0), + phase: egui::TouchPhase::Move, + modifiers: Modifiers::NONE, + }; + // a slow frame merges the moves out of the row and back (or out of the window and back) + for away in [Event::PointerMoved(egui::pos2(5.0, 80.0)), Event::PointerGone] { + let mut h = Harness::new(); + let inside = h.track.center(); + let button = |pressed| Event::PointerButton { pos: inside, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + h.frame(vec![Event::PointerMoved(inside)]); + h.frame(vec![button(true)]); + h.frame(vec![button(false)]); + let selected = h.value; + h.frame(vec![away.clone(), Event::PointerMoved(inside), wheel()]); + for _ in 0..40 { + h.frame(vec![]); + } + assert_eq!(h.value, selected, "the pointer left the row between two frames ({away:?})"); + } +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 55bc35bb4..0fbda3fbe 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-20 · **Change:** minor (LR-PRE-PANEL: the applied preset stays marked per photo) · **Target:** Adobe Lightroom Classic 15.5 / Lightroom 9.6 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** patch (KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the row; leaving it disarms them) · **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 @@ -673,7 +673,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | 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`, `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-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`). The vertical wheel nudges only a slider a click or drag selected, while the pointer stays in its row: leaving the row (even within one frame) disarms it until the next click or drag, and Tab focus arms only the arrow keys (issue #1355, `crates/ui-egui/src/widgets/tests_numeric_entry.rs`) | | 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`, `a_long_click_keeps_the_field_open`), `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; the field stays open however long the click is held (a release over the value keeps it, issue #1435) | | KEY-SLIDER-SCRUB | Scrub a slider value — drag the number | P1 | ✅ | `crates/ui-egui/src/widgets.rs` (`slider`, `SCRUB_START`), `crates/ui-egui/src/widgets/tests_numeric_entry.rs` (`dragging_the_value_sideways_scrubs_it_like_the_thumb`, `scrubbing_back_and_with_shift`, `a_press_that_barely_moves_still_types`) | drag the value next to any slider's name sideways: it moves as dragging the thumb would (the pointer's travel over the track's width, ⇧ a tenth as far), on the control's steps and range, as one undo step; a press that moves less than 3 points sideways stays a click that types a value (KEY-SLIDER-TYPE); the pointer over the value is the horizontal-resize cursor, as Lightroom's scrubby value | | KEY-SHORTCUTS | Shortcut list — ⌘/ | P1 | ✅ | `cmd:app.shortcuts` | | @@ -797,6 +797,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | Date | Change | Summary | |---|---|---| +| 2026-10-21 | patch | KEY-SLIDER-NUDGE: vertical wheel nudges need a click or drag in the slider's row; leaving it (even between two frames) or Tab focus alone does not arm them (issue #1355); status unchanged | | 2026-10-20 | minor | LR-PRE-PANEL: the preset applied to a photo is marked again (with its Amount slider) on returning to the photo, until the photo is edited further (#2392); status unchanged (✅) | | 2026-10-20 | patch | LR-VIEW-ZOOM: the 20 ms rule clears a slow-drag verdict only on a drag timed at full size; a drag that drafts small leaves it unchanged, as the code does; status unchanged | | 2026-10-19 | patch | LR-VIEW-ZOOM: the slow slider-drag verdict is described as per photo and control, as the code keys it; status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index cfeed33ed..d9d432ed6 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,15 @@ # What's new in LightCraft +## 2026-10-21 + +### Scrolling past editing sliders +- Leaving a selected slider disarms its vertical mouse-wheel adjustment (issue #1355), even when the pointer leaves + and comes back between two screen updates. Scrolling back over it now scrolls the panel instead of unexpectedly + changing the photo. Click or drag the slider again to adjust it with the vertical wheel; keyboard arrow + adjustments and an ongoing drag still work. A slider reached with Tab takes the arrow keys but not the wheel: + click it to scroll-adjust it. Sideways scrolling still adjusts the slider under the pointer without selecting it + first. + ## 2026-10-19 ### Hidden Local locations come back one at a time