From 4848effc3031776c58482c5d7c83eb0e31e8b097 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Wed, 19 Feb 2025 16:11:41 +0100 Subject: [PATCH 1/7] Add `Ui::close` and `Response::should_close` --- crates/egui/src/containers/area.rs | 9 +- crates/egui/src/containers/close_tag.rs | 17 ++++ .../egui/src/containers/collapsing_header.rs | 18 +++- crates/egui/src/containers/mod.rs | 1 + crates/egui/src/containers/modal.rs | 3 + crates/egui/src/containers/popup.rs | 4 +- crates/egui/src/containers/window.rs | 15 ++- crates/egui/src/response.rs | 11 +++ crates/egui/src/ui.rs | 94 ++++++++++++++++--- crates/egui/src/ui_builder.rs | 11 ++- crates/egui/src/ui_stack.rs | 4 + crates/egui_demo_lib/src/demo/modals.rs | 6 +- crates/egui_demo_lib/src/demo/popups.rs | 4 + .../egui_demo_lib/src/demo/window_options.rs | 12 ++- 14 files changed, 182 insertions(+), 27 deletions(-) create mode 100644 crates/egui/src/containers/close_tag.rs diff --git a/crates/egui/src/containers/area.rs b/crates/egui/src/containers/area.rs index 147e4426f45..bf4c63d0212 100644 --- a/crates/egui/src/containers/area.rs +++ b/crates/egui/src/containers/area.rs @@ -556,7 +556,8 @@ impl Prepared { .ui_stack_info(UiStackInfo::new(self.kind)) .layer_id(self.layer_id) .max_rect(max_rect) - .layout(self.layout); + .layout(self.layout) + .closable(); if !self.enabled { ui_builder = ui_builder.disabled(); @@ -611,6 +612,12 @@ impl Prepared { response.rect = final_rect; response.interact_rect = final_rect; + // TODO(lucasmerlin): Can the area response be based on Ui::response? Then this won't be needed + // Bubble up the close event + if content_ui.should_close() { + response.set_close(); + } + ctx.memory_mut(|m| m.areas_mut().set_state(layer_id, state)); if sizing_pass { diff --git a/crates/egui/src/containers/close_tag.rs b/crates/egui/src/containers/close_tag.rs new file mode 100644 index 00000000000..e48d8f6b6a5 --- /dev/null +++ b/crates/egui/src/containers/close_tag.rs @@ -0,0 +1,17 @@ +use std::sync::atomic::AtomicBool; + +#[derive(Debug, Default)] +pub struct ClosableTag { + pub close: AtomicBool, +} + +impl ClosableTag { + pub const NAME: &'static str = "egui_close_tag"; + pub fn set_close(&self) { + self.close.store(true, std::sync::atomic::Ordering::Relaxed); + } + + pub fn should_close(&self) -> bool { + self.close.load(std::sync::atomic::Ordering::Relaxed) + } +} diff --git a/crates/egui/src/containers/collapsing_header.rs b/crates/egui/src/containers/collapsing_header.rs index 1ed2a27fb99..c34e5629464 100644 --- a/crates/egui/src/containers/collapsing_header.rs +++ b/crates/egui/src/containers/collapsing_header.rs @@ -2,7 +2,8 @@ use std::hash::Hash; use crate::{ emath, epaint, pos2, remap, remap_clamp, vec2, Context, Id, InnerResponse, NumExt, Rect, - Response, Sense, Stroke, TextStyle, TextWrapMode, Ui, Vec2, WidgetInfo, WidgetText, WidgetType, + Response, Sense, Stroke, TextStyle, TextWrapMode, Ui, UiBuilder, UiKind, UiStackInfo, Vec2, + WidgetInfo, WidgetText, WidgetType, }; use emath::GuiRounding as _; use epaint::{Shape, StrokeKind}; @@ -203,11 +204,16 @@ impl CollapsingState { add_body: impl FnOnce(&mut Ui) -> R, ) -> Option> { let openness = self.openness(ui.ctx()); + + let builder = UiBuilder::new() + .ui_stack_info(UiStackInfo::new(UiKind::Collapsible)) + .closable(); + if openness <= 0.0 { self.store(ui.ctx()); // we store any earlier toggling as promised in the docstring None } else if openness < 1.0 { - Some(ui.scope(|child_ui| { + Some(ui.scope_builder(builder, |child_ui| { let max_height = if self.state.open && self.state.open_height.is_none() { // First frame of expansion. // We don't know full height yet, but we will next frame. @@ -226,6 +232,9 @@ impl CollapsingState { let mut min_rect = child_ui.min_rect(); self.state.open_height = Some(min_rect.height()); + if child_ui.should_close() { + self.state.open = false; + } self.store(child_ui.ctx()); // remember the height // Pretend children took up at most `max_height` space: @@ -234,7 +243,10 @@ impl CollapsingState { ret })) } else { - let ret_response = ui.scope(add_body); + let ret_response = ui.scope_builder(builder, add_body); + if ret_response.response.should_close() { + self.state.open = false; + } let full_size = ret_response.response.rect.size(); self.state.open_height = Some(full_size.y); self.store(ui.ctx()); // remember the height diff --git a/crates/egui/src/containers/mod.rs b/crates/egui/src/containers/mod.rs index 0d9587e62eb..8dce87e1aad 100644 --- a/crates/egui/src/containers/mod.rs +++ b/crates/egui/src/containers/mod.rs @@ -3,6 +3,7 @@ //! For instance, a [`Frame`] adds a frame and background to some contained UI. pub(crate) mod area; +pub mod close_tag; pub mod collapsing_header; mod combo_box; pub mod frame; diff --git a/crates/egui/src/containers/modal.rs b/crates/egui/src/containers/modal.rs index 8bf5966752c..218bfce3653 100644 --- a/crates/egui/src/containers/modal.rs +++ b/crates/egui/src/containers/modal.rs @@ -150,7 +150,10 @@ impl ModalResponse { let escape_clicked = || ctx.input_mut(|i| i.consume_key(crate::Modifiers::NONE, crate::Key::Escape)); + let ui_close_called = self.response.should_close(); + self.backdrop_response.clicked() + || ui_close_called || (self.is_top_modal && !self.any_popup_open && escape_clicked()) } } diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index 877ee420245..76f67b05b37 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -542,7 +542,9 @@ impl<'a> Popup<'a> { PopupCloseBehavior::IgnoreClicks => false, }; - should_close || ctx.input(|i| i.key_pressed(Key::Escape)) + should_close + || ctx.input(|i| i.key_pressed(Key::Escape)) + || response.response.should_close() }; match open_kind { diff --git a/crates/egui/src/containers/window.rs b/crates/egui/src/containers/window.rs index 6075377ff80..c386449f3ce 100644 --- a/crates/egui/src/containers/window.rs +++ b/crates/egui/src/containers/window.rs @@ -434,7 +434,7 @@ impl Window<'_> { ) -> Option>> { let Window { title, - open, + mut open, area, frame, resize, @@ -634,7 +634,12 @@ impl Window<'_> { title_bar.ui( &mut area_content_ui, &content_response, - open, + // TODO: I feel stupid, is there some better way to pass the &mut open + // without moving it? + match &mut open { + None => None, + Some(open) => Some(open), + }, &mut collapsing, collapsible, ); @@ -650,6 +655,12 @@ impl Window<'_> { let full_response = area.end(ctx, area_content_ui); + if full_response.should_close() { + if let Some(open) = open { + *open = false; + } + } + let inner_response = InnerResponse { inner: content_inner, response: full_response, diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index a5a702cfcd5..0300922692a 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -133,6 +133,9 @@ bitflags::bitflags! { /// Note that this can be `true` even if the user did not interact with the widget, /// for instance if an existing slider value was clamped to the given range. const CHANGED = 1<<11; + + /// Should this container be closed? + const CLOSE = 1<<12; } } @@ -528,6 +531,14 @@ impl Response { self.flags.set(Flags::CHANGED, true); } + pub fn should_close(&self) -> bool { + self.flags.contains(Flags::CLOSE) + } + + pub fn set_close(&mut self) { + self.flags.set(Flags::CLOSE, true); + } + /// Show this UI if the widget was hovered (i.e. a tooltip). /// /// The text will not be visible if the widget is not enabled. diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index 8422c3ce847..62c37e85784 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -1,11 +1,14 @@ #![warn(missing_docs)] // Let's keep `Ui` well-documented. #![allow(clippy::use_self)] -use std::{any::Any, hash::Hash, sync::Arc}; - use emath::GuiRounding as _; use epaint::mutex::RwLock; +use log::warn; +use std::{any::Any, hash::Hash, sync::Arc}; +use crate::close_tag::ClosableTag; +#[cfg(debug_assertions)] +use crate::Stroke; use crate::{ containers::{CollapsingHeader, CollapsingResponse, Frame}, ecolor::Hsva, @@ -26,11 +29,9 @@ use crate::{ }, Align, Color32, Context, CursorIcon, DragAndDrop, Id, InnerResponse, InputState, LayerId, Memory, Order, Painter, PlatformOutput, Pos2, Rangef, Rect, Response, Rgba, RichText, Sense, - Style, TextStyle, TextWrapMode, UiBuilder, UiStack, UiStackInfo, Vec2, WidgetRect, WidgetText, + Style, TextStyle, TextWrapMode, UiBuilder, UiKind, UiStack, UiStackInfo, Vec2, WidgetRect, + WidgetText, }; - -#[cfg(debug_assertions)] -use crate::Stroke; // ---------------------------------------------------------------------------- /// This is what you use to place widgets. @@ -763,6 +764,62 @@ impl Ui { pub fn is_rect_visible(&self, rect: Rect) -> bool { self.is_visible() && rect.intersects(self.clip_rect()) } + + /// Find and close the first closable parent. + /// Use [`UiBuilder::closable`] to make a [`Ui`] closable. + /// You can then use [`Ui::should_close`] to check if it should be closed. + /// + /// This is implemented for all egui containers, e.g. [`crate::Popup`], [`crate::Modal`], + /// [`crate::Area`], [`crate::Window`], [`crate::CollapsingHeader`], etc. + /// + /// What exactly happens when you close a container depends on the container implementation. + /// [`crate::Area`] e.g. will return true from it's [`Response::should_close`] method. + /// + /// If you want to close a specific kind of container, use [`Ui::close_kind`] instead. + pub fn close(&self) { + let tag = self.stack.iter().find_map(|stack| { + stack + .info + .tags + .get_downcast::(ClosableTag::NAME) + }); + if let Some(tag) = tag { + tag.set_close(); + } else { + warn!("Tried to close a Ui that has no ClosableTag in its stack."); + } + } + + /// Find and close the first closable parent of a specific [`UiKind`]. + /// This is useful if you want to e.g. close a [`crate::Window`]. Since it contains a + /// `Collapsible`, [`Ui::close`] would close the `Collapsible` instead. + /// You can close the [`crate::Window`] by calling `ui.close_kind(UiKind::Window)`. + pub fn close_kind(&self, ui_kind: UiKind) { + let tag = self + .stack + .iter() + .filter(|stack| stack.info.kind == Some(ui_kind)) + .find_map(|stack| { + stack + .info + .tags + .get_downcast::(ClosableTag::NAME) + }); + if let Some(tag) = tag { + tag.set_close(); + } else { + warn!("Tried to close a Ui that has no ClosableTag in its stack."); + } + } + + /// Was [`Ui::close`] called on this [`Ui`] or any of its children? + pub fn should_close(&self) -> bool { + self.stack + .info + .tags + .get_downcast(ClosableTag::NAME) + .is_some_and(|tag: &ClosableTag| tag.should_close()) + } } /// # Helpers for accessing the underlying [`Context`]. @@ -1095,7 +1152,8 @@ impl Ui { // This is the inverse of Context::read_response. We prefer a response // based on last frame's widget rect since the one from this frame is Rect::NOTHING until // Ui::interact_bg is called or the Ui is dropped. - self.ctx() + let mut response = self + .ctx() .viewport(|viewport| { viewport .prev_pass @@ -1107,7 +1165,11 @@ impl Ui { .map(|widget_rect| self.ctx().get_response(widget_rect)) .expect( "Since we always call Context::create_widget in Ui::new, this should never be None", - ) + ); + if self.should_close() { + response.set_close(); + } + response } /// Update the [`WidgetRect`] created in [`Ui::new`] or [`Ui::new_child`] with the current @@ -1121,7 +1183,7 @@ impl Ui { fs.used_ids.remove(&self.unique_id); }); // This will update the WidgetRect that was first created in `Ui::new`. - self.ctx().create_widget( + let mut response = self.ctx().create_widget( WidgetRect { id: self.unique_id, layer_id: self.layer_id(), @@ -1131,7 +1193,11 @@ impl Ui { enabled: self.enabled, }, false, - ) + ); + if self.should_close() { + response.set_close(); + } + response } /// Interact with the background of this [`Ui`], @@ -2900,11 +2966,9 @@ impl Ui { /// Close the menu we are in (including submenus), if any. /// /// See also: [`Self::menu_button`] and [`Response::context_menu`]. - pub fn close_menu(&mut self) { - if let Some(menu_state) = &mut self.menu_state { - menu_state.write().close(); - } - self.menu_state = None; + #[deprecated = "Use `ui.close()` instead"] + pub fn close_menu(&self) { + self.close(); } pub(crate) fn set_menu_state(&mut self, menu_state: Option>>) { diff --git a/crates/egui/src/ui_builder.rs b/crates/egui/src/ui_builder.rs index d13748543ab..462f156b3c6 100644 --- a/crates/egui/src/ui_builder.rs +++ b/crates/egui/src/ui_builder.rs @@ -1,9 +1,9 @@ use std::{hash::Hash, sync::Arc}; -use crate::{Id, LayerId, Layout, Rect, Sense, Style, UiStackInfo}; - +use crate::close_tag::ClosableTag; #[allow(unused_imports)] // Used for doclinks use crate::Ui; +use crate::{Id, LayerId, Layout, Rect, Sense, Style, UiStackInfo}; /// Build a [`Ui`] as the child of another [`Ui`]. /// @@ -135,4 +135,11 @@ impl UiBuilder { self.sense = Some(sense); self } + + pub fn closable(mut self) -> Self { + self.ui_stack_info + .tags + .insert(ClosableTag::NAME, Some(Arc::new(ClosableTag::default()))); + self + } } diff --git a/crates/egui/src/ui_stack.rs b/crates/egui/src/ui_stack.rs index 550f6b182b8..64a776ee658 100644 --- a/crates/egui/src/ui_stack.rs +++ b/crates/egui/src/ui_stack.rs @@ -53,6 +53,9 @@ pub enum UiKind { /// An [`crate::Area`] that is not of any other kind. GenericArea, + + /// A collapsible container, e.g. a [`crate::CollapsingHeader`]. + Collapsible, } impl UiKind { @@ -81,6 +84,7 @@ impl UiKind { | Self::Frame | Self::ScrollArea | Self::Resize + | Self::Collapsible | Self::TableCell => false, Self::Window diff --git a/crates/egui_demo_lib/src/demo/modals.rs b/crates/egui_demo_lib/src/demo/modals.rs index d344d99c010..f83820e74e0 100644 --- a/crates/egui_demo_lib/src/demo/modals.rs +++ b/crates/egui_demo_lib/src/demo/modals.rs @@ -96,7 +96,9 @@ impl crate::View for Modals { *save_modal_open = true; } if ui.button("Cancel").clicked() { - *user_modal_open = false; + // You can call `ui.close()` to close the modal. + // (This causes the current modals `should_close` to return true) + ui.close(); } }, ); @@ -123,7 +125,7 @@ impl crate::View for Modals { } if ui.button("No Thanks").clicked() { - *save_modal_open = false; + ui.close(); } }, ); diff --git a/crates/egui_demo_lib/src/demo/popups.rs b/crates/egui_demo_lib/src/demo/popups.rs index 0eeb76272c1..0f1939c0e5b 100644 --- a/crates/egui_demo_lib/src/demo/popups.rs +++ b/crates/egui_demo_lib/src/demo/popups.rs @@ -153,6 +153,10 @@ impl crate::View for PopupsDemo { .show(|ui| { _ = ui.button("Menu item 1"); _ = ui.button("Menu item 2"); + + if ui.button("I always close the menu").clicked() { + ui.close(); + } }); self.apply_options(Popup::context_menu(&response).id(Id::new("context_menu"))) diff --git a/crates/egui_demo_lib/src/demo/window_options.rs b/crates/egui_demo_lib/src/demo/window_options.rs index f0d24b4604c..67ec22463f5 100644 --- a/crates/egui_demo_lib/src/demo/window_options.rs +++ b/crates/egui_demo_lib/src/demo/window_options.rs @@ -1,4 +1,4 @@ -use egui::Vec2b; +use egui::{UiKind, Vec2b}; #[derive(Clone, PartialEq)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] @@ -149,6 +149,16 @@ impl crate::View for WindowOptions { self.disabled_time = ui.input(|i| i.time); } egui::reset_button(ui, self, "Reset"); + if ui + .button("Close") + .on_hover_text("You can collapse / close Windows via Ui::close") + .clicked() + { + // Calling close would close the collapsible within the window + // ui.close(); + // Instead, we close the window itself + ui.close_kind(UiKind::Window); + } ui.add(crate::egui_github_link_file!()); }); } From 7e8d4f7119c124d61eef6b239da4975dab63c089 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Wed, 19 Feb 2025 16:21:35 +0100 Subject: [PATCH 2/7] Fixes --- crates/egui/src/containers/close_tag.rs | 3 + crates/egui/src/containers/window.rs | 2 +- crates/egui/src/ui.rs | 115 ++++++++++++------------ crates/egui/src/ui_builder.rs | 7 ++ 4 files changed, 70 insertions(+), 57 deletions(-) diff --git a/crates/egui/src/containers/close_tag.rs b/crates/egui/src/containers/close_tag.rs index e48d8f6b6a5..3d5e3f434c4 100644 --- a/crates/egui/src/containers/close_tag.rs +++ b/crates/egui/src/containers/close_tag.rs @@ -7,10 +7,13 @@ pub struct ClosableTag { impl ClosableTag { pub const NAME: &'static str = "egui_close_tag"; + + /// Set close to `true` pub fn set_close(&self) { self.close.store(true, std::sync::atomic::Ordering::Relaxed); } + /// Returns `true` if [`ClosableTag::set_close`] has been called. pub fn should_close(&self) -> bool { self.close.load(std::sync::atomic::Ordering::Relaxed) } diff --git a/crates/egui/src/containers/window.rs b/crates/egui/src/containers/window.rs index c386449f3ce..eb1863d5609 100644 --- a/crates/egui/src/containers/window.rs +++ b/crates/egui/src/containers/window.rs @@ -634,7 +634,7 @@ impl Window<'_> { title_bar.ui( &mut area_content_ui, &content_response, - // TODO: I feel stupid, is there some better way to pass the &mut open + // TODO(lucas): I feel stupid, is there some better way to pass the &mut open // without moving it? match &mut open { None => None, diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index 62c37e85784..72ea09dace3 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -764,62 +764,6 @@ impl Ui { pub fn is_rect_visible(&self, rect: Rect) -> bool { self.is_visible() && rect.intersects(self.clip_rect()) } - - /// Find and close the first closable parent. - /// Use [`UiBuilder::closable`] to make a [`Ui`] closable. - /// You can then use [`Ui::should_close`] to check if it should be closed. - /// - /// This is implemented for all egui containers, e.g. [`crate::Popup`], [`crate::Modal`], - /// [`crate::Area`], [`crate::Window`], [`crate::CollapsingHeader`], etc. - /// - /// What exactly happens when you close a container depends on the container implementation. - /// [`crate::Area`] e.g. will return true from it's [`Response::should_close`] method. - /// - /// If you want to close a specific kind of container, use [`Ui::close_kind`] instead. - pub fn close(&self) { - let tag = self.stack.iter().find_map(|stack| { - stack - .info - .tags - .get_downcast::(ClosableTag::NAME) - }); - if let Some(tag) = tag { - tag.set_close(); - } else { - warn!("Tried to close a Ui that has no ClosableTag in its stack."); - } - } - - /// Find and close the first closable parent of a specific [`UiKind`]. - /// This is useful if you want to e.g. close a [`crate::Window`]. Since it contains a - /// `Collapsible`, [`Ui::close`] would close the `Collapsible` instead. - /// You can close the [`crate::Window`] by calling `ui.close_kind(UiKind::Window)`. - pub fn close_kind(&self, ui_kind: UiKind) { - let tag = self - .stack - .iter() - .filter(|stack| stack.info.kind == Some(ui_kind)) - .find_map(|stack| { - stack - .info - .tags - .get_downcast::(ClosableTag::NAME) - }); - if let Some(tag) = tag { - tag.set_close(); - } else { - warn!("Tried to close a Ui that has no ClosableTag in its stack."); - } - } - - /// Was [`Ui::close`] called on this [`Ui`] or any of its children? - pub fn should_close(&self) -> bool { - self.stack - .info - .tags - .get_downcast(ClosableTag::NAME) - .is_some_and(|tag: &ClosableTag| tag.should_close()) - } } /// # Helpers for accessing the underlying [`Context`]. @@ -1231,6 +1175,65 @@ impl Ui { pub fn ui_contains_pointer(&self) -> bool { self.rect_contains_pointer(self.min_rect()) } + + /// Find and close the first closable parent. + /// Use [`UiBuilder::closable`] to make a [`Ui`] closable. + /// You can then use [`Ui::should_close`] to check if it should be closed. + /// + /// This is implemented for all egui containers, e.g. [`crate::Popup`], [`crate::Modal`], + /// [`crate::Area`], [`crate::Window`], [`crate::CollapsingHeader`], etc. + /// + /// What exactly happens when you close a container depends on the container implementation. + /// [`crate::Area`] e.g. will return true from it's [`Response::should_close`] method. + /// + /// If you want to close a specific kind of container, use [`Ui::close_kind`] instead. + pub fn close(&self) { + let tag = self.stack.iter().find_map(|stack| { + stack + .info + .tags + .get_downcast::(ClosableTag::NAME) + }); + if let Some(tag) = tag { + tag.set_close(); + } else { + warn!("Tried to close a Ui that has no ClosableTag in its stack."); + } + } + + /// Find and close the first closable parent of a specific [`UiKind`]. + /// This is useful if you want to e.g. close a [`crate::Window`]. Since it contains a + /// `Collapsible`, [`Ui::close`] would close the `Collapsible` instead. + /// You can close the [`crate::Window`] by calling `ui.close_kind(UiKind::Window)`. + pub fn close_kind(&self, ui_kind: UiKind) { + let tag = self + .stack + .iter() + .filter(|stack| stack.info.kind == Some(ui_kind)) + .find_map(|stack| { + stack + .info + .tags + .get_downcast::(ClosableTag::NAME) + }); + if let Some(tag) = tag { + tag.set_close(); + } else { + warn!("Tried to close a Ui that has no ClosableTag in its stack."); + } + } + + /// Was [`Ui::close`] called on this [`Ui`] or any of its children? + /// Only works if the [`Ui`] was created with [`UiBuilder::closable`]. + /// + /// You can also check via this [`Ui`]'s [`Response::should_close`]. + pub fn should_close(&self) -> bool { + self.stack + .info + .tags + .get_downcast(ClosableTag::NAME) + .is_some_and(|tag: &ClosableTag| tag.should_close()) + } } /// # Allocating space: where do I put my widgets? diff --git a/crates/egui/src/ui_builder.rs b/crates/egui/src/ui_builder.rs index 462f156b3c6..1cc8decccb0 100644 --- a/crates/egui/src/ui_builder.rs +++ b/crates/egui/src/ui_builder.rs @@ -136,6 +136,13 @@ impl UiBuilder { self } + /// Make this [`Ui`] closable. + /// Calling [`Ui::close`] in a child [`Ui`] will mark this [`Ui`] for closing. + /// After [`Ui::close`] was called, [`Ui::should_close`] and [`Response::should_close`] will + /// return `true` (for this frame). + /// + /// This works by adding a [`ClosableTag`] to the [`UiStackInfo`]. + #[inline] pub fn closable(mut self) -> Self { self.ui_stack_info .tags From 1b749fb4a1624e13514ee4e6ae2e3a907429d4ea Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Wed, 19 Feb 2025 16:34:07 +0100 Subject: [PATCH 3/7] Replace `close_menu` with `close` --- crates/egui/src/gui_zoom.rs | 6 +++--- crates/egui/src/menu.rs | 8 +++++++- crates/egui/src/response.rs | 4 ++-- crates/egui/src/ui.rs | 12 ++++++------ crates/egui_demo_app/src/backend_panel.rs | 2 +- crates/egui_demo_app/src/wrap_app.rs | 4 ++-- crates/egui_demo_lib/src/demo/context_menu.rs | 10 +++++----- crates/egui_demo_lib/src/demo/demo_app_windows.rs | 6 +++--- 8 files changed, 29 insertions(+), 23 deletions(-) diff --git a/crates/egui/src/gui_zoom.rs b/crates/egui/src/gui_zoom.rs index 4e67d1a1450..b0da1d1b789 100644 --- a/crates/egui/src/gui_zoom.rs +++ b/crates/egui/src/gui_zoom.rs @@ -88,7 +88,7 @@ pub fn zoom_menu_buttons(ui: &mut Ui) { .clicked() { zoom_in(ui.ctx()); - ui.close_menu(); + ui.close(); } if ui @@ -99,7 +99,7 @@ pub fn zoom_menu_buttons(ui: &mut Ui) { .clicked() { zoom_out(ui.ctx()); - ui.close_menu(); + ui.close(); } if ui @@ -110,6 +110,6 @@ pub fn zoom_menu_buttons(ui: &mut Ui) { .clicked() { ui.ctx().set_zoom_factor(1.0); - ui.close_menu(); + ui.close(); } } diff --git a/crates/egui/src/menu.rs b/crates/egui/src/menu.rs index 96aeeac61b6..25cd213ebba 100644 --- a/crates/egui/src/menu.rs +++ b/crates/egui/src/menu.rs @@ -364,7 +364,10 @@ impl MenuRoot { let menu_state = self.menu_state.read(); let escape_pressed = button.ctx.input(|i| i.key_pressed(Key::Escape)); - if menu_state.response.is_close() || escape_pressed { + if menu_state.response.is_close() + || escape_pressed + || inner_response.response.should_close() + { return (MenuResponse::Close, Some(inner_response)); } } @@ -667,6 +670,9 @@ impl MenuState { ) -> Option { let (sub_response, response) = self.submenu(id).map(|sub| { let inner_response = menu_popup(ctx, parent_layer, sub, id, add_contents); + if inner_response.response.should_close() { + sub.write().close(); + } (sub.read().response, inner_response.inner) })?; self.cascade_close_response(sub_response); diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 0300922692a..57285060e42 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -920,13 +920,13 @@ impl Response { /// let response = ui.add(Label::new("Right-click me!").sense(Sense::click())); /// response.context_menu(|ui| { /// if ui.button("Close the menu").clicked() { - /// ui.close_menu(); + /// ui.close(); /// } /// }); /// # }); /// ``` /// - /// See also: [`Ui::menu_button`] and [`Ui::close_menu`]. + /// See also: [`Ui::menu_button`] and [`Ui::close`]. pub fn context_menu(&self, add_contents: impl FnOnce(&mut Ui)) -> Option> { menu::context_menu(self, add_contents) } diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index 72ea09dace3..ce38579ee67 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -2988,14 +2988,14 @@ impl Ui { /// ui.menu_button("My menu", |ui| { /// ui.menu_button("My sub-menu", |ui| { /// if ui.button("Close the menu").clicked() { - /// ui.close_menu(); + /// ui.close(); /// } /// }); /// }); /// # }); /// ``` /// - /// See also: [`Self::close_menu`] and [`Response::context_menu`]. + /// See also: [`Self::close`] and [`Response::context_menu`]. pub fn menu_button( &mut self, title: impl Into, @@ -3019,7 +3019,7 @@ impl Ui { /// ui.menu_image_button(title, img, |ui| { /// ui.menu_button("My sub-menu", |ui| { /// if ui.button("Close the menu").clicked() { - /// ui.close_menu(); + /// ui.close(); /// } /// }); /// }); @@ -3027,7 +3027,7 @@ impl Ui { /// ``` /// /// - /// See also: [`Self::close_menu`] and [`Response::context_menu`]. + /// See also: [`Self::close`] and [`Response::context_menu`]. #[inline] pub fn menu_image_button<'a, R>( &mut self, @@ -3053,14 +3053,14 @@ impl Ui { /// ui.menu_image_text_button(img, title, |ui| { /// ui.menu_button("My sub-menu", |ui| { /// if ui.button("Close the menu").clicked() { - /// ui.close_menu(); + /// ui.close(); /// } /// }); /// }); /// # }); /// ``` /// - /// See also: [`Self::close_menu`] and [`Response::context_menu`]. + /// See also: [`Self::close`] and [`Response::context_menu`]. #[inline] pub fn menu_image_text_button<'a, R>( &mut self, diff --git a/crates/egui_demo_app/src/backend_panel.rs b/crates/egui_demo_app/src/backend_panel.rs index 21c35cd9229..e0a29f14c8b 100644 --- a/crates/egui_demo_app/src/backend_panel.rs +++ b/crates/egui_demo_app/src/backend_panel.rs @@ -333,7 +333,7 @@ fn integration_ui(ui: &mut egui::Ui, _frame: &mut eframe::Frame) { .send_viewport_cmd(egui::ViewportCommand::InnerSize(size)); ui.ctx() .send_viewport_cmd(egui::ViewportCommand::Fullscreen(false)); - ui.close_menu(); + ui.close(); } }); } diff --git a/crates/egui_demo_app/src/wrap_app.rs b/crates/egui_demo_app/src/wrap_app.rs index eb26a5d7cea..a571cfbb2df 100644 --- a/crates/egui_demo_app/src/wrap_app.rs +++ b/crates/egui_demo_app/src/wrap_app.rs @@ -384,12 +384,12 @@ impl WrapApp { .clicked() { ui.ctx().memory_mut(|mem| *mem = Default::default()); - ui.close_menu(); + ui.close(); } if ui.button("Reset everything").clicked() { *cmd = Command::ResetEverything; - ui.close_menu(); + ui.close(); } }); } diff --git a/crates/egui_demo_lib/src/demo/context_menu.rs b/crates/egui_demo_lib/src/demo/context_menu.rs index 35abc0800cd..b6630cf4174 100644 --- a/crates/egui_demo_lib/src/demo/context_menu.rs +++ b/crates/egui_demo_lib/src/demo/context_menu.rs @@ -59,25 +59,25 @@ impl ContextMenus { ui.set_max_width(200.0); // To make sure we wrap long text if ui.button("Open…").clicked() { - ui.close_menu(); + ui.close(); } ui.menu_button("SubMenu", |ui| { ui.menu_button("SubMenu", |ui| { if ui.button("Open…").clicked() { - ui.close_menu(); + ui.close(); } let _ = ui.button("Item"); ui.menu_button("Recursive", Self::nested_menus) }); ui.menu_button("SubMenu", |ui| { if ui.button("Open…").clicked() { - ui.close_menu(); + ui.close(); } let _ = ui.button("Item"); }); let _ = ui.button("Item"); if ui.button("Open…").clicked() { - ui.close_menu(); + ui.close(); } }); ui.menu_button("SubMenu", |ui| { @@ -86,7 +86,7 @@ impl ContextMenus { let _ = ui.button("Item3"); let _ = ui.button("Item4"); if ui.button("Open…").clicked() { - ui.close_menu(); + ui.close(); } }); let _ = ui.button("Very long text for this item that should be wrapped"); diff --git a/crates/egui_demo_lib/src/demo/demo_app_windows.rs b/crates/egui_demo_lib/src/demo/demo_app_windows.rs index 80d32e6976d..33f2543b721 100644 --- a/crates/egui_demo_lib/src/demo/demo_app_windows.rs +++ b/crates/egui_demo_lib/src/demo/demo_app_windows.rs @@ -234,7 +234,7 @@ impl DemoWindows { ui.set_style(ui.ctx().style()); // ignore the "menu" style set by `menu_button`. self.demo_list_ui(ui); if ui.ui_contains_pointer() && ui.input(|i| i.pointer.any_click()) { - ui.close_menu(); + ui.close(); } }); @@ -345,7 +345,7 @@ fn file_menu_button(ui: &mut Ui) { .clicked() { ui.ctx().memory_mut(|mem| mem.reset_areas()); - ui.close_menu(); + ui.close(); } if ui @@ -357,7 +357,7 @@ fn file_menu_button(ui: &mut Ui) { .clicked() { ui.ctx().memory_mut(|mem| *mem = Default::default()); - ui.close_menu(); + ui.close(); } }); } From 06f607ac54cd5da9135ce1d6dc7e734959a62968 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Wed, 19 Feb 2025 16:36:05 +0100 Subject: [PATCH 4/7] Snapshots --- crates/egui_demo_lib/tests/snapshots/demos/Window Options.png | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/egui_demo_lib/tests/snapshots/demos/Window Options.png b/crates/egui_demo_lib/tests/snapshots/demos/Window Options.png index 21b08252a83..e4ee95e396c 100644 --- a/crates/egui_demo_lib/tests/snapshots/demos/Window Options.png +++ b/crates/egui_demo_lib/tests/snapshots/demos/Window Options.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3411a4a8939b7e731c9c1a6331921b0ac905f4e3e86a51af70bdb38d9446f5e1 -size 35193 +oid sha256:e67b1e676ff994cb9557939db3dca5ddd15c69d167afd96c0957a2a3b75c0fd8 +size 36007 From 51cf5e89c8abfc6d3f8f46c8dd7f7e8429cbc534 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Wed, 19 Feb 2025 16:38:14 +0100 Subject: [PATCH 5/7] Fix log --- crates/egui/src/ui.rs | 7 ++++--- crates/egui/src/ui_builder.rs | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index ce38579ee67..f5d547e6cbf 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -3,7 +3,6 @@ use emath::GuiRounding as _; use epaint::mutex::RwLock; -use log::warn; use std::{any::Any, hash::Hash, sync::Arc}; use crate::close_tag::ClosableTag; @@ -1197,7 +1196,8 @@ impl Ui { if let Some(tag) = tag { tag.set_close(); } else { - warn!("Tried to close a Ui that has no ClosableTag in its stack."); + #[cfg(feature = "log")] + log::warn!("Tried to close a Ui that has no ClosableTag in its stack."); } } @@ -1219,7 +1219,8 @@ impl Ui { if let Some(tag) = tag { tag.set_close(); } else { - warn!("Tried to close a Ui that has no ClosableTag in its stack."); + #[cfg(feature = "log")] + log::warn!("Tried to close a Ui that has no ClosableTag in its stack."); } } diff --git a/crates/egui/src/ui_builder.rs b/crates/egui/src/ui_builder.rs index 1cc8decccb0..23521a5df07 100644 --- a/crates/egui/src/ui_builder.rs +++ b/crates/egui/src/ui_builder.rs @@ -138,7 +138,7 @@ impl UiBuilder { /// Make this [`Ui`] closable. /// Calling [`Ui::close`] in a child [`Ui`] will mark this [`Ui`] for closing. - /// After [`Ui::close`] was called, [`Ui::should_close`] and [`Response::should_close`] will + /// After [`Ui::close`] was called, [`Ui::should_close`] and [`crate::Response::should_close`] will /// return `true` (for this frame). /// /// This works by adding a [`ClosableTag`] to the [`UiStackInfo`]. From 2308325e9e4d9d2b767c353a7bad96f9dcc12740 Mon Sep 17 00:00:00 2001 From: Lucas Meurer Date: Thu, 20 Feb 2025 17:52:16 +0100 Subject: [PATCH 6/7] Fixes after review --- crates/egui/src/containers/window.rs | 7 +------ crates/egui/src/response.rs | 7 +++++++ crates/egui/src/ui.rs | 6 ++++-- crates/egui/src/ui_builder.rs | 2 ++ 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/egui/src/containers/window.rs b/crates/egui/src/containers/window.rs index eb1863d5609..2f9e8fd4a60 100644 --- a/crates/egui/src/containers/window.rs +++ b/crates/egui/src/containers/window.rs @@ -634,12 +634,7 @@ impl Window<'_> { title_bar.ui( &mut area_content_ui, &content_response, - // TODO(lucas): I feel stupid, is there some better way to pass the &mut open - // without moving it? - match &mut open { - None => None, - Some(open) => Some(open), - }, + open.as_deref_mut(), &mut collapsing, collapsible, ); diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 57285060e42..863533d1f65 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -531,10 +531,17 @@ impl Response { self.flags.set(Flags::CHANGED, true); } + /// Should the container be closed? + /// + /// Will e.g. be set by calling [`Ui::close`] in a child [`Ui`] or by calling + /// [`Self::set_close`]. pub fn should_close(&self) -> bool { self.flags.contains(Flags::CLOSE) } + /// Set the [`Flags::CLOSE`] flag. + /// + /// Can be used to e.g. signal that a container should be closed. pub fn set_close(&mut self) { self.flags.set(Flags::CLOSE, true); } diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index f5d547e6cbf..6360af12d32 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -1176,6 +1176,7 @@ impl Ui { } /// Find and close the first closable parent. + /// /// Use [`UiBuilder::closable`] to make a [`Ui`] closable. /// You can then use [`Ui::should_close`] to check if it should be closed. /// @@ -1202,6 +1203,7 @@ impl Ui { } /// Find and close the first closable parent of a specific [`UiKind`]. + /// /// This is useful if you want to e.g. close a [`crate::Window`]. Since it contains a /// `Collapsible`, [`Ui::close`] would close the `Collapsible` instead. /// You can close the [`crate::Window`] by calling `ui.close_kind(UiKind::Window)`. @@ -2970,9 +2972,9 @@ impl Ui { /// Close the menu we are in (including submenus), if any. /// /// See also: [`Self::menu_button`] and [`Response::context_menu`]. - #[deprecated = "Use `ui.close()` instead"] + #[deprecated = "Use `ui.close()` or `ui.close_kind(UiKind::Menu)` instead"] pub fn close_menu(&self) { - self.close(); + self.close_kind(UiKind::Menu); } pub(crate) fn set_menu_state(&mut self, menu_state: Option>>) { diff --git a/crates/egui/src/ui_builder.rs b/crates/egui/src/ui_builder.rs index 23521a5df07..4d225ae4c73 100644 --- a/crates/egui/src/ui_builder.rs +++ b/crates/egui/src/ui_builder.rs @@ -125,6 +125,7 @@ impl UiBuilder { } /// Set if you want sense clicks and/or drags. Default is [`Sense::hover`]. + /// /// The sense will be registered below the Senses of any widgets contained in this [`Ui`], so /// if the user clicks a button contained within this [`Ui`], that button will receive the click /// instead. @@ -137,6 +138,7 @@ impl UiBuilder { } /// Make this [`Ui`] closable. + /// /// Calling [`Ui::close`] in a child [`Ui`] will mark this [`Ui`] for closing. /// After [`Ui::close`] was called, [`Ui::should_close`] and [`crate::Response::should_close`] will /// return `true` (for this frame). From 59fdd3b660952df670988bbb23db5663d567442c Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Thu, 20 Feb 2025 17:53:24 +0100 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Emil Ernerfeldt --- crates/egui/src/ui.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/egui/src/ui.rs b/crates/egui/src/ui.rs index 6360af12d32..7d95aa4d193 100644 --- a/crates/egui/src/ui.rs +++ b/crates/egui/src/ui.rs @@ -1198,7 +1198,7 @@ impl Ui { tag.set_close(); } else { #[cfg(feature = "log")] - log::warn!("Tried to close a Ui that has no ClosableTag in its stack."); + log::warn!("Called ui.close() on a Ui that has no closable parent."); } } @@ -1222,7 +1222,7 @@ impl Ui { tag.set_close(); } else { #[cfg(feature = "log")] - log::warn!("Tried to close a Ui that has no ClosableTag in its stack."); + log::warn!("Called ui.close_kind({ui_kind:?}) on ui with no such closable parent."); } }