diff --git a/crates/ui-egui/src/panels.rs b/crates/ui-egui/src/panels.rs index da0aaf90..a776dab4 100644 --- a/crates/ui-egui/src/panels.rs +++ b/crates/ui-egui/src/panels.rs @@ -1996,11 +1996,14 @@ pub fn properties_window(app: &mut PhotocraftApp, ctx: &egui::Context) { ui.add_space(8.0); widgets::hairline(ui); ui.add_space(8.0); - if let LayerContent::Adjustment(adj) = &layer.content { - adjustment_controls(app, ui, id, adj); - } else { - layer_controls(app, ui, &layer); - } + // Per-layer ids, so text still being typed for one layer can't commit to the next. + ui.push_id(id, |ui| { + if let LayerContent::Adjustment(adj) = &layer.content { + adjustment_controls(app, ui, id, adj); + } else { + layer_controls(app, ui, &layer); + } + }); }); }); if drag != egui::Vec2::ZERO && canvas.is_positive() { @@ -2734,3 +2737,47 @@ mod swatch_type_tests { assert_eq!(t.char_runs().len(), 1, "still one white run"); } } + +#[cfg(test)] +mod properties_card_tests { + use super::*; + use egui_kittest::{Harness, kittest::Queryable}; + + /// Arithmetic left uncommitted in one fill layer's card never lands on the layer selected next. + #[test] + fn uncommitted_arithmetic_stays_with_its_layer() { + let mut s = photocraft_engine::Session::new(); + s.execute("file.new", json!({"width": 64, "height": 64})).unwrap(); + let mut ids = Vec::new(); + for color in ["#00ff00", "#0000ff"] { + s.execute("layer.newFillLayer.solidColor", json!({"color": color})).unwrap(); + ids.push(s.active().unwrap().active_layer.unwrap()); + } + // The layer selected next already shows 25, the value the edited one has when `25*` stops it. + s.execute("layer.setProps", json!({"layer": ids[0].0, "opacity": 0.25})).unwrap(); + let mut app = PhotocraftApp::new(s, crate::Services::default()); + app.ui.panels.properties = true; + let mut h = Harness::builder().with_size(vec2(800.0, 600.0)).build_ui_state( + |ui, app: &mut PhotocraftApp| { + if ui.ctx().fonts(|f| f.families().contains(&egui::FontFamily::Name("semibold".into()))) { + properties_window(app, ui.ctx()); + } + }, + app, + ); + PhotocraftApp::setup_context(&h.ctx, crate::theme::ThemeKind::Studio); + h.run_steps(2); + h.query_all_by_role(egui::accesskit::Role::SpinButton).next().unwrap().click(); + h.run_steps(1); + for ch in "25*2".chars() { + h.event(egui::Event::Text(ch.to_string())); + h.run_steps(1); + } + // The Layers panel selects the other layer in the frame the click leaves the field. + h.state_mut().session.execute("layer.select", json!({"layer": ids[0].0})).unwrap(); + h.ctx.memory_mut(|m| m.stop_text_input()); + h.run_steps(3); + let doc = &h.state().session.active().unwrap().doc; + assert_eq!(ids.iter().map(|&id| doc.layer(id).unwrap().opacity).collect::>(), [0.25, 0.25]); + } +} diff --git a/crates/ui-egui/src/prefs_ui.rs b/crates/ui-egui/src/prefs_ui.rs index 94281357..7d30735b 100644 --- a/crates/ui-egui/src/prefs_ui.rs +++ b/crates/ui-egui/src/prefs_ui.rs @@ -986,11 +986,11 @@ fn section_fields(ui: &mut egui::Ui, section: &str, obj: &mut Map let (lo, hi) = prefs::range(&path).unwrap_or((-1e9, 1e9)); if n.is_u64() || n.is_i64() { let mut x = n.as_i64().unwrap_or(0); - ui.add(egui::DragValue::new(&mut x).range(lo as i64..=hi as i64)); + ui.add(egui::DragValue::new(&mut x).range(lo as i64..=hi as i64).custom_parser(crate::widgets::parse_num)); obj.insert(k, json!(x)); } else { let mut x = n.as_f64().unwrap_or(0.0); - ui.add(egui::DragValue::new(&mut x).range(lo..=hi).speed(0.1).max_decimals(3)); + ui.add(egui::DragValue::new(&mut x).range(lo..=hi).speed(0.1).max_decimals(3).custom_parser(crate::widgets::parse_num)); obj.insert(k, json!(x)); } } diff --git a/crates/ui-egui/src/view_cmds.rs b/crates/ui-egui/src/view_cmds.rs index 8cc7d55f..d013e7fc 100644 --- a/crates/ui-egui/src/view_cmds.rs +++ b/crates/ui-egui/src/view_cmds.rs @@ -740,11 +740,11 @@ pub fn form_body(ui: &mut egui::Ui, f: &mut Map) { Value::Number(n) => { ui.label(label_of(&k)); if let Some(mut i) = n.as_i64() { - ui.add(egui::DragValue::new(&mut i)); + ui.add(egui::DragValue::new(&mut i).custom_parser(crate::widgets::parse_num)); json!(i) } else { let mut x = n.as_f64().unwrap_or(0.0); - ui.add(egui::DragValue::new(&mut x).speed(0.5)); + ui.add(egui::DragValue::new(&mut x).speed(0.5).custom_parser(crate::widgets::parse_num)); json!(x) } } diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 3ddce4b7..cab20ca4 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -152,13 +152,8 @@ pub fn value_field(ui: &mut Ui, value: &mut f32, range: std::ops::RangeInclusive ui.style_mut().visuals.widgets.hovered.weak_bg_fill = Color32::TRANSPARENT; ui.style_mut().override_font_id = Some(theme::mono(12.0)); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - ui.add_sized( - field.size(), - egui::DragValue::new(value) - .range(range) - .speed(if fine { 0.01 } else { 0.5 }) - .custom_formatter(|v, _| if fine { fmt_num2(v) } else { fmt_num(v) }), - ) + let layout = egui::Layout::centered_and_justified(ui.layout().main_dir()); + ui.allocate_ui_with_layout(field.size(), layout, |ui| number_edit(ui, value, range, fine)).inner }) .inner } @@ -169,6 +164,36 @@ pub fn value_field(ui: &mut Ui, value: &mut f32, range: std::ops::RangeInclusive resp } +/// The number in a [`value_field`]. A typed number applies as it's typed; arithmetic waits for +/// Enter, Tab or click-away, because per keystroke `5/2` would land first and a caller that +/// rounds it would cut `5/2*2` short. `changed()` means a new value, not just a keystroke. +fn number_edit(ui: &mut Ui, value: &mut f32, range: std::ops::RangeInclusive, fine: bool) -> Response { + let (id, ctx) = (ui.next_auto_id(), ui.ctx().clone()); + let held = id.with("arithmetic"); + let math = ui.memory(|m| m.has_focus(id)) && ui.data(|d| d.get_temp(held)).unwrap_or(false); + ui.data_mut(|d| d.insert_temp(held, math)); + let before = *value; + let mut resp = ui.add( + egui::DragValue::new(value) + .range(range) + .speed(if fine { 0.01 } else { 0.5 }) + .custom_formatter(move |v, _| if fine { fmt_num2(v) } else { fmt_num(v) }) + .update_while_editing(!math) + // Focus is read when parsing, not above: Tab hands it on inside `ui.add`. + .custom_parser(move |s| { + let v = parse_num(s); + if ctx.memory(|m| m.has_focus(id)) && plain(s).is_none() { + // Still typing arithmetic: hold it, and stop applying keystrokes once it parses. + ctx.data_mut(|d| d.insert_temp(held, v.is_some())); + return None; + } + v + }), + ); + resp.flags.set(egui::response::Flags::CHANGED, *value != before); + resp +} + /// Thin-track slider with a round knob. `gradient` paints the track (e.g. hue spectrum). pub fn slider(ui: &mut Ui, value: &mut f32, range: std::ops::RangeInclusive, gradient: Option<&[Color32]>) -> Response { let t = Tokens::get(ui.ctx()); @@ -511,6 +536,47 @@ pub fn fmt_num2(v: f64) -> String { format!("{r:.2}").trim_end_matches('0').trim_end_matches('.').to_string() } +/// Parses a typed number or simple arithmetic such as `1280*2` or `20*2+5-2` (`+ - * /`, +/// `*` and `/` first). Like egui's own parser it ignores whitespace and reads `−` as `-`. +/// `None` for anything else, including a division by zero. +pub fn parse_num(text: &str) -> Option { + let s = clean(text); + s.parse().ok().or_else(|| sum(&s)).filter(|v: &f64| v.is_finite()) +} + +/// A plain typed number, no arithmetic. +fn plain(text: &str) -> Option { + clean(text).parse().ok() +} + +fn clean(text: &str) -> String { + text.chars().filter(|c| !c.is_whitespace()).map(|c| if c == '−' { '-' } else { c }).collect() +} + +/// `a+b-c…`: a `+` or `-` right after an operand splits terms; anywhere else it is a sign. +fn sum(s: &str) -> Option { + let (mut total, mut sign, mut start, mut prev) = (0.0, 1.0, 0, ' '); + for (i, c) in s.char_indices() { + if matches!(c, '+' | '-') && i > start && !matches!(prev, '*' | '/' | 'e' | 'E') { + total += sign * product(s.get(start..i)?)?; + sign = if c == '-' { -1.0 } else { 1.0 }; + start = i + 1; + } + prev = c; + } + Some(total + sign * product(s.get(start..)?)?) +} + +/// `a*b/c…`, left to right. +fn product(s: &str) -> Option { + let mut factors = s.split(['*', '/']).map(str::parse::); + let mut acc = factors.next()?.ok()?; + for (op, x) in s.matches(['*', '/']).zip(factors) { + acc = if op == "*" { acc * x.ok()? } else { acc / x.ok()? }; + } + Some(acc) +} + #[cfg(test)] mod tests { #[test] @@ -521,6 +587,93 @@ mod tests { assert_eq!(super::fmt_num2(2.0), "2"); } + #[test] + fn typed_arithmetic_evaluates() { + use super::parse_num; + for (text, want) in [ + ("1280*2", 2560.0), + ("658 * 1.5", 987.0), + ("48/3", 16.0), + ("20*2+5-2", 43.0), + ("2+3*4", 14.0), + ("10/4*2", 5.0), + ("-5+3", -2.0), + ("5--3", 8.0), + ("2*-3+1", -5.0), + ("−4", -4.0), + ("1 234", 1234.0), + ("1e3/2", 500.0), + ] { + assert_eq!(parse_num(text), Some(want), "{text}"); + } + for text in ["", "abc", "5+", "*2", "4/0", "0/0", "1+*2", "(2+3)", "1e400"] { + assert_eq!(parse_num(text), None, "{text}"); + } + } + + /// Types `text` into a value field holding `start`, then presses `key`. Returns the value an OK + /// button that also fires on Enter saw, as dialogs read it, else the field's value. `round` + /// makes the caller round the value every frame, as pixel fields do. + fn type_and_press(start: f32, text: &str, key: egui::Key, round: bool) -> f32 { + use egui_kittest::{Harness, kittest::Queryable}; + let mut h = Harness::builder().with_size(egui::vec2(300.0, 100.0)).build_ui_state( + move |ui, (v, ok): &mut (f32, Option)| { + super::value_field(ui, v, 0.0..=300000.0, "px", 90.0); + if round { + *v = v.round(); + } + if ui.button("OK").clicked() || ui.input(|i| i.key_pressed(egui::Key::Enter)) { + *ok = Some(*v); + } + }, + (start, None), + ); + h.get_by_role(egui::accesskit::Role::SpinButton).click(); + h.run(); + for c in text.chars() { + h.event(egui::Event::Text(c.to_string())); + h.run(); + } + h.key_press(key); + h.run(); + let (v, ok) = *h.state(); + ok.unwrap_or(v) + } + + /// Plain digits apply as they're typed; arithmetic doesn't report a change until it's committed. + #[test] + fn value_field_holds_arithmetic_until_committed() { + use egui_kittest::{Harness, kittest::Queryable}; + let mut h = Harness::builder().with_size(egui::vec2(300.0, 100.0)).build_ui_state( + |ui, (v, changes): &mut (f32, u32)| { + if super::value_field(ui, v, 0.0..=100.0, "%", 90.0).changed() { + *changes += 1; + } + }, + (100.0, 0), + ); + h.get_by_role(egui::accesskit::Role::SpinButton).click(); + h.run(); + for (c, want) in [('5', (5.0, 1)), ('0', (50.0, 2)), ('/', (50.0, 2)), ('2', (50.0, 2))] { + h.event(egui::Event::Text(c.to_string())); + h.run(); + assert_eq!(*h.state(), want, "after {c}"); + } + h.key_press(egui::Key::Enter); + h.run(); + assert_eq!(*h.state(), (25.0, 3)); + } + + #[test] + fn value_field_applies_typed_arithmetic() { + use egui::Key::{Enter, Tab}; + assert_eq!(type_and_press(500.0, "1280*2", Tab, false), 2560.0); + assert_eq!(type_and_press(500.0, "1280*2", Enter, false), 2560.0); + // `5/2` alone would round to 3 under the caller; the whole expression still applies. + assert_eq!(type_and_press(1.0, "5/2*2", Tab, true), 5.0); + assert_eq!(type_and_press(1.0, "1280/3*2", Enter, true), 853.0); + } + #[test] fn numbers_drop_trailing_zero() { assert_eq!(super::fmt_num(100.0), "100"); diff --git a/docs/scorecard.md b/docs/scorecard.md index 031a9320..bc20446c 100644 --- a/docs/scorecard.md +++ b/docs/scorecard.md @@ -172,7 +172,7 @@ Unread: `general.color_picker`, `general.beep_when_done`, `general.export_clipbo | UI-217-10 | Appearance: animation toggle, canvas pattern toggle per theme, start-screen recents | missing | [#217](https://github.com/storytold/photocraft/issues/217) | No such prefs; start screen (canvas.rs:825) has no recents | | UI-217-11 | Soft dark and soft light themes with picker previews; outlined knobs in light themes | missing | [#217](https://github.com/storytold/photocraft/issues/217) | theme.rs:28 five themes | | UI-217-12 | Save custom New Document presets; a real Recent category | missing | [#217](https://github.com/storytold/photocraft/issues/217) | new_doc_ui.rs:17 'Recent' is a static list | -| UI-217-13 | Numeric fields accept arithmetic and units; scrubby labels; double-click resets a slider | partial | [#217](https://github.com/storytold/photocraft/issues/217) | widgets.rs:131 DragValue scrubs; no expressions, label scrub or reset | +| UI-217-13 | Numeric fields accept arithmetic and units; scrubby labels; double-click resets a slider | partial | [#217](https://github.com/storytold/photocraft/issues/217) | widgets.rs:132 DragValue scrubs and takes + - * / arithmetic; no units in expressions, label scrub or reset | | UI-217-14 | Layers panel: wheel steps blend modes, drop on New/Trash duplicates/deletes, checker behind art | missing | [#217](https://github.com/storytold/photocraft/issues/217) | panels.rs:1108 plain dropdown; trash is click-only (panels.rs:1213) | | UI-217-15 | Curves: smoothing / pencil mode, black/grey/white eyedroppers, tooltips | missing | [#217](https://github.com/storytold/photocraft/issues/217) | adjust_editors.rs:397 has none of them | | UI-217-16 | Hue/Saturation: Saturation and Lightness sliders get hue-following colour tracks | missing | [#217](https://github.com/storytold/photocraft/issues/217) | Only the Hue slider has a gradient | diff --git a/scorecard/ui.toml b/scorecard/ui.toml index 5466e507..9f0b1bfe 100644 --- a/scorecard/ui.toml +++ b/scorecard/ui.toml @@ -94,7 +94,7 @@ id = "UI-217-13" target = "Numeric fields accept arithmetic and units; scrubby labels; double-click resets a slider" status = "partial" issue = 217 -note = "widgets.rs:131 DragValue scrubs; no expressions, label scrub or reset" +note = "widgets.rs:132 DragValue scrubs and takes + - * / arithmetic; no units in expressions, label scrub or reset" [[item]] id = "UI-217-14"