From 0f38f507ee9dff7f8e2737dfeb13e577cf5d451c Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:10:49 +0300 Subject: [PATCH] ui-egui: a click that opens the colour picker no longer closes it in the same frame (#1476) --- crates/ui-egui/src/color_picker.rs | 14 ++++++++++++-- crates/ui-egui/tests/ui_color_hex.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/color_picker.rs b/crates/ui-egui/src/color_picker.rs index 35d8e4eb..efaae6d7 100644 --- a/crates/ui-egui/src/color_picker.rs +++ b/crates/ui-egui/src/color_picker.rs @@ -45,7 +45,13 @@ pub fn color_popup(ui: &mut Ui, id: egui::Id, pos: egui::Pos2, c: &mut [f32; 3]) 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) { + // A press in the pass that first draws the picker must not close it: an Area's rect is only + // at its requested position from its second pass on (the first pass reports it at the + // origin), so the click that just opened the picker would be read as a press outside it and + // the picker would flash and close in that one frame (#1476). + let pass_id = id.with("open-pass"); + let drawn_before = ui.data(|d| d.get_temp::(pass_id)).is_some_and(|p| p < ui.ctx().cumulative_pass_nr()); + if drawn_before && pressed_outside(ui.ctx(), &area.response) { out.close.get_or_insert(true); } if let Some(keep) = out.close { @@ -56,13 +62,17 @@ 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); + d.remove::(pass_id); // Closed: the next Escape is the dialog's again. d.remove::(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)); + ui.data_mut(|d| { + d.insert_temp(pass_id, pass); + d.insert_temp(open_pass_id(), pass); + }); } out.changed } diff --git a/crates/ui-egui/tests/ui_color_hex.rs b/crates/ui-egui/tests/ui_color_hex.rs index 90eb19b8..4f8a8ab1 100644 --- a/crates/ui-egui/tests/ui_color_hex.rs +++ b/crates/ui-egui/tests/ui_color_hex.rs @@ -114,6 +114,32 @@ fn is_open(h: &Harness<'_, [f32; 3]>) -> bool { h.query_by_role_and_label(Role::TextInput, "#").is_some() } +/// #1476: the click that opens the picker must not close it. The picker's area only reports its +/// real position from its second pass — on the pass it first draws, its rect is at the origin — +/// so a click on a swatch outside that stale rect was read as a press outside the popup and the +/// picker flashed and closed in the frame it opened. +#[test] +fn a_click_that_opens_the_picker_does_not_close_it_in_the_same_frame() { + let mut h = Harness::builder().with_size(vec2(560.0, 400.0)).build_ui_state( + |ui, c: &mut [f32; 3]| { + // Away from the top-left corner: the stale first-pass rect covers it. + ui.allocate_space(vec2(440.0, 120.0)); + ui.horizontal(|ui| { + ui.add_space(440.0); + widgets::srgb_color_button(ui, c); + }); + }, + [1.0, 0.0, 0.0], + ); + h.run_steps(2); + let button = h.get_by_role(Role::ColorWell).rect().center(); + h.input_mut().events.push(Event::PointerMoved(button)); + h.input_mut().events.push(Event::PointerButton { pos: button, button: egui::PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE }); + h.input_mut().events.push(Event::PointerButton { pos: button, button: egui::PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(2); + assert!(is_open(&h), "the click that opens the picker must not close it"); +} + #[test] fn the_dialog_colour_button_stores_srgb() { let mut h = picker([1.0, 1.0, 1.0]);