fix open/close behavior using popup logic

crates/viewer/re_ui/examples/re_ui_example/main.rs

@@ -143,8 +143,6 @@ impl eframe::App for ExampleApp {
 
     fn update(&mut self, egui_ctx: &egui::Context, _frame: &mut eframe::Frame) {
         self.show_text_logs_as_notifications();
-        self.notifications
-            .ui(egui_ctx, &mut self.show_notification_panel);
 
         self.top_bar(egui_ctx);
 
@@ -418,11 +416,7 @@ impl ExampleApp {
                 &mut self.show_left_panel,
             );
 
-            notifications::notification_toggle_button(
-                ui,
-                &mut self.show_notification_panel,
-                self.notifications.unread_notification_level(),
-            );
+            notifications::notification_toggle_button(ui, &mut self.notifications);
         });
     }
 }

crates/viewer/re_ui/src/notifications.rs

@@ -55,19 +55,29 @@ fn is_relevant(target: &str, level: re_log::Level) -> bool {
     )
 }
 
-pub fn notification_toggle_button(
-    ui: &mut egui::Ui,
-    show_notification_panel: &mut bool,
-    unread_notification_level: Option<NotificationLevel>,
-) {
-    let response = ui.medium_icon_toggle_button(&icons::NOTIFICATION, show_notification_panel);
+fn notification_panel_popup_id() -> egui::Id {
+    egui::Id::new("notification_panel_popup")
+}
+
+pub fn notification_toggle_button(ui: &mut egui::Ui, notification_ui: &mut NotificationUi) {
+    let popup_id = notification_panel_popup_id();
+
+    let is_panel_visible = ui.memory(|mem| mem.is_popup_open(popup_id));
+    let button_response =
+        ui.medium_icon_toggle_button(&icons::NOTIFICATION, &mut is_panel_visible.clone());
 
-    if let Some(level) = unread_notification_level {
-        let pos = response.rect.right_top() + egui::vec2(-2.0, 2.0);
+    if button_response.clicked() {
+        ui.memory_mut(|mem| mem.toggle_popup(popup_id));
+    }
+
+    if let Some(level) = notification_ui.unread_notification_level {
+        let pos = button_response.rect.right_top() + egui::vec2(-2.0, 2.0);
         let radius = 3.0;
         let color = level.color(ui);
         ui.painter().circle_filled(pos, radius, color);
     }
+
+    notification_ui.ui(ui.ctx(), &button_response);
 }
 
 struct Notification {
@@ -91,7 +101,6 @@ pub struct NotificationUi {
     notifications: Vec<Notification>,
 
     unread_notification_level: Option<NotificationLevel>,
-
     was_open_last_frame: bool,
 
     /// Panel that shows all notifications.
@@ -149,31 +158,36 @@ impl NotificationUi {
         }
     }
 
-    pub fn ui(&mut self, egui_ctx: &egui::Context, is_panel_visible: &mut bool) {
-        if *is_panel_visible {
-            // Opening panel is the same as dismissing all toasts
+    fn ui(&mut self, egui_ctx: &egui::Context, button_response: &egui::Response) {
+        let is_panel_visible =
+            egui_ctx.memory(|mem| mem.is_popup_open(notification_panel_popup_id()));
+        if is_panel_visible {
+            // Dismiss all toasts when opening panel
             self.unread_notification_level = None;
             for notification in &mut self.notifications {
                 notification.toast_ttl = Duration::ZERO;
             }
-        } else if self.was_open_last_frame {
+        }
+        if !is_panel_visible && self.was_open_last_frame {
+            // Mark all as read after closing panel
             for notification in &mut self.notifications {
                 notification.is_unread = false;
             }
         }
-        self.was_open_last_frame = *is_panel_visible;
+        self.was_open_last_frame = is_panel_visible;
 
-        self.panel
-            .show(egui_ctx, &mut self.notifications, is_panel_visible);
-        self.toasts.show(egui_ctx, &mut self.notifications[..]);
-
-        if *is_panel_visible {
+        if is_panel_visible {
+            let panel_response = self.panel.show(egui_ctx, &mut self.notifications);
             let escape_pressed =
                 egui_ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape));
-            if escape_pressed {
-                *is_panel_visible = false;
+            if escape_pressed
+                || button_response.clicked_elsewhere() && panel_response.clicked_elsewhere()
+            {
+                egui_ctx.memory_mut(|mem| mem.close_popup());
             }
         }
+
+        self.toasts.show(egui_ctx, &mut self.notifications[..]);
     }
 }
 
@@ -192,12 +206,7 @@ impl NotificationPanel {
         &self,
         egui_ctx: &egui::Context,
         notifications: &mut Vec<Notification>,
-        is_panel_visible: &mut bool,
-    ) {
-        if !*is_panel_visible {
-            return;
-        }
-
+    ) -> egui::Response {
         let panel_width = 356.0;
         let panel_max_height = (egui_ctx.screen_rect().height() - 100.0)
             .at_least(0.0)
@@ -247,7 +256,7 @@ impl NotificationPanel {
                             }
                             ui.with_layout(egui::Layout::top_down(egui::Align::Max), |ui| {
                                 if ui.small_icon_button(&icons::CLOSE).clicked() {
-                                    *is_panel_visible = false;
+                                    ui.memory_mut(|mem| mem.close_popup());
                                 }
                             });
                         });
@@ -267,15 +276,13 @@ impl NotificationPanel {
             })
             .response;
 
-        if response.clicked_elsewhere() {
-            *is_panel_visible = false;
-        }
-
         if dismiss_all {
             notifications.clear();
         } else if let Some(to_dismiss) = to_dismiss {
             notifications.remove(to_dismiss);
         }
+
+        response
     }
 }
 

