Shapes: the Gradient Editor uses the shared colour picker; Escape closes only the picker (#612)

The stop colour now uses widgets::srgb_color_button, the After Effects-style
picker every colour input opens (#638): hex field, HSB/RGB fields, OK and
Cancel, sRGB as the project stores colours. The stop keeps its alpha.

The dialog Escape handler only stepped aside for egui popups, and this
picker isn't one, so in a modal dialog Escape with the picker open closed
the dialog too. The picker now marks the pass it was open in
(color_picker::open_recently), and the handler leaves Escape to it: Escape
restores the colour the picker opened with and closes only the picker.

Tests: ui_gradient_editor clicks inside the picker's colour field over the
modal editor, types a hex code, closes the picker by clicking outside it
and saves the stop; a new test checks that Escape and the picker's Cancel
restore the stop's colour and keep the editor and its other edits. That
test fails without the Escape change. ui_color_hex still passes.
This commit is contained in:
tylerjenningsw committed 2026-10-10 23:43:40 -04:00
1 parent ec1bc8cc99
commit 5c1484b6e7
4 files changed
+88 -22

No files matched your search

+14
View File
@@ -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<u64> = 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 {
+2 -1
View File
@@ -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));
+4 -6
View File
@@ -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);
}
}
}
+68 -15
View File
@@ -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");
}
}