From e6914861f1479fbafe6eb7fb7f0e86c14169465b Mon Sep 17 00:00:00 2001 From: Michael Gieson Date: Wed, 7 Oct 2026 23:25:09 -0400 Subject: [PATCH] Appearance: the Opacity popup's blend list and slider work egui remembers one open popup at a time, so opening the blend-mode dropdown or the opacity slider inside the Opacity popup closed the popup, and the dropdown or slider with it. A new widgets::popover keeps its own open state and closes only on Escape, its anchor, or a click outside it and the popups it opened. The Opacity popup and the Stroke popover (which holds dropdowns too) use it. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/ui-egui/src/panels/appearance.rs | 17 ++++++------- crates/ui-egui/src/panels/stroke.rs | 2 +- crates/ui-egui/src/panels/tests_appearance.rs | 19 ++++++++++++++ crates/ui-egui/src/widgets.rs | 25 +++++++++++++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/panels/appearance.rs b/crates/ui-egui/src/panels/appearance.rs index 87f440dac..e8db77193 100644 --- a/crates/ui-egui/src/panels/appearance.rs +++ b/crates/ui-egui/src/panels/appearance.rs @@ -668,16 +668,13 @@ fn opacity_row(app: &mut VectorcraftApp, ui: &mut Ui, item: Option, opaci let lx = r.left() + EYE_W + 24.0 + if item.is_some() { 12.0 } else { 0.0 }; let clicked = link(ui, pos2(lx, r.center().y), ("ap-op-link", item), tl!("Opacity:")) || resp.clicked(); text(ui, pos2(lx + 56.0, r.center().y), &opacity_text(opacity, blend), false); - egui::Popup::menu(&resp) - .open_memory(clicked.then_some(egui::SetOpenCommand::Toggle)) - .close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside) - .show(|ui| match widgets::opacity_blend(ui, ("ap-op", item), Some(opacity), Some(blend), true) { - Some(TransparencyEdit::Blend(b)) => { - app.run("transparency.set", json!({"item": item, "blend": b.label()})).ok(); - } - Some(TransparencyEdit::Opacity(o, phase)) => live_run(app, "Opacity", "transparency.set", json!({"item": item, "opacity": o}), phase), - None => {} - }); + widgets::popover(&resp, clicked, |ui| match widgets::opacity_blend(ui, ("ap-op", item), Some(opacity), Some(blend), true) { + Some(TransparencyEdit::Blend(b)) => { + app.run("transparency.set", json!({"item": item, "blend": b.label()})).ok(); + } + Some(TransparencyEdit::Opacity(o, phase)) => live_run(app, "Opacity", "transparency.set", json!({"item": item, "opacity": o}), phase), + None => {} + }); r } diff --git a/crates/ui-egui/src/panels/stroke.rs b/crates/ui-egui/src/panels/stroke.rs index 57b282d91..7283f1b11 100644 --- a/crates/ui-egui/src/panels/stroke.rs +++ b/crates/ui-egui/src/panels/stroke.rs @@ -122,7 +122,7 @@ pub(crate) fn weight_field(app: &mut VectorcraftApp, ui: &mut Ui, id: &str, weig /// The Stroke panel in a popover anchored to `resp` (the Control bar's and the Properties /// panel's Stroke links), which a click on it toggles. pub(crate) fn popover(app: &mut VectorcraftApp, resp: &egui::Response) { - egui::Popup::menu(resp).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| { + widgets::popover(resp, resp.clicked(), |ui| { ui.set_width(260.0); show(app, ui); }); diff --git a/crates/ui-egui/src/panels/tests_appearance.rs b/crates/ui-egui/src/panels/tests_appearance.rs index 52dcccebb..60a423c5e 100644 --- a/crates/ui-egui/src/panels/tests_appearance.rs +++ b/crates/ui-egui/src/panels/tests_appearance.rs @@ -168,6 +168,25 @@ fn item_opacity_popup_sets_the_items_opacity() { assert_eq!((n.appearance.items[0].opacity(), n.opacity), (1.0, 1.0)); } +#[test] +fn opacity_popup_blend_dropdown_sets_the_blend_mode() { + let mut app = app_with_rect(); + let ctx = egui::Context::default(); + let texts = frame_events(&ctx, &mut app, vec![], appearance::show); + // The object's own Opacity row (the last one). + let link = texts.iter().rev().find(|(t, _)| t == "Opacity:").expect("object opacity row").1; + click(&ctx, &mut app, link.center(), appearance::show); + let texts = frame_events(&ctx, &mut app, vec![], appearance::show); + // Opening the dropdown inside the popup keeps the popup open: the list shows. + click(&ctx, &mut app, text_rect(&texts, "Normal").center(), appearance::show); + let texts = frame_events(&ctx, &mut app, vec![], appearance::show); + click(&ctx, &mut app, text_rect(&texts, "Multiply").center(), appearance::show); + assert_eq!(first_selected(&app).unwrap().blend, vectorcraft_color::BlendMode::Multiply); + // The popup is still open after choosing. + let texts = frame_events(&ctx, &mut app, vec![], appearance::show); + assert!(texts.iter().any(|(t, _)| t == "Multiply"), "{texts:?}"); +} + #[test] fn object_row_names_the_selection_like_the_control_bar() { let mut app = app_with_rect(); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 6135094b4..a0b3f4803 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -559,6 +559,31 @@ pub(crate) fn combo( .inner } +/// A popover anchored to `resp`, opened and closed by `toggle` (usually `resp.clicked()`), and +/// closed by Escape or a click outside it and outside any popup it opened. Use it instead of +/// `egui::Popup::menu` when the popover holds dropdowns or popups of its own: egui remembers one +/// open popup at a time, so opening a dropdown inside a remembered popup would close the popup +/// (and the dropdown with it). This one keeps its open state itself. +pub fn popover(resp: &Response, toggle: bool, content: impl FnOnce(&mut Ui) -> R) -> Option { + let ctx = &resp.ctx; + let id = resp.id.with("popover"); + let was_open = ctx.data(|d| d.get_temp::(id)).unwrap_or(false); + let mut open = if toggle { + !was_open + } else { + // Popups (this one and those it opened) are on foreground layers; a click anywhere + // else closes it. + let clicked_elsewhere = ctx + .input(|i| i.pointer.any_click().then(|| i.pointer.interact_pos()).flatten()) + .is_some_and(|pos| ctx.layer_id_at(pos).is_none_or(|l| l.order != egui::Order::Foreground)); + was_open && !clicked_elsewhere + }; + let inner = + egui::Popup::menu(resp).id(id).open_bool(&mut open).close_behavior(egui::PopupCloseBehavior::IgnoreClicks).show(content).map(|r| r.inner); + ctx.data_mut(|d| d.insert_temp(id, open)); + inner +} + /// The body of a menu or popup list: as tall as its items up to the bottom of the window, and /// scrolling only past that, so long menus (Window, Effect, a panel's menu) stay reachable on /// small windows.