fix(ui): keep colour popups open when "+" opens the custom colour picker (#4066)

* fix(ui): keep colour popups open when "+" opens the custom colour picker

The colour popups used egui's default menu close behaviour (CloseOnClick),
which closes on any click, including the "+" swatch inside the popup. The
picker's open state was remembered, so it appeared only on the next open.

Close these popups only on clicks outside them: the quick bar's colour and
fill popups, the comment context menu's Colour submenu and Format text's
colour popup. Picking a swatch or pressing Apply still closes them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* contributors: credit agent-reed by GitHub profile name

* test(ui): pin the fill popup and context-menu Colour submenu keeping their picker open

The quick bar's Fill colour popup and the comment context menu's Colour
submenu each get a kittest test: "+" shows the picker without closing the
popup, and choosing a colour still closes it. Both fail with the fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

contributors/people.toml

@@ -173,3 +173,6 @@ sync_github_name = true
 
 [people.amoisttowe1]
 real_name = "Nick Nyssen"
+
+[people.agent-reed]
+sync_github_name = true

crates/ui-egui/src/comments.rs

@@ -1873,7 +1873,8 @@ pub(crate) fn context_menu(ui: &mut egui::Ui, view: &mut DocView, info: &DocInfo
                         }
                     }
                 });
-                ui.menu_button(tl!("Colour"), |ui| {
+                let keep_open = egui::containers::menu::MenuConfig::new().close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside);
+                let _ = egui::containers::menu::SubMenuButton::new(tl!("Colour")).config(keep_open).ui(ui, |ui| {
                     if let Some(c) = swatch_grid(ui, a.color.map(|c| c.map(f64::from))) {
                         action = Some(CanvasAction::Edit(Box::new(Edit::StyleAnnotation {
                             page,
@@ -2092,7 +2093,7 @@ pub(crate) fn quick_bar_controls(ui: &mut egui::Ui, tool: CommentTool, prefs: &m
     ui.painter().circle_stroke(r.center(), 9.0, Stroke::new(1.0, Color32::from_black_alpha(50)));
     resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, tl!("Colour")));
     let resp = resp.on_hover_text(tl!("Colour"));
-    egui::Popup::menu(&resp).align(egui::RectAlign::RIGHT_START).show(|ui| {
+    egui::Popup::menu(&resp).align(egui::RectAlign::RIGHT_START).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| {
         if let Some(c) = swatch_grid(ui, Some(style.color)) {
             prefs.set_color(tool, c);
             selected_color = Some(c);
@@ -2101,7 +2102,7 @@ pub(crate) fn quick_bar_controls(ui: &mut egui::Ui, tool: CommentTool, prefs: &m
     });
     if tool.has_fill() {
         let resp = fill_button(ui, style.fill);
-        egui::Popup::menu(&resp).align(egui::RectAlign::RIGHT_START).show(|ui| {
+        egui::Popup::menu(&resp).align(egui::RectAlign::RIGHT_START).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| {
             ui.label(egui::RichText::new(tl!("Fill colour")).font(theme::semibold(12.0)));
             if let Some(f) = fill_picker(ui, style.fill) {
                 prefs.set_fill(tool, f);
@@ -2339,6 +2340,106 @@ mod tests {
         assert_eq!(h.query_all_by_label("Apply").count(), 1, "only the first grid's picker opens");
     }
 
+    #[test]
+    fn other_colour_opens_the_picker_inside_the_quick_bar_popup() {
+        use egui_kittest::kittest::Queryable;
+        let mut h = egui_kittest::Harness::new_ui_state(
+            |ui, prefs: &mut CommentPrefs| {
+                let _ = quick_bar_controls(ui, CommentTool::Highlight, prefs);
+            },
+            CommentPrefs::default(),
+        );
+        h.run();
+        h.get_by_label("Colour").click();
+        h.run();
+        h.get_by_label("Other colour…").click();
+        h.run();
+        assert!(h.query_by_label("Apply").is_some(), "the popup stays open with the picker showing");
+        h.get_by_label("Apply").click();
+        h.run();
+        assert!(h.query_by_label("Other colour…").is_none(), "applying a colour closes the popup");
+    }
+
+    #[test]
+    fn other_colour_opens_the_picker_inside_the_quick_bar_fill_popup() {
+        use egui_kittest::kittest::Queryable;
+        let prefs = CommentPrefs::default();
+        let fill_label = if prefs.style(CommentTool::Rectangle).fill.is_some() { "Fill colour" } else { "Fill colour: none" };
+        let mut h = egui_kittest::Harness::new_ui_state(
+            |ui, prefs: &mut CommentPrefs| {
+                let _ = quick_bar_controls(ui, CommentTool::Rectangle, prefs);
+            },
+            prefs,
+        );
+        h.ctx.set_fonts(theme::font_definitions());
+        h.run();
+        h.get_by_label(fill_label).click();
+        h.run();
+        h.get_by_label("Other colour…").click();
+        h.run();
+        assert!(h.query_by_label("Apply").is_some(), "the popup stays open with the picker showing");
+        h.get_by_label("Apply").click();
+        h.run();
+        assert!(h.query_by_label("No fill").is_none(), "applying a colour closes the popup");
+        assert!(h.state().style(CommentTool::Rectangle).fill.is_some());
+    }
+
+    #[test]
+    fn context_menu_colour_submenu_keeps_its_picker_open_and_a_swatch_closes_the_menu() {
+        use egui_kittest::kittest::Queryable;
+        let info = DocInfo {
+            pages: vec![PageInfo { width: 300.0, height: 400.0, label: String::new(), crop: [0.0, 0.0, 300.0, 400.0], rotation: 0 }],
+            annotations: vec![Annotation {
+                page: 0,
+                subtype: "Square".into(),
+                author: None,
+                contents: None,
+                modified: None,
+                name: None,
+                in_reply_to: None,
+                rect: [10.0, 10.0, 50.0, 50.0],
+                color: None,
+                index: 0,
+                state: None,
+                quads: Vec::new(),
+                locked: false,
+                intent: None,
+                fill_sign: false,
+                points: Vec::new(),
+            }],
+            ..Default::default()
+        };
+        let mut view = DocView::new(pdfcraft_engine::DocId(1), &info, crate::canvas::ViewDefaults::default());
+        view.comments.selected = Some((0, 0));
+        let prefs = CommentPrefs::default();
+        let mut h = egui_kittest::Harness::new_ui_state(
+            |ui, (view, actions): &mut (DocView, Vec<CanvasAction>)| {
+                ui.button("Page").context_menu(|ui| {
+                    if let Some(a) = context_menu(ui, view, &info, &prefs, true) {
+                        actions.push(a);
+                    }
+                });
+            },
+            (view, Vec::new()),
+        );
+        h.run();
+        h.get_by_label("Page").click_secondary();
+        h.run();
+        h.get_by_label("Colour ⏵").click();
+        h.run();
+        h.get_by_label("Other colour…").click();
+        h.run();
+        assert!(h.query_by_label("Apply").is_some(), "the submenu stays open with the picker showing");
+        h.get_by_label("Red").click();
+        h.run();
+        assert!(h.query_by_label("Edit text…").is_none(), "picking a swatch closes the whole menu");
+        let actions = &h.state().1;
+        assert!(
+            matches!(actions.as_slice(), [CanvasAction::Edit(e)] if matches!(**e, Edit::StyleAnnotation { page: 0, index: 0, color: Some(_), .. })),
+            "{actions:?}"
+        );
+    }
+
     #[test]
     fn shift_keeps_the_current_proportions_unless_the_box_has_its_own() {
         let r = Rect::from_min_max(pos2(100.0, 100.0), pos2(196.0, 132.0));

crates/ui-egui/src/format_text_ui.rs

@@ -138,7 +138,7 @@ pub(crate) fn panel(ui: &mut egui::Ui, t: &Tokens, look: &mut AddedText, extras:
             .response
             .on_hover_text(tl!("Font size"));
         let swatch = colour_button(ui, t, look.color);
-        egui::Popup::menu(&swatch).show(|ui| {
+        egui::Popup::menu(&swatch).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| {
             if let Some(picked) = crate::comments::swatch_grid(ui, Some(look.color)) {
                 look.color = picked;
                 ui.close();