mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 10:09:01 -06:00
KEY-SLIDER-NUDGE: disarm the wheel on any move out of the row (#1355)
Review follow-ups: - Check every PointerMoved / PointerGone of the frame, not only where the pointer ended: a slow frame merged leaving the row and coming back, so the slider stayed armed (new test for both events). - One global armed slider (`wheel_slider_key`) instead of a bool per slider: unarmed sliders no longer write context data every frame, and arming another slider disarms the previous one. The wheel remainder is cleared on arming and disarming, so the second focus check is gone. - The sideways branch uses `pointer_in_row` too. - Document that Tab focus arms the arrows only; move the whats-new entry to 2026-10-21; update the KEY-SLIDER-NUDGE row, header and history; fix a test comment that described the old remainder carry-over. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
f8978a0b93
commit
868de3a9fe
@@ -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();
|
||||
|
||||
@@ -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::<egui::Id>(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::<f32>(wheel_id);
|
||||
});
|
||||
} else if !enabled || !resp.has_focus() || (!pointer_in_row && !resp.is_pointer_button_down_on()) {
|
||||
ui.data_mut(|d| d.remove::<bool>(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::<egui::Id>(wheel_slider_key());
|
||||
d.remove::<f32>(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::<f32>(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::<bool>(wheel_selected_id)).unwrap_or(false) {
|
||||
ui.data_mut(|m| m.remove::<f32>(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 {
|
||||
|
||||
@@ -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:?})");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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 |
|
||||
|
||||
+10
-6
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user