diff --git a/crates/ui-egui/src/dialogs/color_picker.rs b/crates/ui-egui/src/dialogs/color_picker.rs index bb6344c35..0169993d5 100644 --- a/crates/ui-egui/src/dialogs/color_picker.rs +++ b/crates/ui-egui/src/dialogs/color_picker.rs @@ -307,6 +307,10 @@ fn new_original_chips(ui: &mut Ui, new: &Color, original: &Color) -> bool { /// The HSB and RGB fields with the channel radios, hex, Lab and CMYK. Returns the edited colour /// and the newly chosen channel. +/// +/// HSB, RGB and hex are in one grid, and Lab and CMYK are in a second grid beside it. Tab goes +/// through the fields in the order they are drawn: H, S, B, R, G, B, hex, then L, a, b, C, M, Y, +/// K, with a stop at each channel's radio button before its field. fn value_fields(ui: &mut Ui, p: &Picked, channel: Channel) -> (Option, Option) { let t = Tokens::get(ui.ctx()); let rgb = p.color.to_rgb(); @@ -315,59 +319,72 @@ fn value_fields(ui: &mut Ui, p: &Picked, channel: Channel) -> (Option, O let cmyk = p.color.to_cmyk_managed(cms.settings().intent); let mut edit = None; let mut chosen = None; - egui::Grid::new("cp-values").num_columns(4).spacing([6.0, 4.0]).show(ui, |ui| { - for i in 0..7 { - if let Some(&ch) = Channel::ALL.get(i) { - if widgets::radio(ui, ch.label(), ch == channel, true) { - chosen = Some(ch); + let spacing = vec2(6.0, 4.0); + ui.horizontal_top(|ui| { + ui.spacing_mut().item_spacing.x = spacing.x; + let left = egui::Grid::new("cp-values").spacing(spacing).show(ui, |ui| { + for i in 0..7 { + if let Some(&ch) = Channel::ALL.get(i) { + if widgets::radio(ui, ch.label(), ch == channel, true) { + chosen = Some(ch); + } + let (v, suffix) = match i { + 0 => (p.hsb[0], "°"), + 1 | 2 => (p.hsb[i] * 100.0, "%"), + _ => (rgb[i - 3] * 255.0, ""), + }; + if let Some(nv) = widgets::plain_field(ui, ("cp-left", i), v as f64, suffix, 0, VALUE_W) { + let nv = nv as f32; + edit = Some(match i { + 0..3 => { + let mut hsb = p.hsb; + hsb[i] = if i == 0 { nv.clamp(0.0, 360.0) } else { (nv / 100.0).clamp(0.0, 1.0) }; + Picked::from_hsb(hsb) + } + _ => { + let mut c = rgb; + c[i - 3] = (nv / 255.0).clamp(0.0, 1.0); + Picked::of(Color::rgb(c[0], c[1], c[2])) + } + }); + } + } else if let Some(c) = widgets::hex_field(ui, "cp-hex", &hex_digits(&p.color)).as_deref().and_then(parse_hex) { + edit = Some(Picked::of(c)); } - let (v, suffix) = match i { - 0 => (p.hsb[0], "°"), - 1 | 2 => (p.hsb[i] * 100.0, "%"), - _ => (rgb[i - 3] * 255.0, ""), + ui.end_row(); + } + }); + egui::Grid::new("cp-values-right").spacing(spacing).show(ui, |ui| { + for i in 0..7 { + let (label, v, suffix) = match i { + 0 => ("L", lab.l, ""), + 1 => ("a", lab.a, ""), + 2 => ("b", lab.b, ""), + _ => (["C", "M", "Y", "K"][i - 3], cmyk[i - 3] * 100.0, "%"), }; - if let Some(nv) = widgets::plain_field(ui, ("cp-left", i), v as f64, suffix, 0, VALUE_W) { + // The hex field is taller than a value field. K's row takes the height of the hex + // field's row beside it. + let height = if i == 6 { left.response.rect.bottom() - ui.cursor().top() } else { ui.spacing().interact_size.y }; + let size = vec2(ui.available_size_before_wrap().x, height); + ui.allocate_ui_with_layout(size, egui::Layout::left_to_right(egui::Align::Center), |ui| { + ui.add_space(12.0); + ui.label(egui::RichText::new(label).size(12.5).color(t.text)); + }); + if let Some(nv) = widgets::plain_field(ui, ("cp-right", i), v as f64, suffix, 0, VALUE_W) { let nv = nv as f32; - edit = Some(match i { - 0..3 => { - let mut hsb = p.hsb; - hsb[i] = if i == 0 { nv.clamp(0.0, 360.0) } else { (nv / 100.0).clamp(0.0, 1.0) }; - Picked::from_hsb(hsb) - } - _ => { - let mut c = rgb; - c[i - 3] = (nv / 255.0).clamp(0.0, 1.0); - Picked::of(Color::rgb(c[0], c[1], c[2])) - } - }); + edit = Some(Picked::of(if i < 3 { + let mut l = [lab.l, lab.a, lab.b]; + l[i] = if i == 0 { nv.clamp(0.0, 100.0) } else { nv.clamp(-128.0, 127.0) }; + cms.from_lab(Lab::new(l[0], l[1], l[2])) + } else { + let mut k = cmyk; + k[i - 3] = (nv / 100.0).clamp(0.0, 1.0); + Color::cmyk(k[0], k[1], k[2], k[3]) + })); } - } else if let Some(c) = widgets::hex_field(ui, "cp-hex", &hex_digits(&p.color)).as_deref().and_then(parse_hex) { - edit = Some(Picked::of(c)); + ui.end_row(); } - let (label, v, suffix) = match i { - 0 => ("L", lab.l, ""), - 1 => ("a", lab.a, ""), - 2 => ("b", lab.b, ""), - _ => (["C", "M", "Y", "K"][i - 3], cmyk[i - 3] * 100.0, "%"), - }; - ui.horizontal(|ui| { - ui.add_space(12.0); - ui.label(egui::RichText::new(label).size(12.5).color(t.text)); - }); - if let Some(nv) = widgets::plain_field(ui, ("cp-right", i), v as f64, suffix, 0, VALUE_W) { - let nv = nv as f32; - edit = Some(Picked::of(if i < 3 { - let mut l = [lab.l, lab.a, lab.b]; - l[i] = if i == 0 { nv.clamp(0.0, 100.0) } else { nv.clamp(-128.0, 127.0) }; - cms.from_lab(Lab::new(l[0], l[1], l[2])) - } else { - let mut k = cmyk; - k[i - 3] = (nv / 100.0).clamp(0.0, 1.0); - Color::cmyk(k[0], k[1], k[2], k[3]) - })); - } - ui.end_row(); - } + }); }); (edit, chosen) } @@ -441,6 +458,37 @@ mod tests { crate::panels::current_paints(app).0.color().unwrap().to_hex() } + /// Runs one frame of the dialog with `events` in `ctx`, which keeps the keyboard focus from + /// frame to frame, and returns the texts drawn with their screen rects. + fn frame_in(ctx: &egui::Context, app: &mut VectorcraftApp, events: Vec) -> Vec<(String, Rect)> { + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| super::super::show(app, ui.ctx())); + out.textures_delta.clear(); + fn collect(s: &egui::Shape, out: &mut Vec<(String, Rect)>) { + match s { + egui::Shape::Text(t) => out.push((t.galley.text().to_string(), t.visual_bounding_rect())), + 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 + } + + /// Returns the label of the value field that has the keyboard, which is the letter (or `#`) + /// nearest to the field's left on its row, and the field's rect. Returns `None` when no text + /// field has the keyboard. + fn focused_field(ctx: &egui::Context, texts: &[(String, Rect)]) -> Option<(String, Rect)> { + let id = ctx.memory(|m| m.focused()).filter(|&id| egui::TextEdit::load_state(ctx, id).is_some())?; + let field = ctx.read_response(id)?.rect; + let labels = ["H", "S", "B", "R", "G", "#", "L", "a", "b", "C", "M", "Y", "K"]; + let (label, _) = texts + .iter() + .filter(|(t, r)| labels.contains(&t.as_str()) && r.max.x <= field.min.x && field.y_range().contains(r.center().y)) + .max_by(|a, b| a.1.max.x.total_cmp(&b.1.max.x))?; + Some((label.clone(), field)) + } + #[test] fn field_and_slider_map_back_per_channel() { let p = Picked::of(Color::rgb8(51, 153, 204)); @@ -558,6 +606,38 @@ mod tests { assert_eq!(crate::panels::current_paints(&app).0.color(), Some(Color::cmyk(0.0, 1.0, 1.0, 0.0))); } + /// #1692: Tab goes through one color model's fields before the next: H, S, B, then R, G, B, the + /// hex field, then L, a, b, then C, M, Y, K. Each Lab or CMYK field is level with the field on + /// its left. + #[test] + fn tab_goes_through_one_color_models_fields_before_the_next() { + let mut app = app(); + app.run("ui.colorPicker", json!({})).unwrap(); + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx); + let tab = egui::Event::Key { key: egui::Key::Tab, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::NONE }; + // The picker opens with the keyboard in H. The loop presses Tab until K has the keyboard + // and records each stop. A stop is `None` when no text field has the keyboard, as at a + // channel's radio button. + let mut texts = vec![]; + for _ in 0..3 { + texts = frame_in(&ctx, &mut app, vec![]); + } + let mut stops = vec![focused_field(&ctx, &texts)]; + while stops.len() < 40 && stops.last().and_then(|s| s.as_ref()).is_none_or(|(label, _)| label != "K") { + texts = frame_in(&ctx, &mut app, vec![tab.clone()]); + stops.push(focused_field(&ctx, &texts)); + } + let names: Vec> = stops.iter().map(|s| s.as_ref().map(|(label, _)| label.as_str())).collect(); + let fields: Vec<&str> = names.iter().flatten().copied().collect(); + assert_eq!(fields, ["H", "S", "B", "R", "G", "B", "#", "L", "a", "b", "C", "M", "Y", "K"], "{names:?}"); + assert_eq!(names[names.len() - 2..], [Some("Y"), Some("K")], "one Tab takes Y to K"); + let found: Vec<&(String, Rect)> = stops.iter().flatten().collect(); + for (left, right) in found[..7].iter().zip(&found[7..]) { + assert_eq!(left.1.center().y, right.1.center().y, "{} and {} are level", left.0, right.0); + } + } + #[test] fn every_mode_draws_and_cmyk_documents_get_cmyk() { let mut app = app();