diff --git a/crates/ui-egui/src/color_picker.rs b/crates/ui-egui/src/color_picker.rs index cd24dbc0e..947bffb47 100644 --- a/crates/ui-egui/src/color_picker.rs +++ b/crates/ui-egui/src/color_picker.rs @@ -41,6 +41,9 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) let area = egui::Area::new(id.with("area")).order(egui::Order::Foreground).fixed_pos(pos + vec2(0.0, 4.0)).show(ui.ctx(), |ui| { egui::Frame::popup(ui.style()).show(ui, |ui| out = color_picker(ui, id, c, original)); }); + // Open this pass: dialogs leave Escape to the picker (see `open_recently`). + let pass = ui.ctx().cumulative_pass_nr(); + ui.data_mut(|d| d.insert_temp(open_pass_id(), pass)); if ui.input(|i| i.key_pressed(egui::Key::Escape)) { out.close = Some(false); } @@ -60,6 +63,17 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) out.changed } +fn open_pass_id() -> egui::Id { + egui::Id::new("color-picker-open-pass") +} + +/// A colour picker was open in this pass or the last one. Escape then belongs to the picker (it +/// goes back to the original colour and closes), not to the dialog it was opened from. +pub fn open_recently(ctx: &egui::Context) -> bool { + let pass: Option = ctx.data(|d| d.get_temp(open_pass_id())); + pass.is_some_and(|p| p.saturating_add(1) >= ctx.cumulative_pass_nr()) +} + /// What the picker did this frame. #[derive(Default)] struct Outcome { diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 2b0a0f4f1..1386da491 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1608,7 +1608,8 @@ fn handle_shortcuts_impl(app: &mut EffectcraftApp, ctx: &egui::Context, tab_only let start = modifiers_at_start(ctx); // Dialog cancellation owns Escape even when a text field has keyboard focus. if !tab_only && app.dialog.is_some() && ctx.input(|i| i.key_pressed(egui::Key::Escape)) { - if crate::panels::shortcut_editor::recording(app) || egui::Popup::is_any_open(ctx) { + // A popup or colour picker takes Escape first (the picker goes back to its original colour). + if crate::panels::shortcut_editor::recording(app) || egui::Popup::is_any_open(ctx) || crate::color_picker::open_recently(ctx) { return; } ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)); diff --git a/crates/ui-egui/src/panels/gradient_editor.rs b/crates/ui-egui/src/panels/gradient_editor.rs index e2b92a5ce..bb6923025 100644 --- a/crates/ui-egui/src/panels/gradient_editor.rs +++ b/crates/ui-egui/src/panels/gradient_editor.rs @@ -378,14 +378,12 @@ pub fn show(app: &mut EffectcraftApp, ctx: &egui::Context, t: &Tokens) { Track::Color => { ui.label(tr("Color:")); let c = app.dialog_state.gradient.colors.get(i).map_or([1.0; 4], |s| s.value); - // egui's colour button: its picker is a popup that works inside this modal, and - // Escape closes the picker first (dialogs ignore Escape while a popup is open). - // sRGB like the swatches; the stop keeps its alpha. - let mut col = to_color32([c[0], c[1], c[2], 1.0]); - let swatch = egui::color_picker::color_edit_button_srgba(ui, &mut col, egui::color_picker::Alpha::Opaque); + // The shared colour picker (sRGB, hex field, OK / Cancel); the stop keeps its alpha. + let mut rgb = [c[0], c[1], c[2]]; + let swatch = crate::widgets::srgb_color_button(ui, &mut rgb); app.auto.add("dialog.gradient.color", swatch.rect, "Color"); if swatch.changed() { - app.dialog_state.gradient.set_rgb(id, [col.r() as f32 / 255.0, col.g() as f32 / 255.0, col.b() as f32 / 255.0]); + app.dialog_state.gradient.set_rgb(id, rgb); } } } diff --git a/crates/ui-egui/tests/ui_gradient_editor.rs b/crates/ui-egui/tests/ui_gradient_editor.rs index 72a241548..4e9e95565 100644 --- a/crates/ui-egui/tests/ui_gradient_editor.rs +++ b/crates/ui-egui/tests/ui_gradient_editor.rs @@ -10,6 +10,7 @@ use effectcraft_engine::project::{LayerId, PropGroup}; use effectcraft_engine::time::Tick; use effectcraft_ui_egui::panels::gradient_editor::Track; use effectcraft_ui_egui::{Dialog, EffectcraftApp}; +use egui::accesskit::Role; use egui::{Event, Key, Modifiers, PointerButton, Pos2, pos2}; use egui_kittest::Harness; use egui_kittest::kittest::{NodeT, Queryable}; @@ -130,6 +131,27 @@ fn add_key(h: &mut Harness<'_, EffectcraftApp>, l: u64, colors: u64, comp_t: f64 .unwrap(); } +fn near(a: [f32; 4], b: [f32; 4]) -> bool { + a.iter().zip(b).all(|(x, y)| (x - y).abs() < 1e-4) +} + +/// The shared colour picker is showing (its colour field is in the accessibility tree). +fn picker_open(h: &Harness<'_, EffectcraftApp>) -> bool { + h.query_by_label("Color field").is_some() +} + +/// Type `hex` into the open picker's Hex field, over the code it shows. +fn type_hex(h: &mut Harness<'_, EffectcraftApp>, hex: &str) { + let at = h.get_by_role_and_label(Role::TextInput, "#").rect().center(); + click_at(h, at); + for pressed in [true, false] { + h.input_mut().events.push(Event::Key { key: Key::A, physical_key: None, pressed, repeat: false, modifiers: Modifiers::COMMAND }); + } + h.step(); + h.input_mut().events.push(Event::Text(hex.into())); + h.run_steps(3); +} + fn stop_color(h: &Harness<'_, EffectcraftApp>, id: u64) -> [f32; 4] { h.state().gradient_draft().colors.iter().find(|s| s.id == id).expect("the stop").value } @@ -156,23 +178,22 @@ fn stop_edits_ok_sets_the_gradient_in_one_undo_step() { let (track, id) = h.state().gradient_draft().selected.expect("the new stop is selected"); assert_eq!(track, Track::Color); let sampled = stop_color(&h, id); - // The swatch opens the colour picker (a new layer over the modal); a click in its - // saturation/value square, below the current-colour strip, picks a new colour. - let layers = h.ctx.memory(|m| m.areas().visible_layer_ids()); + // The swatch opens the shared colour picker over the modal editor. A click in its colour field + // reaches it (a modal blocks the layers below it), and so does typing a hex code. click(&mut h, "dialog.gradient.color"); - assert!(egui::Popup::is_any_open(&h.ctx), "the colour picker opened"); - let popup = h.ctx.memory(|m| m.areas().visible_layer_ids().into_iter().filter(|l| !layers.contains(l)).filter_map(|l| m.area_rect(l.id)).next()); - let popup = popup.expect("the picker's layer"); - let sp = h.ctx.global_style().spacing.clone(); - let p = pos2(popup.min.x + 8.0 + sp.slider_width * 0.8, popup.min.y + 8.0 + sp.interact_size.y + sp.item_spacing.y + sp.slider_width * 0.3); - click_at(&mut h, p); - // Escape closes the picker, not the editor (its edits stay). - key(&mut h, Key::Escape); - assert!(!egui::Popup::is_any_open(&h.ctx), "the picker closed"); - assert_eq!(h.state().dialog, Some(Dialog::GradientEditor), "Escape closed only the picker"); + assert!(picker_open(&h), "the colour picker opened"); + let field = h.get_by_label("Color field").rect().center(); + click_at(&mut h, field + egui::vec2(20.0, -20.0)); + assert_ne!(stop_color(&h, id), sampled, "a click in the picker's colour field changed the stop"); + type_hex(&mut h, "336699"); let picked = stop_color(&h, id); - assert_ne!(picked, sampled, "the picker changed the stop's colour"); - assert_eq!(picked[3], sampled[3], "and kept its alpha"); + assert!(near(picked, [0.2, 0.4, 0.6, sampled[3]]), "the hex code's colour, and the stop kept its alpha: {picked:?}"); + // Clicking outside the picker keeps the colour and closes it; the editor stays. + let title = h.get_by_label("Gradient Editor").rect().center(); + click_at(&mut h, title); + assert!(!picker_open(&h), "the picker closed"); + assert_eq!(h.state().dialog, Some(Dialog::GradientEditor)); + assert_eq!(stop_color(&h, id), picked); let from = center(&h, "dialog.gradient.opacityStop.1"); let to = along(&h, "dialog.gradient.opacityStop.1", 0.5); drag(&mut h, from, to); @@ -416,3 +437,35 @@ fn edit_gradient_by_id_rejects_bad_targets() { h.run_steps(2); assert_eq!(h.state().dialog, Some(Dialog::GradientEditor)); } + +/// In the editor, the colour picker's Escape and Cancel go back to the stop's colour and close only +/// the picker: the editor and its other edits stay (dialogs leave Escape to an open picker). +#[test] +fn picker_escape_and_cancel_restore_the_stop_and_keep_the_editor() { + let (mut h, _, colors) = setup(&[]); + open_editor(&mut h, colors); + let added = along(&h, "dialog.gradient.addOpacity", 0.3); + click_at(&mut h, added); + click(&mut h, "dialog.gradient.colorStop.0"); + let id = h.state().gradient_draft().selected.expect("a selected stop").1; + let original = stop_color(&h, id); + for how in ["escape", "cancel"] { + click(&mut h, "dialog.gradient.color"); + assert!(picker_open(&h), "{how}: the picker opened"); + type_hex(&mut h, "ff8000"); + assert!(near(stop_color(&h, id), [1.0, 128.0 / 255.0, 0.0, original[3]]), "{how}: the picker previews as it goes"); + if how == "escape" { + key(&mut h, Key::Escape); + } else { + // The picker's Cancel, not the editor's. + let editor_cancel = rect(&h, "dialog.gradient.cancel"); + let cancel = + h.query_all_by_label("Cancel").map(|n| n.rect()).find(|r| (r.min.x - editor_cancel[0]).abs() > 1.0 || (r.min.y - editor_cancel[1]).abs() > 1.0); + click_at(&mut h, cancel.expect("the picker's Cancel").center()); + } + assert!(!picker_open(&h), "{how}: the picker closed"); + assert_eq!(h.state().dialog, Some(Dialog::GradientEditor), "{how}: the editor stays open"); + assert_eq!(stop_color(&h, id), original, "{how}: the stop's colour is back"); + assert_eq!(h.state().gradient_draft().opacities.len(), 3, "{how}: the editor's other edits stay"); + } +}