From 92c7237c50cb6eecf07296d2f2f9640ccc2ccb10 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 18:05:19 +0000 Subject: [PATCH] Fix #322: type an exact value into a slider Click the value next to a slider's name to edit it: the old value starts selected, Return or clicking away applies the typed number on the control's steps and range as one undo step (like a keyboard nudge), and Esc or text that isn't a number keeps the old value. A decimal comma, a leading + and a % or K unit are accepted. The keys typed go to the field, not to the shortcuts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01912WaK6Rmew3wxZuPyX7E8 --- crates/ui-egui/src/headless.rs | 31 ++++++++++++ crates/ui-egui/src/widgets.rs | 91 +++++++++++++++++++++++++++++++--- docs/parity.md | 1 + docs/whats-new.md | 4 ++ 4 files changed, 121 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 976ecd55a..2ebcceb7c 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1053,6 +1053,37 @@ mod tests { assert_ne!(h.app.session.active(), Some(first), "advanced"); } + /// Click a slider's value and type one (issue #322): Return sets it as one undo step, Esc + /// keeps the old value, and the keys typed don't reach the shortcuts (1 = one star). + #[test] + fn slider_values_can_be_typed() { + let mut h = demo([1300.0, 900.0]); + let t = Duration::from_secs(10); + h.request("ui.set", json!({"view": "detail", "right": "edit"}), t); + h.settle(SETTLE); + let active = h.app.session.active().unwrap(); + let exposure = |h: &Headless| h.app.session.develop_of(h.app.session.active().unwrap()).unwrap().light.exposure; + let rating = |h: &Headless| h.app.session.catalog.photo(active).unwrap().rating; + let (undo0, rating0) = (h.app.session.undo.len(), rating(&h)); + assert_eq!(h.request("ui.clickWidget", json!({"id": "sliderValue:light.exposure"}), t)["ok"], true); + h.request("ui.text", json!({"text": "1,5"}), t); + h.request("ui.key", json!({"key": "Enter"}), t); + assert!((exposure(&h) - 1.5).abs() < 1e-9, "{}", exposure(&h)); + assert_eq!(h.app.session.undo.len(), undo0 + 1, "one undo step"); + assert_eq!(rating(&h), rating0, "typing 1 set no rating"); + // Esc keeps the value + assert_eq!(h.request("ui.clickWidget", json!({"id": "sliderValue:light.exposure"}), t)["ok"], true); + h.request("ui.text", json!({"text": "-2"}), t); + h.request("ui.key", json!({"key": "Escape"}), t); + assert!((exposure(&h) - 1.5).abs() < 1e-9, "{}", exposure(&h)); + // something that isn't a number changes nothing + assert_eq!(h.request("ui.clickWidget", json!({"id": "sliderValue:light.exposure"}), t)["ok"], true); + h.request("ui.text", json!({"text": "bright"}), t); + h.request("ui.key", json!({"key": "Enter"}), t); + assert!((exposure(&h) - 1.5).abs() < 1e-9, "{}", exposure(&h)); + assert_eq!(h.app.session.undo.len(), undo0 + 1); + } + /// Return commits a tool panel back to Edit; elsewhere it does nothing. #[test] fn return_commits_the_crop_tool() { diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index d89a9ce32..b6554dd53 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -142,8 +142,26 @@ pub struct SliderOut { pub reset: bool, } +/// What a value typed into a slider asks for (issue #322): a number such as `0.5`, `+12`, `-1,5` +/// (comma or point) or `5600 K`, on the control's steps and clamped to its range. `None` when it +/// isn't a finite number, so the slider keeps its value. +pub fn typed_value(spec: &ControlSpec, text: &str) -> Option { + let t = text.trim().replace(',', "."); + let t = t.trim_end_matches(|c: char| c == '%' || c == 'K' || c == 'k' || c.is_whitespace()); + let v: f64 = t.parse().ok().filter(|v: &f64| v.is_finite())?; + let step = spec.step.max(1e-9); + Some(((v / step).round() * step).clamp(spec.min, spec.max)) +} + +/// A slider's value as shown next to its label. +fn shown_value(spec: &ControlSpec, v: f64) -> String { + let shown = if spec.id == "wb.temp" { format!("{v:.0}") } else { spec.format(v).replace("+0.00", "0").replace("-0.00", "0") }; + if shown == "+0" || shown == "-0" { "0".to_string() } else { shown } +} + /// 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. +/// 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. 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(); @@ -159,12 +177,53 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ resp.widget_info(|| egui::WidgetInfo::slider(enabled, value, label_text.clone())); let label_resp = ui.interact(label_rect, id.with("label"), Sense::click()); 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); + let value_resp = ui.interact(value_rect, id.with("value"), if enabled { Sense::click() } else { Sense::hover() }); + register(ui.ctx(), format!("sliderValue:{}", spec.id), value_rect); + let typing_id = id.with("typing"); + let field_id = id.with("typingField"); + let mut typing: Option = ui.data(|m| m.get_temp(typing_id)); + if enabled && typing.is_none() && value_resp.clicked() { + let text = shown_value(spec, value).trim_start_matches('+').to_string(); + // the old value starts selected, so typing replaces it + let mut state = egui::text_edit::TextEditState::load(ui.ctx(), field_id).unwrap_or_default(); + let all = egui::text::CCursorRange::two(egui::text::CCursor::new(0), egui::text::CCursor::new(text.chars().count())); + state.cursor.set_char_range(Some(all)); + state.store(ui.ctx(), field_id); + typing = Some(text); + ui.memory_mut(|m| m.request_focus(field_id)); + } let mut out = SliderOut::default(); let span = (spec.max - spec.min).max(1e-9); let to_x = |v: f64| track_rect.left() + ((v - spec.min) / span).clamp(0.0, 1.0) as f32 * track_rect.width(); let from_x = |x: f32| spec.min + ((x - track_rect.left()) / track_rect.width()).clamp(0.0, 1.0) as f64 * span; let mut v = value; - if resp.double_clicked() || label_resp.double_clicked() { + if let Some(mut text) = typing.take() { + let field = egui::TextEdit::singleline(&mut text) + .id(field_id) + .font(t.font(12.5)) + .horizontal_align(egui::Align::RIGHT) + .desired_width(value_rect.width()) + .margin(egui::Margin::ZERO); + let te = ui.put(value_rect, field); + if te.lost_focus() || !enabled { + // Esc (or the slider turning off) keeps the old value; Return or clicking away applies + let cancelled = !enabled || ui.input(|i| i.key_pressed(egui::Key::Escape)); + if let Some(nv) = typed_value(spec, &text).filter(|_| !cancelled) { + if (nv - value).abs() > 1e-12 { + out.value = Some(nv); + out.drag_started = true; + out.drag_stopped = true; + v = nv; + } + } + ui.data_mut(|m| m.remove::(typing_id)); + } else { + typing = Some(text.clone()); + ui.data_mut(|m| m.insert_temp(typing_id, text)); + } + } else if resp.double_clicked() || label_resp.double_clicked() { out.reset = true; out.value = Some(spec.default); v = spec.default; @@ -202,7 +261,7 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ out.drag_stopped = true; } // ↑ / ↓ while the pointer rests on the row nudge the value (⇧: five times as much) - if enabled && out.value.is_none() && ui.rect_contains_pointer(row) { + if enabled && out.value.is_none() && typing.is_none() && ui.rect_contains_pointer(row) { let (up, down, shift) = ui.input_mut(|i| { let shift = i.modifiers.shift; let m = if shift { egui::Modifiers::SHIFT } else { egui::Modifiers::NONE }; @@ -224,9 +283,12 @@ pub fn slider(ui: &mut Ui, spec: &ControlSpec, value: f64, enabled: bool, label_ let text_c = if enabled { t.text_label } else { t.text_disabled }; let p = ui.painter(); p.text(label_rect.left_center(), Align2::LEFT_CENTER, crate::i18n::tr(label_override.unwrap_or(spec.label)), t.font(12.5), text_c); - let shown = if spec.id == "wb.temp" { format!("{v:.0}") } else { spec.format(v).replace("+0.00", "0").replace("-0.00", "0") }; - let shown = if shown == "+0" || shown == "-0" { "0".to_string() } else { shown }; - p.text(label_rect.right_center(), Align2::RIGHT_CENTER, shown, t.font(12.5), text_c); + if typing.is_none() { + p.text(label_rect.right_center(), Align2::RIGHT_CENTER, shown_value(spec, v), t.font(12.5), text_c); + if enabled && value_resp.hovered() { + ui.ctx().set_cursor_icon(egui::CursorIcon::Text); + } + } let ring = 7.0; let tx = to_x(v); paint_track(ui, track_rect, &spec.track, &t, tx, ring); @@ -468,6 +530,23 @@ mod tests { assert_eq!(nudged(&spec("wb.temp", 2000.0, 50000.0, 1.0), 6500.0, 1.0), 6550.0); assert_eq!(nudged(&spec("light.contrast", -100.0, 100.0, 1.0), 99.0, 5.0), 100.0, "clamped"); } + + /// Typed values (issue #322): signs, a decimal comma, units; on the control's steps, in range. + #[test] + fn typed_values_are_stepped_and_clamped() { + let exposure = lightcraft_develop::controls::find("light.exposure").unwrap(); + let contrast = lightcraft_develop::controls::find("light.contrast").unwrap(); + let temp = lightcraft_develop::controls::find("wb.temp").unwrap(); + assert_eq!(typed_value(exposure, "1.5"), Some(1.5)); + assert_eq!(typed_value(exposure, " +0,25 "), Some(0.25)); + assert_eq!(typed_value(exposure, "-12"), Some(exposure.min), "clamped"); + assert_eq!(typed_value(contrast, "33.4"), Some(33.0), "on the control's steps"); + assert_eq!(typed_value(contrast, "-20 %"), Some(-20.0)); + assert_eq!(typed_value(temp, "5600 K"), Some(5600.0)); + for bad in ["", "abc", "1.2.3", "NaN", "inf", "--1"] { + assert_eq!(typed_value(contrast, bad), None, "{bad:?}"); + } + } } #[cfg(test)] diff --git a/docs/parity.md b/docs/parity.md index d5a31ea9c..f36a7fef1 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -694,6 +694,7 @@ From `06-shortcuts.md` part 1. Evidence is our binding; conflicts are explained | 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-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`) | 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 | | KEY-SHORTCUTS | Shortcut list — ⌘/ | P1 | ✅ | `cmd:app.shortcuts` | | | KEY-HELP | Help — F1 | P2 | ✅ | `cmd:app.help` | | | KEY-VIDEO-PLAY | Play/pause video — Space | P1 | ⬜ | | | diff --git a/docs/whats-new.md b/docs/whats-new.md index ea0243f42..1fb0a23be 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -13,6 +13,10 @@ with a counterpart here come along, the rest is listed. - 23 new built-in presets: Portrait, Landscape, Urban, Food, Seasons, Vintage and B&W toners. +### Editing +- Type an exact value into any slider (issue #322): click the number next to its name, type (`1.5`, `-20`, `5600`) + and press Return; Esc keeps the old value. + ### Reliability - LightCraft no longer crashes at launch on Windows PCs whose Vulkan driver is broken (issue #136, e.g. some Intel UHD 630 drivers): on Windows the window and GPU rendering use DirectX 12 only and never load the Vulkan driver unless