refactor:extract popup builder and dimensions

adapters/src/rendering/femtovg/popup_window.rs

@@ -95,6 +95,10 @@ impl PopupWindow {
     pub fn scale_factor(&self) -> f32 {
         self.scale_factor.get()
     }
+
+    pub fn popup_key(&self) -> Option<usize> {
+        self.popup_key.get()
+    }
 }
 
 impl WindowAdapter for PopupWindow {

adapters/src/wayland/shell_adapter.rs

@@ -180,7 +180,7 @@ impl WaylandWindowingSystem {
 
             let serial = serial_holder.get();
 
-            let params = if let Some((request, width, height)) = popup_manager_clone.take_pending_popup_request() {
+            let params = if let Some((request, width, height)) = popup_manager_clone.take_pending_popup() {
                 log::info!(
                     "Using popup request: component='{}', position=({}, {}), size={}x{}, mode={:?}",
                     request.component,

adapters/src/wayland/surfaces/mod.rs

@@ -1,5 +1,6 @@
 pub mod dimensions;
 pub mod layer_surface;
+pub mod popup_builder;
 pub mod popup_manager;
 pub mod popup_surface;
 pub mod surface_builder;

adapters/src/wayland/surfaces/popup_builder.rs

@@ -0,0 +1,200 @@
+use crate::errors::{LayerShikaError, Result};
+use crate::rendering::femtovg::popup_window::PopupWindow;
+use layer_shika_domain::value_objects::popup_dimensions::PopupDimensions;
+use layer_shika_domain::value_objects::popup_request::{PopupHandle, PopupRequest, PopupSize};
+use log::{debug, info};
+use slint::ComponentHandle;
+use slint_interpreter::{ComponentDefinition, ComponentInstance, Value};
+use smithay_client_toolkit::reexports::protocols_wlr::layer_shell::v1::client::zwlr_layer_surface_v1::ZwlrLayerSurfaceV1;
+use std::rc::Rc;
+use wayland_client::QueueHandle;
+
+use super::popup_manager::{CreatePopupParams, PopupManager};
+use super::surface_state::WindowState;
+
+pub struct PopupBuilder<'a> {
+    popup_manager: &'a Rc<PopupManager>,
+    queue_handle: &'a QueueHandle<WindowState>,
+    parent_layer_surface: &'a ZwlrLayerSurfaceV1,
+}
+
+impl<'a> PopupBuilder<'a> {
+    #[must_use]
+    pub const fn new(
+        popup_manager: &'a Rc<PopupManager>,
+        queue_handle: &'a QueueHandle<WindowState>,
+        parent_layer_surface: &'a ZwlrLayerSurfaceV1,
+    ) -> Self {
+        Self {
+            popup_manager,
+            queue_handle,
+            parent_layer_surface,
+        }
+    }
+
+    pub fn build(
+        &self,
+        component_def: &ComponentDefinition,
+        request: &PopupRequest,
+        serial: u32,
+    ) -> Result<PopupHandle> {
+        info!(
+            "Building popup for component '{}' at position ({}, {}) with mode {:?}",
+            request.component,
+            request.at.position().0,
+            request.at.position().1,
+            request.mode
+        );
+
+        let dimensions = Self::resolve_dimensions(component_def, &request.size)?;
+        dimensions
+            .validate()
+            .map_err(|e| LayerShikaError::WindowConfiguration {
+                message: format!("Invalid popup dimensions: {e}"),
+            })?;
+
+        debug!(
+            "Resolved popup dimensions: {}x{}",
+            dimensions.width(),
+            dimensions.height()
+        );
+
+        let params = CreatePopupParams {
+            last_pointer_serial: serial,
+            reference_x: request.at.position().0,
+            reference_y: request.at.position().1,
+            width: dimensions.width(),
+            height: dimensions.height(),
+            positioning_mode: request.mode,
+        };
+
+        let popup_window = self.popup_manager.create_popup(
+            self.queue_handle,
+            self.parent_layer_surface,
+            params,
+        )?;
+
+        let instance = Self::create_component_instance(component_def, &popup_window)?;
+
+        self.setup_popup_callbacks(&instance, &popup_window)?;
+
+        let handle = PopupHandle::new(popup_window.popup_key().ok_or_else(|| {
+            LayerShikaError::WindowConfiguration {
+                message: "Popup window has no key assigned".to_string(),
+            }
+        })?);
+
+        info!("Popup built successfully with handle {:?}", handle);
+
+        Ok(handle)
+    }
+
+    fn resolve_dimensions(
+        component_def: &ComponentDefinition,
+        size: &PopupSize,
+    ) -> Result<PopupDimensions> {
+        match size {
+            PopupSize::Fixed { w, h } => {
+                debug!("Using fixed popup size: {}x{}", w, h);
+                Ok(PopupDimensions::new(*w, *h))
+            }
+            PopupSize::Content => {
+                debug!("Measuring popup dimensions from component content");
+                Self::measure_component_dimensions(component_def)
+            }
+        }
+    }
+
+    fn measure_component_dimensions(
+        component_def: &ComponentDefinition,
+    ) -> Result<PopupDimensions> {
+        debug!("Creating temporary component instance to measure dimensions");
+
+        let temp_instance =
+            component_def
+                .create()
+                .map_err(|e| LayerShikaError::WindowConfiguration {
+                    message: format!(
+                        "Failed to create temporary instance for dimension measurement: {}",
+                        e
+                    ),
+                })?;
+
+        temp_instance
+            .show()
+            .map_err(|e| LayerShikaError::WindowConfiguration {
+                message: format!("Failed to show temporary instance: {}", e),
+            })?;
+
+        let width = Self::read_property(&temp_instance, "popup-width", 120.0);
+        let height = Self::read_property(&temp_instance, "popup-height", 120.0);
+
+        debug!(
+            "Measured dimensions from component properties: {}x{}",
+            width, height
+        );
+
+        Ok(PopupDimensions::new(width, height))
+    }
+
+    fn read_property(instance: &ComponentInstance, name: &str, default: f32) -> f32 {
+        instance
+            .get_property(name)
+            .ok()
+            .and_then(|v| v.try_into().ok())
+            .unwrap_or_else(|| {
+                debug!(
+                    "Property '{}' not found or invalid, using default: {}",
+                    name, default
+                );
+                default
+            })
+    }
+
+    fn create_component_instance(
+        component_def: &ComponentDefinition,
+        _popup_window: &Rc<PopupWindow>,
+    ) -> Result<ComponentInstance> {
+        debug!("Creating popup component instance");
+
+        let instance =
+            component_def
+                .create()
+                .map_err(|e| LayerShikaError::WindowConfiguration {
+                    message: format!("Failed to create popup instance: {}", e),
+                })?;
+
+        instance
+            .show()
+            .map_err(|e| LayerShikaError::WindowConfiguration {
+                message: format!("Failed to show popup instance: {}", e),
+            })?;
+
+        debug!("Popup component instance created and shown successfully");
+
+        Ok(instance)
+    }
+
+    fn setup_popup_callbacks(
+        &self,
+        instance: &ComponentInstance,
+        _popup_window: &Rc<PopupWindow>,
+    ) -> Result<()> {
+        debug!("Setting up popup callbacks");
+
+        let popup_manager = Rc::clone(self.popup_manager);
+        instance
+            .set_callback("closed", move |_| {
+                info!("Popup 'closed' callback triggered");
+                popup_manager.close_current_popup();
+                Value::Void
+            })
+            .map_err(|e| LayerShikaError::WindowConfiguration {
+                message: format!("Failed to set popup 'closed' callback: {}", e),
+            })?;
+
+        debug!("Popup callbacks configured successfully");
+
+        Ok(())
+    }
+}

adapters/src/wayland/surfaces/popup_manager.rs

@@ -68,54 +68,84 @@ struct ActivePopup {
     window: Rc<PopupWindow>,
 }
 
+struct PopupState {
+    scale_factor: f32,
+    output_size: PhysicalSize,
+}
+
+struct PendingPopup {
+    request: PopupRequest,
+    width: f32,
+    height: f32,
+}
+
 pub struct PopupManager {
     context: PopupContext,
     popups: RefCell<Slab<ActivePopup>>,
-    current_scale_factor: RefCell<f32>,
-    current_output_size: RefCell<PhysicalSize>,
-    pending_popup_request: RefCell<Option<(PopupRequest, f32, f32)>>,
-    last_popup_key: RefCell<Option<usize>>,
+    state: RefCell<PopupState>,
+    current_popup_key: RefCell<Option<usize>>,
+    pending_popup: RefCell<Option<PendingPopup>>,
 }
 
 impl PopupManager {
     #[must_use]
-    pub const fn new(context: PopupContext, initial_scale_factor: f32) -> Self {
+    pub fn new(context: PopupContext, initial_scale_factor: f32) -> Self {
         Self {
             context,
             popups: RefCell::new(Slab::new()),
-            current_scale_factor: RefCell::new(initial_scale_factor),
-            current_output_size: RefCell::new(PhysicalSize::new(0, 0)),
-            pending_popup_request: RefCell::new(None),
-            last_popup_key: RefCell::new(None),
+            state: RefCell::new(PopupState {
+                scale_factor: initial_scale_factor,
+                output_size: PhysicalSize::new(0, 0),
+            }),
+            current_popup_key: RefCell::new(None),
+            pending_popup: RefCell::new(None),
         }
     }
 
-    pub fn set_pending_popup_request(&self, request: PopupRequest, width: f32, height: f32) {
-        *self.pending_popup_request.borrow_mut() = Some((request, width, height));
+    pub fn set_pending_popup(&self, request: PopupRequest, width: f32, height: f32) {
+        *self.pending_popup.borrow_mut() = Some(PendingPopup {
+            request,
+            width,
+            height,
+        });
     }
 
     #[must_use]
-    pub fn take_pending_popup_request(&self) -> Option<(PopupRequest, f32, f32)> {
-        self.pending_popup_request.borrow_mut().take()
+    pub fn take_pending_popup(&self) -> Option<(PopupRequest, f32, f32)> {
+        self.pending_popup
+            .borrow_mut()
+            .take()
+            .map(|p| (p.request, p.width, p.height))
+    }
+
+    #[must_use]
+    pub fn scale_factor(&self) -> f32 {
+        self.state.borrow().scale_factor
+    }
+
+    #[must_use]
+    pub fn output_size(&self) -> PhysicalSize {
+        self.state.borrow().output_size
+    }
+
+    pub fn update_scale_factor(&self, scale_factor: f32) {
+        self.state.borrow_mut().scale_factor = scale_factor;
+    }
+
+    pub fn update_output_size(&self, output_size: PhysicalSize) {
+        self.state.borrow_mut().output_size = output_size;
     }
 
     pub fn close_current_popup(&self) {
-        let key = self.last_popup_key.borrow_mut().take();
+        let key = self.current_popup_key.borrow_mut().take();
         if let Some(key) = key {
             self.destroy_popup(key);
         }
     }
 
-    pub fn update_scale_factor(&self, scale_factor: f32) {
-        *self.current_scale_factor.borrow_mut() = scale_factor;
-    }
-
-    pub fn update_output_size(&self, output_size: PhysicalSize) {
-        *self.current_output_size.borrow_mut() = output_size;
-    }
-
-    pub fn output_size(&self) -> PhysicalSize {
-        *self.current_output_size.borrow()
+    #[must_use]
+    pub fn current_popup_key(&self) -> Option<usize> {
+        *self.current_popup_key.borrow()
     }
 
     pub fn create_popup(
@@ -130,7 +160,7 @@ impl PopupManager {
             }
         })?;
 
-        let scale_factor = *self.current_scale_factor.borrow();
+        let scale_factor = self.scale_factor();
         info!(
             "Creating popup window with scale factor {scale_factor}, reference=({}, {}), size=({} x {}), mode={:?}",
             params.reference_x,
@@ -192,7 +222,7 @@ impl PopupManager {
             window: Rc::clone(&popup_window),
         });
         popup_window.set_popup_manager(Rc::downgrade(self), key);
-        *self.last_popup_key.borrow_mut() = Some(key);
+        *self.current_popup_key.borrow_mut() = Some(key);
 
         info!("Popup window created successfully with key {key}");
 

composition/src/system.rs

@@ -132,7 +132,7 @@ impl RuntimeState<'_> {
         self.window_state.compilation_result()
     }
 
-    pub fn show_popup(&mut self, req: PopupRequest) -> Result<()> {
+    pub fn show_popup(&mut self, req: PopupRequest) -> Result<PopupHandle> {
         let compilation_result = self.compilation_result().ok_or_else(|| {
             Error::Domain(DomainError::Configuration {
                 message: "No compilation result available for popup creation".to_string(),
@@ -172,7 +172,7 @@ impl RuntimeState<'_> {
             .map(Rc::clone)?;
 
         log::debug!(
-            "Setting pending popup request for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}",
+            "Creating popup for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}",
             req.component,
             width,
             height,
@@ -181,11 +181,17 @@ impl RuntimeState<'_> {
             req.mode
         );
 
-        popup_manager.set_pending_popup_request(req, width, height);
+        popup_manager.set_pending_popup(req.clone(), width, height);
 
         Self::create_popup_instance(&definition, &popup_manager)?;
 
-        Ok(())
+        Ok(PopupHandle::new(
+            popup_manager.current_popup_key().ok_or_else(|| {
+                Error::Domain(DomainError::Configuration {
+                    message: "No popup key available after creation".to_string(),
+                })
+            })?,
+        ))
     }
 
     pub fn close_popup(&mut self, handle: PopupHandle) -> Result<()> {

domain/src/value_objects/mod.rs

@@ -4,5 +4,6 @@ pub mod keyboard_interactivity;
 pub mod layer;
 pub mod margins;
 pub mod popup_config;
+pub mod popup_dimensions;
 pub mod popup_positioning_mode;
 pub mod popup_request;

domain/src/value_objects/popup_dimensions.rs

@@ -0,0 +1,42 @@
+use crate::errors::{DomainError, Result};
+
+#[derive(Debug, Clone, Copy, PartialEq)]
+pub struct PopupDimensions {
+    width: f32,
+    height: f32,
+}
+
+impl PopupDimensions {
+    #[must_use]
+    pub const fn new(width: f32, height: f32) -> Self {
+        Self { width, height }
+    }
+
+    #[must_use]
+    pub const fn width(&self) -> f32 {
+        self.width
+    }
+
+    #[must_use]
+    pub const fn height(&self) -> f32 {
+        self.height
+    }
+
+    pub fn validate(&self) -> Result<()> {
+        if self.width <= 0.0 || self.height <= 0.0 {
+            return Err(DomainError::Configuration {
+                message: format!(
+                    "Invalid popup dimensions: width={}, height={}. Both must be positive.",
+                    self.width, self.height
+                ),
+            });
+        }
+        Ok(())
+    }
+}
+
+impl Default for PopupDimensions {
+    fn default() -> Self {
+        Self::new(120.0, 120.0)
+    }
+}