From 224af23fd1efa2820114a431fcfd45c41fed6600 Mon Sep 17 00:00:00 2001
From: Ezra Barrow <41446523+barrowsys@users.noreply.github.com>
Date: Wed, 21 Jul 2021 05:43:02 -0400
Subject: [PATCH] Pass more inner return values (#557)

* add Window.show_with_return

* Fixed all missed opportunities to pass an inner return value
---
 CHANGELOG.md                     |  7 +++
 egui/src/containers/area.rs      | 11 +++--
 egui/src/containers/combo_box.rs | 66 +++++++++++++++----------
 egui/src/containers/popup.rs     | 84 ++++++++++++++++++++++----------
 egui/src/containers/window.rs    | 35 ++++++++-----
 egui/src/menu.rs                 | 56 ++++++++++++++-------
 egui/src/widgets/color_picker.rs |  3 +-
 7 files changed, 176 insertions(+), 86 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a8df010d..051c0c55 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,13 @@ NOTE: [`eframe`](eframe/CHANGELOG.md), [`egui_web`](egui_web/CHANGELOG.md) and [
 
 
 ## Unreleased
+* Replaced all missed opportunities to return an inner return value. (this is a breaking change!)
+  * `Area::show`
+  * `ComboBox::show_ui`
+  * `ComboBox::combo_box_with_label`
+  * `Window::show`
+  * `popup::*`
+  * `menu::menu`
 
 ### Added ⭐
 * Plot:
diff --git a/egui/src/containers/area.rs b/egui/src/containers/area.rs
index bf90c255..e6e3dcba 100644
--- a/egui/src/containers/area.rs
+++ b/egui/src/containers/area.rs
@@ -219,11 +219,16 @@ impl Area {
         }
     }
 
-    pub fn show(self, ctx: &CtxRef, add_contents: impl FnOnce(&mut Ui)) -> Response {
+    pub fn show<R>(
+        self,
+        ctx: &CtxRef,
+        add_contents: impl FnOnce(&mut Ui) -> R,
+    ) -> InnerResponse<R> {
         let prepared = self.begin(ctx);
         let mut content_ui = prepared.content_ui(ctx);
-        add_contents(&mut content_ui);
-        prepared.end(ctx, content_ui)
+        let inner = add_contents(&mut content_ui);
+        let response = prepared.end(ctx, content_ui);
+        InnerResponse { inner, response }
     }
 
     pub fn show_open_close_animation(&self, ctx: &CtxRef, frame: &Frame, is_open: bool) {
diff --git a/egui/src/containers/combo_box.rs b/egui/src/containers/combo_box.rs
index 042ff345..acb41398 100644
--- a/egui/src/containers/combo_box.rs
+++ b/egui/src/containers/combo_box.rs
@@ -63,7 +63,13 @@ impl ComboBox {
     }
 
     /// Show the combo box, with the given ui code for the menu contents.
-    pub fn show_ui(self, ui: &mut Ui, menu_contents: impl FnOnce(&mut Ui)) -> Response {
+    ///
+    /// Returns `InnerResponse { inner: None }` if the combo box is closed.
+    pub fn show_ui<R>(
+        self,
+        ui: &mut Ui,
+        menu_contents: impl FnOnce(&mut Ui) -> R,
+    ) -> InnerResponse<Option<R>> {
         let Self {
             id_source,
             label,
@@ -77,14 +83,16 @@ impl ComboBox {
             if let Some(width) = width {
                 ui.spacing_mut().slider_width = width; // yes, this is ugly. Will remove later.
             }
-            let mut response = combo_box(ui, button_id, selected_text, menu_contents);
+            let mut ir = combo_box(ui, button_id, selected_text, menu_contents);
             if let Some(label) = label {
-                response.widget_info(|| WidgetInfo::labeled(WidgetType::ComboBox, label.text()));
-                response |= ui.add(label);
+                ir.response
+                    .widget_info(|| WidgetInfo::labeled(WidgetType::ComboBox, label.text()));
+                ir.response |= ui.add(label);
             } else {
-                response.widget_info(|| WidgetInfo::labeled(WidgetType::ComboBox, ""));
+                ir.response
+                    .widget_info(|| WidgetInfo::labeled(WidgetType::ComboBox, ""));
             }
-            response
+            ir
         })
         .inner
     }
@@ -117,14 +125,16 @@ impl ComboBox {
 
         let mut changed = false;
 
-        let mut response = slf.show_ui(ui, |ui| {
-            for i in 0..len {
-                if ui.selectable_label(i == *selected, get(i)).clicked() {
-                    *selected = i;
-                    changed = true;
+        let mut response = slf
+            .show_ui(ui, |ui| {
+                for i in 0..len {
+                    if ui.selectable_label(i == *selected, get(i)).clicked() {
+                        *selected = i;
+                        changed = true;
+                    }
                 }
-            }
-        });
+            })
+            .response;
 
         if changed {
             response.mark_changed();
@@ -137,6 +147,8 @@ impl ComboBox {
 ///
 /// Deprecated! Use [`ComboBox`] instead!
 ///
+/// Returns `InnerResponse { inner: None }` if the combo box is closed.
+///
 /// ```
 /// # #[derive(Debug, PartialEq)]
 /// # enum Enum { First, Second, Third }
@@ -149,31 +161,32 @@ impl ComboBox {
 /// });
 /// ```
 #[deprecated = "Use egui::ComboBox::from_label instead"]
-pub fn combo_box_with_label(
+pub fn combo_box_with_label<R>(
     ui: &mut Ui,
     label: impl Into<Label>,
     selected: impl ToString,
-    menu_contents: impl FnOnce(&mut Ui),
-) -> Response {
+    menu_contents: impl FnOnce(&mut Ui) -> R,
+) -> InnerResponse<Option<R>> {
     let label = label.into();
     let button_id = ui.make_persistent_id(label.text());
 
     ui.horizontal(|ui| {
-        let mut response = combo_box(ui, button_id, selected, menu_contents);
-        response.widget_info(|| WidgetInfo::labeled(WidgetType::ComboBox, label.text()));
-        response |= ui.add(label);
-        response
+        let mut ir = combo_box(ui, button_id, selected, menu_contents);
+        ir.response
+            .widget_info(|| WidgetInfo::labeled(WidgetType::ComboBox, label.text()));
+        ir.response |= ui.add(label);
+        ir
     })
     .inner
 }
 
 #[allow(clippy::needless_pass_by_value)]
-fn combo_box(
+fn combo_box<R>(
     ui: &mut Ui,
     button_id: Id,
     selected: impl ToString,
-    menu_contents: impl FnOnce(&mut Ui),
-) -> Response {
+    menu_contents: impl FnOnce(&mut Ui) -> R,
+) -> InnerResponse<Option<R>> {
     let popup_id = button_id.with("popup");
 
     let is_popup_open = ui.memory().is_popup_open(popup_id);
@@ -211,11 +224,14 @@ fn combo_box(
     if button_response.clicked() {
         ui.memory().toggle_popup(popup_id);
     }
-    crate::popup::popup_below_widget(ui, popup_id, &button_response, |ui| {
+    let inner = crate::popup::popup_below_widget(ui, popup_id, &button_response, |ui| {
         ScrollArea::from_max_height(ui.spacing().combo_height).show(ui, menu_contents)
     });
 
-    button_response
+    InnerResponse {
+        inner,
+        response: button_response,
+    }
 }
 
 fn button_frame(
diff --git a/egui/src/containers/popup.rs b/egui/src/containers/popup.rs
index 4c11fe7d..de2978ea 100644
--- a/egui/src/containers/popup.rs
+++ b/egui/src/containers/popup.rs
@@ -41,6 +41,8 @@ impl MonoState {
 ///
 /// See also [`show_tooltip_text`].
 ///
+/// Returns `None` if the tooltip could not be placed.
+///
 /// ```
 /// # let mut ui = egui::Ui::__test();
 /// if ui.ui_contains_pointer() {
@@ -49,7 +51,7 @@ impl MonoState {
 ///     });
 /// }
 /// ```
-pub fn show_tooltip(ctx: &CtxRef, id: Id, add_contents: impl FnOnce(&mut Ui)) {
+pub fn show_tooltip<R>(ctx: &CtxRef, id: Id, add_contents: impl FnOnce(&mut Ui) -> R) -> Option<R> {
     show_tooltip_at_pointer(ctx, id, add_contents)
 }
 
@@ -59,6 +61,8 @@ pub fn show_tooltip(ctx: &CtxRef, id: Id, add_contents: impl FnOnce(&mut Ui)) {
 ///
 /// See also [`show_tooltip_text`].
 ///
+/// Returns `None` if the tooltip could not be placed.
+///
 /// ```
 /// # let mut ui = egui::Ui::__test();
 /// if ui.ui_contains_pointer() {
@@ -67,7 +71,11 @@ pub fn show_tooltip(ctx: &CtxRef, id: Id, add_contents: impl FnOnce(&mut Ui)) {
 ///     });
 /// }
 /// ```
-pub fn show_tooltip_at_pointer(ctx: &CtxRef, id: Id, add_contents: impl FnOnce(&mut Ui)) {
+pub fn show_tooltip_at_pointer<R>(
+    ctx: &CtxRef,
+    id: Id,
+    add_contents: impl FnOnce(&mut Ui) -> R,
+) -> Option<R> {
     let suggested_pos = ctx
         .input()
         .pointer
@@ -76,7 +84,12 @@ pub fn show_tooltip_at_pointer(ctx: &CtxRef, id: Id, add_contents: impl FnOnce(&
     show_tooltip_at(ctx, id, suggested_pos, add_contents)
 }
 
-pub fn show_tooltip_under(ctx: &CtxRef, id: Id, rect: &Rect, add_contents: impl FnOnce(&mut Ui)) {
+pub fn show_tooltip_under<R>(
+    ctx: &CtxRef,
+    id: Id,
+    rect: &Rect,
+    add_contents: impl FnOnce(&mut Ui) -> R,
+) -> Option<R> {
     show_tooltip_at(
         ctx,
         id,
@@ -85,12 +98,15 @@ pub fn show_tooltip_under(ctx: &CtxRef, id: Id, rect: &Rect, add_contents: impl
     )
 }
 
-pub fn show_tooltip_at(
+/// Show a tooltip at the given position.
+///
+/// Returns `None` if the tooltip could not be placed.
+pub fn show_tooltip_at<R>(
     ctx: &CtxRef,
     mut id: Id,
     suggested_position: Option<Pos2>,
-    add_contents: impl FnOnce(&mut Ui),
-) {
+    add_contents: impl FnOnce(&mut Ui) -> R,
+) -> Option<R> {
     let mut tooltip_rect = Rect::NOTHING;
 
     let position = if let Some((stored_id, stored_tooltip_rect)) = ctx.frame_state().tooltip_rect {
@@ -103,7 +119,7 @@ pub fn show_tooltip_at(
     } else if ctx.memory().everything_is_visible() {
         Pos2::default()
     } else {
-        return; // No good place for a tooltip :(
+        return None; // No good place for a tooltip :(
     };
 
     let expected_size = ctx
@@ -115,13 +131,14 @@ pub fn show_tooltip_at(
     let position = position.min(ctx.input().screen_rect().right_bottom() - expected_size);
     let position = position.max(ctx.input().screen_rect().left_top());
 
-    let response = show_tooltip_area(ctx, id, position, add_contents);
+    let InnerResponse { inner, response } = show_tooltip_area(ctx, id, position, add_contents);
     ctx.memory()
         .data_temp
         .get_mut_or_default::<crate::containers::popup::MonoState>()
         .set_tooltip_size(id, response.rect.size());
 
     ctx.frame_state().tooltip_rect = Some((id, tooltip_rect.union(response.rect)));
+    Some(inner)
 }
 
 /// Show some text at the current pointer position (if any).
@@ -130,35 +147,39 @@ pub fn show_tooltip_at(
 ///
 /// See also [`show_tooltip`].
 ///
+/// Returns `None` if the tooltip could not be placed.
+///
 /// ```
 /// # let mut ui = egui::Ui::__test();
 /// if ui.ui_contains_pointer() {
 ///     egui::show_tooltip_text(ui.ctx(), egui::Id::new("my_tooltip"), "Helpful text");
 /// }
 /// ```
-pub fn show_tooltip_text(ctx: &CtxRef, id: Id, text: impl ToString) {
+pub fn show_tooltip_text(ctx: &CtxRef, id: Id, text: impl ToString) -> Option<()> {
     show_tooltip(ctx, id, |ui| {
         ui.add(crate::widgets::Label::new(text));
     })
 }
 
 /// Show a pop-over window.
-fn show_tooltip_area(
+fn show_tooltip_area<R>(
     ctx: &CtxRef,
     id: Id,
     window_pos: Pos2,
-    add_contents: impl FnOnce(&mut Ui),
-) -> Response {
+    add_contents: impl FnOnce(&mut Ui) -> R,
+) -> InnerResponse<R> {
     use containers::*;
     Area::new(id)
         .order(Order::Tooltip)
         .fixed_pos(window_pos)
         .interactable(false)
         .show(ctx, |ui| {
-            Frame::popup(&ctx.style()).show(ui, |ui| {
-                ui.set_max_width(ui.spacing().tooltip_width);
-                add_contents(ui);
-            });
+            Frame::popup(&ctx.style())
+                .show(ui, |ui| {
+                    ui.set_max_width(ui.spacing().tooltip_width);
+                    add_contents(ui)
+                })
+                .inner
         })
 }
 
@@ -168,6 +189,8 @@ fn show_tooltip_area(
 ///
 /// You must open the popup with [`Memory::open_popup`] or  [`Memory::toggle_popup`].
 ///
+/// Returns `None` if the popup is not open.
+///
 /// ```
 /// # let ui = &mut egui::Ui::__test();
 /// let response = ui.button("Open popup");
@@ -181,32 +204,39 @@ fn show_tooltip_area(
 ///     ui.label("…");
 /// });
 /// ```
-pub fn popup_below_widget(
+pub fn popup_below_widget<R>(
     ui: &Ui,
     popup_id: Id,
     widget_response: &Response,
-    add_contents: impl FnOnce(&mut Ui),
-) {
+    add_contents: impl FnOnce(&mut Ui) -> R,
+) -> Option<R> {
     if ui.memory().is_popup_open(popup_id) {
         let parent_clip_rect = ui.clip_rect();
 
-        Area::new(popup_id)
+        let inner = Area::new(popup_id)
             .order(Order::Foreground)
             .fixed_pos(widget_response.rect.left_bottom())
             .show(ui.ctx(), |ui| {
                 ui.set_clip_rect(parent_clip_rect); // for when the combo-box is in a scroll area.
                 let frame = Frame::popup(ui.style());
                 let frame_margin = frame.margin;
-                frame.show(ui, |ui| {
-                    ui.with_layout(Layout::top_down_justified(Align::LEFT), |ui| {
-                        ui.set_width(widget_response.rect.width() - 2.0 * frame_margin.x);
-                        add_contents(ui)
-                    });
-                });
-            });
+                frame
+                    .show(ui, |ui| {
+                        ui.with_layout(Layout::top_down_justified(Align::LEFT), |ui| {
+                            ui.set_width(widget_response.rect.width() - 2.0 * frame_margin.x);
+                            add_contents(ui)
+                        })
+                        .inner
+                    })
+                    .inner
+            })
+            .inner;
 
         if ui.input().key_pressed(Key::Escape) || widget_response.clicked_elsewhere() {
             ui.memory().close_popup();
         }
+        Some(inner)
+    } else {
+        None
     }
 }
diff --git a/egui/src/containers/window.rs b/egui/src/containers/window.rs
index 5501af7a..2162367e 100644
--- a/egui/src/containers/window.rs
+++ b/egui/src/containers/window.rs
@@ -231,16 +231,21 @@ impl<'open> Window<'open> {
 }
 
 impl<'open> Window<'open> {
-    /// Returns `None` if the windows is not open (if [`Window::open`] was called with `&mut false`.
-    pub fn show(self, ctx: &CtxRef, add_contents: impl FnOnce(&mut Ui)) -> Option<Response> {
+    /// Returns `None` if the window is not open (if [`Window::open`] was called with `&mut false`).
+    /// Returns `Some(InnerResponse { inner: None })` if the window is collapsed.
+    pub fn show<R>(
+        self,
+        ctx: &CtxRef,
+        add_contents: impl FnOnce(&mut Ui) -> R,
+    ) -> Option<InnerResponse<Option<R>>> {
         self.show_impl(ctx, Box::new(add_contents))
     }
 
-    fn show_impl<'c>(
+    fn show_impl<'c, R>(
         self,
         ctx: &CtxRef,
-        add_contents: Box<dyn FnOnce(&mut Ui) + 'c>,
-    ) -> Option<Response> {
+        add_contents: Box<dyn FnOnce(&mut Ui) -> R + 'c>,
+    ) -> Option<InnerResponse<Option<R>>> {
         let Window {
             title_label,
             open,
@@ -315,7 +320,7 @@ impl<'open> Window<'open> {
 
         let mut area_content_ui = area.content_ui(ctx);
 
-        {
+        let content_inner = {
             // BEGIN FRAME --------------------------------
             let frame_stroke = frame.stroke;
             let mut frame = frame.begin(&mut area_content_ui);
@@ -342,7 +347,7 @@ impl<'open> Window<'open> {
                 None
             };
 
-            let content_response = collapsing
+            let (content_inner, content_response) = collapsing
                 .add_contents(&mut frame.content_ui, collapsing_id, |ui| {
                     resize.show(ui, |ui| {
                         if title_bar.is_some() {
@@ -350,13 +355,14 @@ impl<'open> Window<'open> {
                         }
 
                         if let Some(scroll) = scroll {
-                            scroll.show(ui, add_contents);
+                            scroll.show(ui, add_contents)
                         } else {
-                            add_contents(ui);
+                            add_contents(ui)
                         }
                     })
                 })
-                .map(|ir| ir.response);
+                .map(|ir| (Some(ir.inner), Some(ir.response)))
+                .unwrap_or((None, None));
 
             let outer_rect = frame.end(&mut area_content_ui).rect;
             paint_resize_corner(&mut area_content_ui, &possible, outer_rect, frame_stroke);
@@ -396,10 +402,15 @@ impl<'open> Window<'open> {
                     );
                 }
             }
-        }
+            content_inner
+        };
         let full_response = area.end(ctx, area_content_ui);
 
-        Some(full_response)
+        let inner_response = InnerResponse {
+            inner: content_inner,
+            response: full_response,
+        };
+        Some(inner_response)
     }
 }
 
diff --git a/egui/src/menu.rs b/egui/src/menu.rs
index 4b9f70b5..db2810a0 100644
--- a/egui/src/menu.rs
+++ b/egui/src/menu.rs
@@ -60,12 +60,22 @@ pub fn bar<R>(ui: &mut Ui, add_contents: impl FnOnce(&mut Ui) -> R) -> InnerResp
 }
 
 /// Construct a top level menu in a menu bar. This would be e.g. "File", "Edit" etc.
-pub fn menu(ui: &mut Ui, title: impl ToString, add_contents: impl FnOnce(&mut Ui)) {
+///
+/// Returns `None` if the menu is not open.
+pub fn menu<R>(
+    ui: &mut Ui,
+    title: impl ToString,
+    add_contents: impl FnOnce(&mut Ui) -> R,
+) -> Option<R> {
     menu_impl(ui, title, Box::new(add_contents))
 }
 
 #[allow(clippy::needless_pass_by_value)]
-fn menu_impl<'c>(ui: &mut Ui, title: impl ToString, add_contents: Box<dyn FnOnce(&mut Ui) + 'c>) {
+fn menu_impl<'c, R>(
+    ui: &mut Ui,
+    title: impl ToString,
+    add_contents: Box<dyn FnOnce(&mut Ui) -> R + 'c>,
+) -> Option<R> {
     let title = title.to_string();
     let bar_id = ui.id();
     let menu_id = bar_id.with(&title);
@@ -91,32 +101,42 @@ fn menu_impl<'c>(ui: &mut Ui, title: impl ToString, add_contents: Box<dyn FnOnce
         bar_state.open_menu = Some(menu_id);
     }
 
-    if bar_state.open_menu == Some(menu_id) || ui.ctx().memory().everything_is_visible() {
+    let inner = if bar_state.open_menu == Some(menu_id) || ui.ctx().memory().everything_is_visible()
+    {
         let area = Area::new(menu_id)
             .order(Order::Foreground)
             .fixed_pos(button_response.rect.left_bottom());
         let frame = Frame::menu(ui.style());
 
-        area.show(ui.ctx(), |ui| {
-            frame.show(ui, |ui| {
-                let mut style = (**ui.style()).clone();
-                style.spacing.button_padding = vec2(2.0, 0.0);
-                // style.visuals.widgets.active.bg_fill = Color32::TRANSPARENT;
-                style.visuals.widgets.active.bg_stroke = Stroke::none();
-                // style.visuals.widgets.hovered.bg_fill = Color32::TRANSPARENT;
-                style.visuals.widgets.hovered.bg_stroke = Stroke::none();
-                style.visuals.widgets.inactive.bg_fill = Color32::TRANSPARENT;
-                style.visuals.widgets.inactive.bg_stroke = Stroke::none();
-                ui.set_style(style);
-                ui.with_layout(Layout::top_down_justified(Align::LEFT), add_contents);
-            });
-        });
+        let inner = area
+            .show(ui.ctx(), |ui| {
+                frame
+                    .show(ui, |ui| {
+                        let mut style = (**ui.style()).clone();
+                        style.spacing.button_padding = vec2(2.0, 0.0);
+                        // style.visuals.widgets.active.bg_fill = Color32::TRANSPARENT;
+                        style.visuals.widgets.active.bg_stroke = Stroke::none();
+                        // style.visuals.widgets.hovered.bg_fill = Color32::TRANSPARENT;
+                        style.visuals.widgets.hovered.bg_stroke = Stroke::none();
+                        style.visuals.widgets.inactive.bg_fill = Color32::TRANSPARENT;
+                        style.visuals.widgets.inactive.bg_stroke = Stroke::none();
+                        ui.set_style(style);
+                        ui.with_layout(Layout::top_down_justified(Align::LEFT), add_contents)
+                            .inner
+                    })
+                    .inner
+            })
+            .inner;
 
         // TODO: this prevents sub-menus in menus. We should fix that.
         if ui.input().key_pressed(Key::Escape) || button_response.clicked_elsewhere() {
             bar_state.open_menu = None;
         }
-    }
+        Some(inner)
+    } else {
+        None
+    };
 
     bar_state.save(ui.ctx(), bar_id);
+    inner
 }
diff --git a/egui/src/widgets/color_picker.rs b/egui/src/widgets/color_picker.rs
index ed359492..2babad70 100644
--- a/egui/src/widgets/color_picker.rs
+++ b/egui/src/widgets/color_picker.rs
@@ -355,7 +355,8 @@ pub fn color_edit_button_hsva(ui: &mut Ui, hsva: &mut Hsva, alpha: Alpha) -> Res
                         button_response.mark_changed();
                     }
                 });
-            });
+            })
+            .response;
 
         if !button_response.clicked()
             && (ui.input().key_pressed(Key::Escape) || area_response.clicked_elsewhere())