diff --git a/crates/ui-egui/src/dialogs/swatch_options.rs b/crates/ui-egui/src/dialogs/swatch_options.rs index 6f4a2ed1c..9d6feca10 100644 --- a/crates/ui-egui/src/dialogs/swatch_options.rs +++ b/crates/ui-egui/src/dialogs/swatch_options.rs @@ -252,7 +252,11 @@ fn hex_field(ui: &mut egui::Ui, hex: &str) -> Option { let t = Tokens::get(ui.ctx()); let id = ui.id().with("swatch-hex"); let editing = ui.memory(|m| m.has_focus(id)); - let mut buf: String = if editing { ui.data_mut(|m| m.get_temp::(id)).unwrap_or_else(|| hex.to_string()) } else { hex.to_string() }; + // As in `widgets::hex_field`: until the user types into it, the field shows `hex` as it is now + // (#1692). + let edited_id = id.with("edited"); + let edited = editing && ui.data_mut(|m| m.get_temp::(edited_id)).unwrap_or(false); + let mut buf: String = if edited { ui.data_mut(|m| m.get_temp::(id)).unwrap_or_else(|| hex.to_string()) } else { hex.to_string() }; let resp = egui::Frame::NONE .fill(t.input) .stroke(egui::Stroke::new(1.0, if editing { t.accent } else { t.input_border })) @@ -263,7 +267,11 @@ fn hex_field(ui: &mut egui::Ui, hex: &str) -> Option { ui.add(egui::TextEdit::singleline(&mut buf).id(id).frame(egui::Frame::NONE).desired_width(70.0).char_limit(7)) }) .inner; - ui.data_mut(|m| m.insert_temp(id, buf.clone())); + let focused = ui.memory(|m| m.has_focus(id)); + ui.data_mut(|m| { + m.insert_temp(id, buf.clone()); + m.insert_temp(edited_id, (edited || resp.changed()) && focused); + }); (resp.lost_focus() && buf != hex).then(|| parse_hex(&buf)).flatten() } diff --git a/crates/ui-egui/src/dialogs/tests.rs b/crates/ui-egui/src/dialogs/tests.rs index f60a29d1a..c9570b1fc 100644 --- a/crates/ui-egui/src/dialogs/tests.rs +++ b/crates/ui-egui/src/dialogs/tests.rs @@ -658,6 +658,74 @@ fn a_dialogs_focus_skips_a_greyed_out_first_field() { assert_eq!((info["spacing"].clone(), info["steps"].clone()), (json!("steps"), json!(7)), "{info}"); } +/// #1692: a value typed into the field before the # field of the Color Picker or Swatch Options +/// stays when Tab passes through the # field. The # field takes the focus in the frame the value is +/// committed, still showing the color from before it, and commits nothing when Tab moves on. A hex +/// typed into the # field is still committed by Tab. +#[test] +fn tab_through_the_hex_field_keeps_the_value_typed_before_it() { + let mut app = app(); + app.run("swatch.new", json!({"name": "Sky", "color": "#3366cc"})).unwrap(); + let ctx = egui::Context::default(); + theme::install_fonts(&ctx); + theme::apply(&ctx, Default::default()); + // One frame of the dialog layer: every text drawn, with its center. + let frame = |app: &mut VectorcraftApp, events: Vec| { + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| show(app, ui.ctx())); + out.textures_delta.clear(); + fn collect(s: &egui::Shape, out: &mut Vec<(String, egui::Pos2)>) { + match s { + egui::Shape::Text(t) => out.push((t.galley.text().to_string(), t.visual_bounding_rect().center())), + egui::Shape::Vec(v) => v.iter().for_each(|s| collect(s, out)), + _ => {} + } + } + let mut texts = vec![]; + out.shapes.iter().for_each(|c| collect(&c.shape, &mut texts)); + texts + }; + let key = |key, modifiers| egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }; + let (tab, back) = (key(egui::Key::Tab, egui::Modifiers::NONE), key(egui::Key::Tab, egui::Modifiers::SHIFT)); + let button = |pos, pressed| egui::Event::PointerButton { pos, button: egui::PointerButton::Primary, pressed, modifiers: Default::default() }; + let color = |app: &VectorcraftApp| app.ui.dialog.as_ref().unwrap().fields["color"].clone(); + type Open = fn(&mut VectorcraftApp); + let opens: [(&str, Open, &str); 2] = [ + ("Color Picker", |app| drop(app.run("ui.colorPicker", json!({"color": "#ffffff"}))), "FFFFFF"), + ("Swatch Options", |app| drop(app.run("ui.swatchOptions", json!({"name": "Sky"}))), "3366CC"), + ]; + for (name, open, hex) in opens { + open(&mut app); + let before = color(&app); + for _ in 0..3 { + frame(&mut app, vec![]); + } + let at = frame(&mut app, vec![]).into_iter().find(|(t, _)| t == hex).unwrap_or_else(|| panic!("{name}: no # field shows {hex}")).1; + // Click into the # field, then Shift+Tab to the field before it and type 40 over its value. + frame(&mut app, vec![egui::Event::PointerMoved(at), button(at, true)]); + frame(&mut app, vec![button(at, false)]); + frame(&mut app, vec![back.clone()]); + frame(&mut app, vec![]); + frame(&mut app, vec![egui::Event::Text("40".into())]); + // Tab commits it and gives the # field the focus; the next Tab moves on. + frame(&mut app, vec![tab.clone()]); + let edited = color(&app); + assert_ne!(edited, before, "{name}: the field before # took 40"); + for events in [vec![], vec![tab.clone()], vec![]] { + frame(&mut app, events); + } + assert_eq!(color(&app), edited, "{name}: passing through # put the color from before the edit back"); + // Click into the # field, select its text, type a hex over it and press Tab. + frame(&mut app, vec![egui::Event::PointerMoved(at), button(at, true)]); + frame(&mut app, vec![button(at, false)]); + frame(&mut app, vec![key(egui::Key::A, egui::Modifiers::COMMAND)]); + frame(&mut app, vec![egui::Event::Text("00FF00".into())]); + frame(&mut app, vec![tab.clone()]); + assert!(frame(&mut app, vec![]).iter().any(|(t, _)| t == "00FF00"), "{name}: the # field took 00FF00"); + cancel(&mut app); + frame(&mut app, vec![]); + } +} + /// #1000: a count typed in Object › Create Object Mosaic… reaches the command (a whole number typed /// in a command's dialog is an integer), so the mosaic has that many columns, not the 10 × 10 grid. #[test] diff --git a/crates/ui-egui/src/tests_numfields.rs b/crates/ui-egui/src/tests_numfields.rs index 6358958e4..0c45813e7 100644 --- a/crates/ui-egui/src/tests_numfields.rs +++ b/crates/ui-egui/src/tests_numfields.rs @@ -45,6 +45,135 @@ fn a_focused_field_follows_a_value_changed_from_outside() { assert_eq!(frame(50.0, vec![button(away, false)]).0, None); } +/// #1692: a hex field that the focus only passes through shows the color as it is now and commits +/// nothing. Tab out of an edited field gives the hex field after it the focus in the frame the edit +/// is committed, and the color changes after the hex field is drawn, as in the Color Picker. +/// Shift+Tab out of the field after it gives it the focus a frame later, in the frame that field +/// commits. +#[test] +fn a_hex_field_passed_through_with_tab_commits_nothing() { + let mut f = Field::new(); + // Red and Blue fields on either side of the hex field of the color they make. + let rgb = Cell::new([255.0, 128.0, 0.0]); + let (hex, blue, hex_id) = (Cell::new(None), Cell::new(None), Cell::new(egui::Id::NULL)); + let draw = |ui: &mut egui::Ui| { + let [r, g, b] = rgb.get(); + let red = widgets::plain_field(ui, "red", r, "", 0, 80.0); + hex_id.set(ui.id().with("hex")); + hex.set(widgets::hex_field(ui, "hex", &format!("{:02X}{:02X}{:02X}", r as u8, g as u8, b as u8))); + blue.set(widgets::plain_field(ui, "blue", b, "", 0, 80.0)); + red + }; + // One frame, its edits applied after it. → What the hex field committed. + let step = |f: &mut Field, events: Vec| { + let red = f.frame(events, &draw); + let [r, g, b] = rgb.get(); + rgb.set([red.unwrap_or(r), g, blue.take().unwrap_or(b)]); + hex.take() + }; + // The text the hex field shows (it keeps it under its id). + let shows = |f: &Field| f.ctx.data(|d| d.get_temp::(hex_id.get())); + let (tab, back) = (key(Key::Tab, Modifiers::NONE), key(Key::Tab, Modifiers::SHIFT)); + step(&mut f, vec![]); + // Tab into Red, type 50 over it, and Tab on to the hex field, then on to Blue. + for events in [vec![tab.clone()], vec![Event::Text("50".into())], vec![tab.clone()], vec![]] { + assert_eq!(step(&mut f, events), None); + } + assert_eq!(shows(&f).as_deref(), Some("328000"), "the hex field shows Red's edit"); + for events in [vec![tab.clone()], vec![]] { + assert_eq!(step(&mut f, events), None); + } + assert_eq!(rgb.get(), [50.0, 128.0, 0.0], "Red keeps what was typed"); + // Type 200 over Blue, Shift+Tab back into the hex field and Tab out of it again. + for events in [vec![Event::Text("200".into())], vec![back], vec![], vec![]] { + assert_eq!(step(&mut f, events), None); + } + assert_eq!(shows(&f).as_deref(), Some("3280C8"), "the hex field shows Blue's edit"); + for events in [vec![tab], vec![]] { + assert_eq!(step(&mut f, events), None); + } + assert_eq!(rgb.get(), [50.0, 128.0, 200.0], "Blue keeps what was typed"); +} + +/// What is typed into a hex field is committed when Tab or Enter ends its focus, and Escape drops +/// it. +#[test] +fn typing_into_a_hex_field_commits_it() { + let mut f = Field::new(); + let hex = Cell::new(None); + let draw = |ui: &mut egui::Ui| { + let red = widgets::plain_field(ui, "red", 255.0, "", 0, 80.0); + hex.set(widgets::hex_field(ui, "hex", "FF8000")); + red + }; + let step = |f: &mut Field, events: Vec| { + f.frame(events, &draw); + hex.take() + }; + let tab = || key(Key::Tab, Modifiers::NONE); + let escape = || key(Key::Escape, Modifiers::NONE); + step(&mut f, vec![]); + for (end, commits) in [(tab(), true), (enter(), true), (escape(), false)] { + // Tab into Red and on into the hex field, select its text and type over it. + let mut got = vec![]; + for events in + [vec![tab()], vec![tab()], vec![], vec![key(Key::A, Modifiers::COMMAND)], vec![Event::Text("00FF00".into())], vec![end.clone()], vec![]] + { + got.extend(step(&mut f, events)); + } + let want: Vec = if commits { vec!["00FF00".into()] } else { vec![] }; + assert_eq!(got, want, "{end:?}"); + // Neither field has the focus for the next round. + step(&mut f, vec![escape()]); + } +} + +/// #1692: the Stroke panel's dash and gap fields ([`widgets::opt_field`]) are blank while the +/// selected objects' dashes differ. Tab out of the dash after typing into it gives the gap the +/// focus; the gap then shows the dash's length, which the committed dash repeats as its gap. Tab +/// out of the gap commits nothing, and what is typed into the gap replaces all of that length. +#[test] +fn a_gap_field_passed_through_with_tab_commits_nothing() { + use crate::panels::stroke::{dash_fields, dash_pattern}; + let shown = Cell::new([None; 6]); + let (dash, gap) = (Cell::new(None), Cell::new(None)); + let draw = |ui: &mut egui::Ui| -> Option { + let [d, g, ..] = shown.get(); + dash.set(widgets::opt_field(ui, "dash", d, Unit::Points, 60.0)); + gap.set(widgets::opt_field(ui, "gap", g, Unit::Points, 60.0)); + None + }; + // One frame, then its commit applied the way the Stroke panel applies it. → What the gap + // committed. + let step = |f: &mut Field, events: Vec| { + f.frame(events, &draw); + let (d, g) = (dash.take(), gap.take()); + if d.is_some() || g.is_some() { + let mut fields = shown.get(); + fields[0] = d.unwrap_or(fields[0]); + fields[1] = g.unwrap_or(fields[1]); + shown.set(dash_fields(&dash_pattern(&fields))); + } + g + }; + let tab = || key(Key::Tab, Modifiers::NONE); + for (typed, commits, pattern) in [(None, None, [20.0, 20.0]), (Some("5"), Some(Some(5.0)), [20.0, 5.0])] { + let mut f = Field::new(); + shown.set([None; 6]); + step(&mut f, vec![]); + // Tab into the dash, type 20 and Tab on into the gap. + for events in [vec![tab()], vec![Event::Text("20".into())], vec![tab()], vec![]] { + assert_eq!(step(&mut f, events), None); + } + if let Some(text) = typed { + assert_eq!(step(&mut f, vec![Event::Text(text.into())]), None); + } + assert_eq!(step(&mut f, vec![tab()]), commits, "{typed:?}"); + assert_eq!(step(&mut f, vec![]), None); + assert_eq!(dash_pattern(&shown.get()), pattern, "{typed:?}"); + } +} + /// Headless frames of one field: its rect and id as last drawn. struct Field { ctx: egui::Context, diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 2f3af5e8d..5a7341473 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -1468,13 +1468,19 @@ pub fn channel_slider( } /// A hex colour field (`E67828`, no `#`) after a `#` label. Returns the text on commit when it -/// changed. +/// was typed into the field and differs from `hex`. pub fn hex_field(ui: &mut Ui, id: impl std::hash::Hash + std::fmt::Debug, hex: &str) -> Option { let t = Tokens::get(ui.ctx()); ui.label(egui::RichText::new("#").size(15.0).color(t.text)); let id = ui.id().with(id); let editing = ui.memory(|m| m.has_focus(id)); - let mut buf: String = if editing { ui.data_mut(|d| d.get_temp::(id)).unwrap_or_else(|| hex.to_string()) } else { hex.to_string() }; + // Until the user types into it, the field shows `hex` as it is now, also while it has the + // focus. Tab can give it the focus in the frame another field commits an edit, before the edit + // changes `hex`; the text from before that edit is not committed when the focus moves on + // (#1692). + let edited_id = id.with("edited"); + let edited = editing && ui.data_mut(|d| d.get_temp::(edited_id)).unwrap_or(false); + let mut buf: String = if edited { ui.data_mut(|d| d.get_temp::(id)).unwrap_or_else(|| hex.to_string()) } else { hex.to_string() }; let resp = egui::Frame::NONE .fill(t.input) .stroke(Stroke::new(1.0, if editing { t.accent } else { t.input_border })) @@ -1493,7 +1499,11 @@ pub fn hex_field(ui: &mut Ui, id: impl std::hash::Hash + std::fmt::Debug, hex: & }) .inner; let commit = resp.lost_focus() && buf != hex; - ui.data_mut(|d| d.insert_temp(id, buf.clone())); + let focused = ui.memory(|m| m.has_focus(id)); + ui.data_mut(|d| { + d.insert_temp(id, buf.clone()); + d.insert_temp(edited_id, (edited || resp.changed()) && focused); + }); commit.then_some(buf) } @@ -1569,7 +1579,16 @@ pub fn opt_field(ui: &mut Ui, id: impl std::hash::Hash + std::fmt::Debug, value: let id = ui.id().with(id); let shown = value.map(|v| unit.number(v)).unwrap_or_default(); let editing = ui.memory(|m| m.has_focus(id)); - let mut buf: String = if editing { ui.data_mut(|d| d.get_temp::(id)).unwrap_or_else(|| shown.clone()) } else { shown.clone() }; + // As in `hex_field`: until the user types into it, the field shows `shown` as it is now + // (#1692). When `shown` changes while the field has the focus and nothing has been typed into + // it, all of the new text is selected, as on keyboard focus. + let edited_id = id.with("edited"); + let edited = editing && ui.data_mut(|d| d.get_temp::(edited_id)).unwrap_or(false); + let before = ui.data_mut(|d| d.get_temp::(id)); + let mut buf: String = if edited { before.clone().unwrap_or_else(|| shown.clone()) } else { shown.clone() }; + if editing && !edited && before.is_some_and(|b| b != buf) { + select_all(ui, id, &buf); + } let enabled = ui.is_enabled(); let framed = egui::Frame::NONE .fill(t.input) @@ -1591,7 +1610,11 @@ pub fn opt_field(ui: &mut Ui, id: impl std::hash::Hash + std::fmt::Debug, value: // ↑/↓ and the wheel step a value (an empty field stays empty); a drag on its label scrubs it. let stepped = step_value(ui, &resp, &mut buf, |b| Some(unit.from_pt(unit.parse(b)?)), |v| unit.number(unit.to_pt(v))); let scrubbed = scrub::field(ui, id, framed.response.rect, &buf, value.map(|v| unit.from_pt(v)), STEP_DECIMALS); - ui.data_mut(|d| d.insert_temp(id, buf.clone())); + let focused = ui.memory(|m| m.has_focus(id)); + ui.data_mut(|d| { + d.insert_temp(id, buf.clone()); + d.insert_temp(edited_id, (edited || resp.changed()) && focused); + }); if resp.lost_focus() && buf != shown { let s = buf.trim(); if s.is_empty() { Some(None) } else { typed_unit(ui.ctx(), unit).parse(s).map(Some) }