From be0fcb5795db3bda65428601be9470beec6f3f4e Mon Sep 17 00:00:00 2001 From: kolla <78259478+kollakek1@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:15:14 +0300 Subject: [PATCH] fix(ui): keep the style picker open while editing a style's colour The colour picker's Window area is keyed by egui as Id::new(Some(title)), but the style picker looked it up as Id::new("color_picker"), so the lookup always missed and any click in the picker counted as an outside click that closed the editor (and the picker with it). Give the window a fixed id and query it through colorpicker::window_id(), with a regression test. --- crates/ui-egui/src/colorpicker.rs | 9 ++++++++ crates/ui-egui/src/style_picker.rs | 2 +- crates/ui-egui/src/style_picker_tests.rs | 28 ++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/colorpicker.rs b/crates/ui-egui/src/colorpicker.rs index d6aad31d..f6a19f72 100644 --- a/crates/ui-egui/src/colorpicker.rs +++ b/crates/ui-egui/src/colorpicker.rs @@ -435,6 +435,12 @@ fn alpha_slider(ui: &mut Ui, rect: Rect, color: Color) -> Option<(f32, bool)> { slider_knob(ui, rect, color.a, Id::new("cp_alpha")) } +/// The colour picker window's area id. A fixed id (not `Window::new`'s, which egui keys by +/// `Id::new(Some(title))`) so the style picker can tell a click inside the picker from one outside. +pub fn window_id() -> Id { + Id::new("color_picker") +} + pub fn show(app: &mut FigApp, ctx: &egui::Context) { let Some(target) = app.ui.picker.clone() else { return }; // A layer's picker belongs to the selection it was opened for: once that changes, close it @@ -463,6 +469,9 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) { let mut open = true; let mut close_now = false; egui::Window::new("color_picker") + // A fixed area id so the style picker can tell a click in here from a click outside + // (`Window::new` alone would key the area by `Id::new(Some(title))`). + .id(window_id()) .title_bar(false) .fixed_pos(pos) .fixed_size(vec2(240.0, 0.0)) diff --git a/crates/ui-egui/src/style_picker.rs b/crates/ui-egui/src/style_picker.rs index 29c9c5a9..22713000 100644 --- a/crates/ui-egui/src/style_picker.rs +++ b/crates/ui-egui/src/style_picker.rs @@ -909,7 +909,7 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) { // Clicks in the colour picker editing this picker's style, or in a menu the editor opened, // keep it open. let picker_rect = - app.ui.picker.as_ref().filter(|t| matches!(t, PickerTarget::Style { .. })).and(ctx.memory(|m| m.area_rect(Id::new("color_picker")))); + app.ui.picker.as_ref().filter(|t| matches!(t, PickerTarget::Style { .. })).and(ctx.memory(|m| m.area_rect(crate::colorpicker::window_id()))); // So do clicks in the variable picker binding one of its fields (Escape closes that first). let variables = crate::variable_ui::picker_rect(ctx); let in_popup = egui::Popup::is_any_open(ctx) || variables.is_some(); diff --git a/crates/ui-egui/src/style_picker_tests.rs b/crates/ui-egui/src/style_picker_tests.rs index becc42ce..ad2c2971 100644 --- a/crates/ui-egui/src/style_picker_tests.rs +++ b/crates/ui-egui/src/style_picker_tests.rs @@ -164,6 +164,34 @@ fn chars_linked_to_several_styles_show_mixed() { assert_eq!(text_link(&app, &doc, &[t]), None); } +#[test] +fn a_click_in_the_colour_picker_keeps_the_style_editor_open() { + use crate::layers_drag_tests::Ui; + let mut s = Session::new(); + s.execute("style.create", &json!({"type": "paint", "name": "Brand", "value": [{"type": "solid", "color": "#0D99FF"}]})).unwrap(); + let mut u = Ui::new(s); + open_for_selection(&mut u.app, StyleSlot::Fill, Some("Brand")); + for _ in 0..3 { + u.frame(vec![]); + } + let id = u.app.session.doc().unwrap().style("Brand").unwrap().id.clone(); + u.app.ui.picker = Some(crate::colorpicker::PickerTarget::Style { id, index: 0 }); + for _ in 0..3 { + u.frame(vec![]); + } + let cp = u.ctx.memory(|m| m.area_rect(crate::colorpicker::window_id())).expect("colour picker shows"); + let p = cp.center(); + u.button(p, true); + u.button(p, false); + u.frame(vec![]); + assert!(u.app.ui.style_picker.is_some(), "the style editor stays open while the colour picker is used"); + assert!( + matches!(u.app.ui.picker, Some(crate::colorpicker::PickerTarget::Style { .. })), + "the colour picker stays open too: {:?}", + u.app.ui.picker + ); +} + #[test] fn local_style_drops_move_into_the_folder_above() { let st = |id: &str, name: &str| Style {