crates/viewer/re_viewer/src/app.rs

@@ -195,7 +195,6 @@ pub struct App {
 
     /// Notification panel.
     pub(crate) notifications: notifications::NotificationUi,
-    pub(crate) notifications_panel_open: bool,
 
     memory_panel: crate::memory_panel::MemoryPanel,
     memory_panel_open: bool,
@@ -335,7 +334,6 @@ impl App {
                 &crate::app_blueprint::setup_welcome_screen_blueprint,
             )),
             notifications: notifications::NotificationUi::new(),
-            notifications_panel_open: false,
 
             memory_panel: Default::default(),
             memory_panel_open: false,
@@ -1065,16 +1063,6 @@ impl App {
 
                 crate::ui::mobile_warning_ui(ui);
 
-                // NOTE: We do this here because the `notifications` UI is on top,
-                //       and may want to consume some key events.
-                // This should come _before_ `top_panel`, otherwise the notification panel
-                // is closed when the toggle button is clicked.
-                self.show_text_logs_as_notifications();
-                if !self.screenshotter.is_screenshotting() {
-                    self.notifications
-                        .ui(egui_ctx, &mut self.notifications_panel_open);
-                }
-
                 crate::ui::top_panel(
                     frame,
                     self,
@@ -1127,6 +1115,8 @@ impl App {
                         render_ctx.before_submit();
                     }
                 }
+
+                self.show_text_logs_as_notifications();
             });
     }
 

crates/viewer/re_viewer/src/ui/top_panel.rs

@@ -312,11 +312,7 @@ fn panel_buttons_r2l(app: &mut App, app_blueprint: &AppBlueprint<'_>, ui: &mut e
         app_blueprint.toggle_blueprint_panel(&app.command_sender);
     }
 
-    re_ui::notifications::notification_toggle_button(
-        ui,
-        &mut app.notifications_panel_open,
-        app.notifications.unread_notification_level(),
-    );
+    re_ui::notifications::notification_toggle_button(ui, &mut app.notifications);
 }
 
 /// Shows clickable website link as an image (text doesn't look as nice)