Shapes: a closed colour picker leaves the next Escape to the dialog (#612)

The picker marked every pass it drew, including the one it closed in, so
for one pass after closing the dialog Escape handler still stepped aside
and a quick second Escape did nothing. The mark is now set only while the
picker stays open and cleared in the pass it closes (OK, Cancel, Escape or
a click outside); the picker also consumes the Escape it handles.

Test: the_escape_after_the_picker_closes_cancels_the_editor presses Escape
to close the picker and again in the very next pass, with no settling
frames: the editor cancels. It fails on the previous marking.
This commit is contained in:
tylerjenningsw committed 2026-10-11 08:29:54 -04:00
1 parent 5c1484b6e7
commit 3df7e8f77c
2 files changed
+34 -6

No files matched your search

+12 -6
View File
@@ -41,10 +41,8 @@ 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)) {
// Escape is the picker's: consume it, so nothing later this pass takes it too.
if ui.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) {
out.close = Some(false);
}
if pressed_outside(ui.ctx(), &area.response) {
@@ -58,7 +56,13 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3])
ui.data_mut(|d| {
d.insert_temp(id.with("open"), false);
d.remove::<[f32; 3]>(orig_id);
// Closed: the next Escape is the dialog's again.
d.remove::<u64>(open_pass_id());
});
} else {
// Still open after 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));
}
out.changed
}
@@ -67,8 +71,10 @@ 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.
/// A colour picker is open: it was drawn, and stayed open, in this pass or the last one (so one
/// that stops being drawn without closing, its dialog gone, doesn't count for long). Escape then
/// belongs to the picker (it goes back to the original colour and closes), not to the dialog it
/// was opened from. Closing clears this at once, so the next Escape reaches the dialog.
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())
@@ -469,3 +469,25 @@ fn picker_escape_and_cancel_restore_the_stop_and_keep_the_editor() {
assert_eq!(h.state().gradient_draft().opacities.len(), 3, "{how}: the editor's other edits stay");
}
}
/// The Escape that closes the picker is its own: the next Escape, even in the very next pass,
/// cancels the editor (a closed picker doesn't keep the dialog from it).
#[test]
fn the_escape_after_the_picker_closes_cancels_the_editor() {
let (mut h, _, colors) = setup(&[]);
let project = h.state().session.project.clone();
open_editor(&mut h, colors);
click(&mut h, "dialog.gradient.colorStop.0");
click(&mut h, "dialog.gradient.color");
assert!(picker_open(&h), "the picker opened");
let escape = |h: &mut Harness<'_, EffectcraftApp>| {
h.input_mut().events.push(Event::Key { key: Key::Escape, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE });
h.input_mut().events.push(Event::Key { key: Key::Escape, physical_key: None, pressed: false, repeat: false, modifiers: Modifiers::NONE });
h.step();
};
escape(&mut h);
assert_eq!(h.state().dialog, Some(Dialog::GradientEditor), "the first Escape closed only the picker");
escape(&mut h);
assert_eq!(h.state().dialog, None, "the next pass's Escape cancels the editor");
assert_eq!(h.state().session.project, project, "and changes nothing");
}