fix(popup): re-position on resize due to content change

src/bar.rs

@@ -10,7 +10,6 @@ use gtk::gdk::Monitor;
 use gtk::prelude::*;
 use gtk::{Application, ApplicationWindow, IconTheme, Orientation, Window, WindowType};
 use gtk_layer_shell::LayerShell;
-use std::cell::RefCell;
 use std::rc::Rc;
 use std::time::Duration;
 use tracing::{debug, info};
@@ -18,7 +17,7 @@ use tracing::{debug, info};
 #[derive(Debug, Clone)]
 enum Inner {
     New { config: Option<Config> },
-    Loaded { popup: Rc<RefCell<Popup>> },
+    Loaded { popup: Rc<Popup> },
 }
 
 #[derive(Debug, Clone)]
@@ -269,7 +268,7 @@ impl Bar {
 
         // popup ignores module location so can bodge this for now
         let popup = Popup::new(&info!(ModuleLocation::Left), config.popup_gap);
-        let popup = Rc::new(RefCell::new(popup));
+        let popup = Rc::new(popup);
 
         if let Some(modules) = config.start {
             let info = info!(ModuleLocation::Left);
@@ -315,7 +314,7 @@ impl Bar {
         &self.monitor_name
     }
 
-    pub fn popup(&self) -> Rc<RefCell<Popup>> {
+    pub fn popup(&self) -> Rc<Popup> {
         match &self.inner {
             Inner::New { .. } => {
                 panic!("Attempted to get popup of uninitialized bar. This is a serious bug!")
@@ -339,7 +338,7 @@ fn create_container(name: &str, orientation: Orientation) -> gtk::Box {
 
 #[derive(Debug)]
 struct BarLoadResult {
-    popup: Rc<RefCell<Popup>>,
+    popup: Rc<Popup>,
 }
 
 /// Adds modules into a provided GTK box,
@@ -349,7 +348,7 @@ fn add_modules(
     modules: Vec<ModuleConfig>,
     info: &ModuleInfo,
     ironbar: &Rc<Ironbar>,
-    popup: &Rc<RefCell<Popup>>,
+    popup: &Rc<Popup>,
 ) -> Result<()> {
     let orientation = info.bar_position.orientation();
 

src/ipc/server.rs

@@ -167,13 +167,13 @@ impl Ipc {
                 match bar {
                     Some(bar) => {
                         let popup = bar.popup();
-                        let current_widget = popup.borrow().current_widget();
+                        let current_widget = popup.current_widget();
 
-                        popup.borrow_mut().hide();
+                        popup.hide();
 
                         let data = popup
-                            .borrow()
                             .cache
+                            .borrow()
                             .iter()
                             .find(|(_, value)| value.name == name)
                             .map(|(id, value)| (*id, value.content.buttons.first().cloned()));
@@ -181,7 +181,6 @@ impl Ipc {
                         match data {
                             Some((id, Some(button))) if current_widget != Some(id) => {
                                 let button_id = button.popup_id();
-                                let mut popup = popup.borrow_mut();
 
                                 if popup.is_visible() {
                                     popup.hide();
@@ -207,11 +206,11 @@ impl Ipc {
                         let popup = bar.popup();
 
                         // only one popup per bar, so hide if open for another widget
-                        popup.borrow_mut().hide();
+                        popup.hide();
 
                         let data = popup
-                            .borrow()
                             .cache
+                            .borrow()
                             .iter()
                             .find(|(_, value)| value.name == name)
                             .map(|(id, value)| (*id, value.content.buttons.first().cloned()));
@@ -219,7 +218,7 @@ impl Ipc {
                         match data {
                             Some((id, Some(button))) => {
                                 let button_id = button.popup_id();
-                                popup.borrow_mut().show(id, button_id);
+                                popup.show(id, button_id);
 
                                 Response::Ok
                             }
@@ -236,7 +235,7 @@ impl Ipc {
                 match bar {
                     Some(bar) => {
                         let popup = bar.popup();
-                        popup.borrow_mut().hide();
+                        popup.hide();
 
                         Response::Ok
                     }

src/modules/mod.rs

@@ -1,4 +1,3 @@
-use std::cell::RefCell;
 use std::fmt::Debug;
 use std::rc::Rc;
 use std::sync::Arc;
@@ -215,7 +214,7 @@ pub fn create_module<TModule, TWidget, TSend, TRec>(
     ironbar: Rc<Ironbar>,
     name: Option<String>,
     info: &ModuleInfo,
-    popup: &Rc<RefCell<Popup>>,
+    popup: &Rc<Popup>,
 ) -> Result<ModuleParts<TWidget>>
 where
     TModule: Module<TWidget, SendMessage = TSend, ReceiveMessage = TRec>,
@@ -251,7 +250,7 @@ where
             .style_context()
             .add_class(&format!("popup-{module_name}"));
 
-        register_popup_content(popup, id, instance_name, popup_content);
+        popup.register_content(id, instance_name, popup_content);
     }
 
     setup_receiver(tx, ui_rx, popup.clone(), module_name, id);
@@ -259,16 +258,6 @@ where
     Ok(module_parts)
 }
 
-/// Registers the popup content with the popup.
-fn register_popup_content(
-    popup: &Rc<RefCell<Popup>>,
-    id: usize,
-    name: String,
-    popup_content: ModulePopupParts,
-) {
-    popup.borrow_mut().register_content(id, name, popup_content);
-}
-
 /// Sets up the bridge channel receiver
 /// to pick up events from the controller, widget or popup.
 ///
@@ -277,7 +266,7 @@ fn register_popup_content(
 fn setup_receiver<TSend>(
     tx: broadcast::Sender<TSend>,
     rx: mpsc::Receiver<ModuleUpdateEvent<TSend>>,
-    popup: Rc<RefCell<Popup>>,
+    popup: Rc<Popup>,
     name: &'static str,
     id: usize,
 ) where
@@ -294,7 +283,6 @@ fn setup_receiver<TSend>(
             }
             ModuleUpdateEvent::TogglePopup(button_id) => {
                 debug!("Toggling popup for {} [#{}]", name, id);
-                let mut popup = popup.borrow_mut();
                 if popup.is_visible() {
                     popup.hide();
                 } else {
@@ -309,8 +297,6 @@ fn setup_receiver<TSend>(
             }
             ModuleUpdateEvent::OpenPopup(button_id) => {
                 debug!("Opening popup for {} [#{}]", name, id);
-
-                let mut popup = popup.borrow_mut();
                 popup.hide();
                 popup.show(id, button_id);
 
@@ -324,7 +310,6 @@ fn setup_receiver<TSend>(
             ModuleUpdateEvent::OpenPopupAt(geometry) => {
                 debug!("Opening popup for {} [#{}]", name, id);
 
-                let mut popup = popup.borrow_mut();
                 popup.hide();
                 popup.show_at(id, geometry);
 
@@ -336,8 +321,6 @@ fn setup_receiver<TSend>(
             }
             ModuleUpdateEvent::ClosePopup => {
                 debug!("Closing popup for {} [#{}]", name, id);
-
-                let mut popup = popup.borrow_mut();
                 popup.hide();
             }
         }

src/popup.rs

@@ -1,11 +1,13 @@
 use glib::Propagation;
+use std::cell::RefCell;
 use std::collections::HashMap;
+use std::rc::Rc;
 
 use gtk::gdk::Monitor;
 use gtk::prelude::*;
-use gtk::{ApplicationWindow, Orientation};
+use gtk::{ApplicationWindow, Button, Orientation};
 use gtk_layer_shell::LayerShell;
-use tracing::debug;
+use tracing::{debug, trace};
 
 use crate::config::BarPosition;
 use crate::gtk_helpers::{IronbarGtkExt, WidgetGeometry};
@@ -21,10 +23,10 @@ pub struct PopupCacheValue {
 #[derive(Debug, Clone)]
 pub struct Popup {
     pub window: ApplicationWindow,
-    pub cache: HashMap<usize, PopupCacheValue>,
+    pub cache: Rc<RefCell<HashMap<usize, PopupCacheValue>>>,
     monitor: Monitor,
     pos: BarPosition,
-    current_widget: Option<usize>,
+    current_widget: Rc<RefCell<Option<(usize, usize)>>>,
 }
 
 impl Popup {
@@ -33,7 +35,7 @@ impl Popup {
     /// and an empty `gtk::Box` container.
     pub fn new(module_info: &ModuleInfo, gap: i32) -> Self {
         let pos = module_info.bar_position;
-        let orientation = pos.get_orientation();
+        let orientation = pos.orientation();
 
         let win = ApplicationWindow::builder()
             .application(module_info.app)
@@ -104,14 +106,14 @@ impl Popup {
 
         Self {
             window: win,
-            cache: HashMap::new(),
+            cache: Rc::new(RefCell::new(HashMap::new())),
             monitor: module_info.monitor.clone(),
             pos,
-            current_widget: None,
+            current_widget: Rc::new(RefCell::new(None)),
         }
     }
 
-    pub fn register_content(&mut self, key: usize, name: String, content: ModulePopupParts) {
+    pub fn register_content(&self, key: usize, name: String, content: ModulePopupParts) {
         debug!("Registered popup content for #{}", key);
 
         for button in &content.buttons {
@@ -119,45 +121,94 @@ impl Popup {
             button.set_tag("popup-id", id);
         }
 
-        self.cache.insert(key, PopupCacheValue { name, content });
+        let orientation = self.pos.orientation();
+        let monitor = self.monitor.clone();
+        let window = self.window.clone();
+
+        let current_widget = self.current_widget.clone();
+        let cache = self.cache.clone();
+
+        content
+            .container
+            .connect_size_allocate(move |container, rect| {
+                if container.is_visible() {
+                    trace!("Resized:  {}x{}", rect.width(), rect.height());
+
+                    if let Some((widget_id, button_id)) = *current_widget.borrow() {
+                        if let Some(PopupCacheValue { content, .. }) =
+                            cache.borrow().get(&widget_id)
+                        {
+                            Self::set_position(
+                                &content.buttons,
+                                button_id,
+                                orientation,
+                                &monitor,
+                                &window,
+                            );
+                        }
+                    }
+                }
+            });
+
+        self.cache
+            .borrow_mut()
+            .insert(key, PopupCacheValue { name, content });
     }
 
-    pub fn show(&mut self, widget_id: usize, button_id: usize) {
+    pub fn show(&self, widget_id: usize, button_id: usize) {
         self.clear_window();
 
-        if let Some(PopupCacheValue { content, .. }) = self.cache.get(&widget_id) {
+        if let Some(PopupCacheValue { content, .. }) = self.cache.borrow().get(&widget_id) {
-            self.current_widget = Some(widget_id);
+            *self.current_widget.borrow_mut() = Some((widget_id, button_id));
 
             content.container.style_context().add_class("popup");
             self.window.add(&content.container);
 
             self.window.show();
 
-            let button = content
+            Self::set_position(
-                .buttons
+                &content.buttons,
-                .iter()
+                button_id,
-                .find(|b| b.popup_id() == button_id)
+                self.pos.orientation(),
-                .expect("to find valid button");
+                &self.monitor,
-
+                &self.window,
-            let orientation = self.pos.get_orientation();
+            );
-            let geometry = button.geometry(orientation);
-
-            self.set_pos(geometry);
         }
     }
 
     pub fn show_at(&self, widget_id: usize, geometry: WidgetGeometry) {
         self.clear_window();
 
-        if let Some(PopupCacheValue { content, .. }) = self.cache.get(&widget_id) {
+        if let Some(PopupCacheValue { content, .. }) = self.cache.borrow().get(&widget_id) {
             content.container.style_context().add_class("popup");
             self.window.add(&content.container);
 
             self.window.show();
-            self.set_pos(geometry);
+            Self::set_pos(
+                geometry,
+                self.pos.orientation(),
+                &self.monitor,
+                &self.window,
+            );
         }
     }
 
+    fn set_position(
+        buttons: &[Button],
+        button_id: usize,
+        orientation: Orientation,
+        monitor: &Monitor,
+        window: &ApplicationWindow,
+    ) {
+        let button = buttons
+            .iter()
+            .find(|b| b.popup_id() == button_id)
+            .expect("to find valid button");
+
+        let geometry = button.geometry(orientation);
+        Self::set_pos(geometry, orientation, monitor, window);
+    }
+
     fn clear_window(&self) {
         let children = self.window.children();
         for child in children {
@@ -166,8 +217,8 @@ impl Popup {
     }
 
     /// Hides the popover
-    pub fn hide(&mut self) {
+    pub fn hide(&self) {
-        self.current_widget = None;
+        *self.current_widget.borrow_mut() = None;
         self.window.hide();
     }
 
@@ -177,22 +228,25 @@ impl Popup {
     }
 
     pub fn current_widget(&self) -> Option<usize> {
-        self.current_widget
+        self.current_widget.borrow().map(|w| w.0)
     }
 
     /// Sets the popup's X/Y position relative to the left or border of the screen
     /// (depending on orientation).
-    fn set_pos(&self, geometry: WidgetGeometry) {
+    fn set_pos(
-        let orientation = self.pos.get_orientation();
+        geometry: WidgetGeometry,
-
+        orientation: Orientation,
-        let mon_workarea = self.monitor.workarea();
+        monitor: &Monitor,
+        window: &ApplicationWindow,
+    ) {
+        let mon_workarea = monitor.workarea();
         let screen_size = if orientation == Orientation::Horizontal {
             mon_workarea.width()
         } else {
             mon_workarea.height()
         };
 
-        let (popup_width, popup_height) = self.window.size();
+        let (popup_width, popup_height) = window.size();
         let popup_size = if orientation == Orientation::Horizontal {
             popup_width
         } else {
@@ -217,6 +271,6 @@ impl Popup {
             gtk_layer_shell::Edge::Top
         };
 
-        self.window.set_layer_shell_margin(edge, offset as i32);
+        window.set_layer_shell_margin(edge, offset as i32);
     }
 }