diff --git a/crates/ui-egui/src/tests_masking.rs b/crates/ui-egui/src/tests_masking.rs index 703911702..ff4b9647e 100644 --- a/crates/ui-egui/src/tests_masking.rs +++ b/crates/ui-egui/src/tests_masking.rs @@ -1638,7 +1638,7 @@ fn a_selected_develop_slider_accepts_wheel_nudges() { // 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); - // Exceed a full notch after cancelling the previous positive fractional remainder. + // 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(); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index efae3454d..5fef361fa 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -240,9 +240,21 @@ 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() }); - // Keyboard focus survives leaving the row; wheel edits require a fresh click or drag. - let wheel_selected_id = id.with("wheel-selected"); + // 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)); @@ -251,10 +263,16 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ resp.request_focus(); ui.data_mut(|d| { d.insert_temp(focused_slider_key(), id); - d.insert_temp(wheel_selected_id, true); + d.insert_temp(wheel_slider_key(), id); + d.remove::(wheel_id); }); - } else if !enabled || !resp.has_focus() || (!pointer_in_row && !resp.is_pointer_button_down_on()) { - ui.data_mut(|d| d.remove::(wheel_selected_id)); + wheel_armed = true; + } else if wheel_armed && (!enabled || !resp.has_focus() || ((!pointer_in_row || left_row) && !resp.is_pointer_button_down_on())) { + 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(); @@ -475,7 +493,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 @@ -493,10 +511,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(|m| m.get_temp::(wheel_selected_id)).unwrap_or(false) { - ui.data_mut(|m| m.remove::(wheel_id)); - } else if enabled && typing.is_none() && out.value.is_none() && pointer_in_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 { @@ -1019,6 +1035,11 @@ fn focused_slider_key() -> egui::Id { egui::Id::new("lightcraft-focused-slider") } +/// 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 { diff --git a/crates/ui-egui/src/widgets/tests_numeric_entry.rs b/crates/ui-egui/src/widgets/tests_numeric_entry.rs index ceb6fa414..c35dbc78b 100644 --- a/crates/ui-egui/src/widgets/tests_numeric_entry.rs +++ b/crates/ui-egui/src/widgets/tests_numeric_entry.rs @@ -258,3 +258,28 @@ fn leaving_the_row_during_a_drag_keeps_dragging_but_release_disarms_the_wheel() } assert_eq!(h.value, released, "releasing outside disarms subsequent wheel edits"); } + +#[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 02f5bf08f..73ae79b76 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-VIEW-ZOOM: only a full-size drag within 20 ms clears a slow-drag verdict) · **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`), `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-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 | 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 | | 2026-10-15 | minor | LR-VIEW-ZOOM: a slow slider-drag verdict needs two drags over 20 ms and expires after 30 s (doubling to 240 s); hidden-window frame times are ignored; the slow set forgets its oldest entry, not all (issue #330); status unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index 581904622..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 @@ -233,12 +243,6 @@ ## 2026-10-13 -### Scrolling past editing sliders -- Leaving a selected slider disarms its vertical mouse-wheel adjustment (issue #1355). 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. Sideways scrolling - still adjusts the slider under the pointer without selecting it first. - ### Edits keep up in a large library - A small edit (a keyword, a rating, a flag, a slider) no longer has the grid's list worked out again from every photo: only the photos the edit touched are asked about again and put at their place in the order. In a library of