From fefb5a4ef3b92132530840778dab5b99e89bebd1 Mon Sep 17 00:00:00 2001 From: drendog Date: Mon, 15 Dec 2025 02:34:50 +0100 Subject: [PATCH] refactor: popup lifecycle and add more docs --- .../src/rendering/femtovg/popup_window.rs | 86 ++++-- .../src/wayland/surfaces/event_context.rs | 14 + .../src/wayland/surfaces/popup_manager.rs | 7 +- .../src/wayland/surfaces/surface_state.rs | 4 + crates/composition/src/layer_shika.rs | 7 +- crates/composition/src/popup_builder.rs | 216 ++----------- crates/composition/src/shell.rs | 11 +- crates/composition/src/system.rs | 284 +++++++++++++----- src/lib.rs | 2 +- src/window.rs | 4 +- 10 files changed, 329 insertions(+), 306 deletions(-) diff --git a/crates/adapters/src/rendering/femtovg/popup_window.rs b/crates/adapters/src/rendering/femtovg/popup_window.rs index b44b844..06eac40 100644 --- a/crates/adapters/src/rendering/femtovg/popup_window.rs +++ b/crates/adapters/src/rendering/femtovg/popup_window.rs @@ -12,6 +12,21 @@ use slint_interpreter::ComponentInstance; use std::cell::{Cell, OnceCell, RefCell}; use std::rc::{Rc, Weak}; +/// Represents the rendering lifecycle state of a popup window +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum PopupRenderState { + /// Awaiting Wayland configure event before rendering can begin + Unconfigured, + /// Wayland is recalculating geometry; rendering is paused + Repositioning, + /// Ready to render, no pending changes + ReadyClean, + /// Ready to render, frame is dirty and needs redraw + ReadyDirty, + /// Needs an additional layout pass after the next render + NeedsRelayout, +} + pub struct PopupWindow { window: Window, renderer: FemtoVGRenderer, @@ -20,9 +35,7 @@ pub struct PopupWindow { scale_factor: Cell, popup_handle: Cell>, on_close: OnceCell, - configured: Cell, - repositioning: Cell, - needs_relayout: Cell, + popup_render_state: Cell, component_instance: RefCell>, } @@ -39,9 +52,7 @@ impl PopupWindow { scale_factor: Cell::new(1.), popup_handle: Cell::new(None), on_close: OnceCell::new(), - configured: Cell::new(false), - repositioning: Cell::new(false), - needs_relayout: Cell::new(false), + popup_render_state: Cell::new(PopupRenderState::Unconfigured), component_instance: RefCell::new(None), } }) @@ -96,11 +107,26 @@ impl PopupWindow { pub fn mark_configured(&self) { info!("Popup window marked as configured"); - self.configured.set(true); + + if matches!( + self.popup_render_state.get(), + PopupRenderState::Unconfigured + ) { + info!("Transitioning from Unconfigured to ReadyDirty state"); + self.popup_render_state.set(PopupRenderState::ReadyDirty); + } else { + info!( + "Preserving current render state to avoid overwriting: {:?}", + self.popup_render_state.get() + ); + } } pub fn is_configured(&self) -> bool { - self.configured.get() + !matches!( + self.popup_render_state.get(), + PopupRenderState::Unconfigured + ) } pub fn set_component_instance(&self, instance: ComponentInstance) { @@ -110,6 +136,9 @@ impl PopupWindow { info!("Component instance already set for popup window - replacing"); } *comp = Some(instance); + + self.window() + .dispatch_event(WindowEvent::WindowActiveChanged(true)); } pub fn request_resize(&self, width: f32, height: f32) { @@ -119,25 +148,32 @@ impl PopupWindow { } pub fn begin_repositioning(&self) { - self.repositioning.set(true); + self.popup_render_state.set(PopupRenderState::Repositioning); } pub fn end_repositioning(&self) { - self.repositioning.set(false); - self.needs_relayout.set(true); + self.popup_render_state.set(PopupRenderState::NeedsRelayout); } } impl RenderableWindow for PopupWindow { fn render_frame_if_dirty(&self) -> Result<()> { - if !self.configured.get() { - info!("Popup not yet configured, skipping render"); - return Ok(()); - } - - if self.repositioning.get() { - info!("Popup repositioning in progress, skipping render"); - return Ok(()); + match self.popup_render_state.get() { + PopupRenderState::Unconfigured => { + info!("Popup not yet configured, skipping render"); + return Ok(()); + } + PopupRenderState::Repositioning => { + info!("Popup repositioning in progress, skipping render"); + return Ok(()); + } + PopupRenderState::ReadyClean => { + // Nothing to render + return Ok(()); + } + PopupRenderState::ReadyDirty | PopupRenderState::NeedsRelayout => { + // Proceed with rendering + } } if matches!( @@ -156,10 +192,15 @@ impl RenderableWindow for PopupWindow { })?; info!("Popup frame rendered successfully"); - if self.needs_relayout.get() { + if matches!( + self.popup_render_state.get(), + PopupRenderState::NeedsRelayout + ) { info!("Popup needs relayout, requesting additional render"); - self.needs_relayout.set(false); + self.popup_render_state.set(PopupRenderState::ReadyDirty); RenderableWindow::request_redraw(self); + } else { + self.popup_render_state.set(PopupRenderState::ReadyClean); } } Ok(()) @@ -203,6 +244,9 @@ impl WindowAdapter for PopupWindow { } fn request_redraw(&self) { + if matches!(self.popup_render_state.get(), PopupRenderState::ReadyClean) { + self.popup_render_state.set(PopupRenderState::ReadyDirty); + } RenderableWindow::request_redraw(self); } } diff --git a/crates/adapters/src/wayland/surfaces/event_context.rs b/crates/adapters/src/wayland/surfaces/event_context.rs index 9549e80..845516e 100644 --- a/crates/adapters/src/wayland/surfaces/event_context.rs +++ b/crates/adapters/src/wayland/surfaces/event_context.rs @@ -142,15 +142,29 @@ impl EventContext { self.active_surface = ActiveWindow::None; } + pub const fn is_popup_active(&self) -> bool { + matches!(self.active_surface, ActiveWindow::Popup(_)) + } + pub fn dispatch_to_active_window(&self, event: WindowEvent) { match self.active_surface { ActiveWindow::Main => { self.main_window.window().dispatch_event(event); } ActiveWindow::Popup(handle) => { + let is_pointer_event = matches!( + event, + WindowEvent::PointerMoved { .. } + | WindowEvent::PointerPressed { .. } + | WindowEvent::PointerReleased { .. } + ); + if let Some(popup_manager) = &self.popup_manager { if let Some(popup_surface) = popup_manager.get_popup_window(handle.key()) { popup_surface.dispatch_event(event); + if is_pointer_event { + popup_surface.request_redraw(); + } } } } diff --git a/crates/adapters/src/wayland/surfaces/popup_manager.rs b/crates/adapters/src/wayland/surfaces/popup_manager.rs index 0c76dcc..0a6d465 100644 --- a/crates/adapters/src/wayland/surfaces/popup_manager.rs +++ b/crates/adapters/src/wayland/surfaces/popup_manager.rs @@ -109,6 +109,8 @@ struct ActivePopup { impl Drop for ActivePopup { fn drop(&mut self) { info!("ActivePopup being dropped - cleaning up resources"); + self.window.cleanup_resources(); + self.surface.destroy(); } } @@ -424,10 +426,9 @@ impl PopupManager { } fn destroy_popup(&self, id: PopupId) { - if let Some(popup) = self.state.borrow_mut().popups.remove(&id) { + if let Some(_popup) = self.state.borrow_mut().popups.remove(&id) { info!("Destroying popup with id {:?}", id); - popup.window.cleanup_resources(); - popup.surface.destroy(); + // cleanup happens automatically via ActivePopup::drop() } } diff --git a/crates/adapters/src/wayland/surfaces/surface_state.rs b/crates/adapters/src/wayland/surfaces/surface_state.rs index 45747bd..0b1251c 100644 --- a/crates/adapters/src/wayland/surfaces/surface_state.rs +++ b/crates/adapters/src/wayland/surfaces/surface_state.rs @@ -264,6 +264,10 @@ impl SurfaceState { self.event_context.borrow_mut().clear_entered_surface(); } + pub fn is_popup_active(&self) -> bool { + self.event_context.borrow().is_popup_active() + } + pub fn dispatch_to_active_window(&self, event: WindowEvent) { self.event_context.borrow().dispatch_to_active_window(event); } diff --git a/crates/composition/src/layer_shika.rs b/crates/composition/src/layer_shika.rs index 67ba778..ff34c40 100644 --- a/crates/composition/src/layer_shika.rs +++ b/crates/composition/src/layer_shika.rs @@ -451,7 +451,7 @@ impl Runtime { match command { PopupCommand::Show(request) => { - if let Err(e) = ctx.show_popup(&request, Some(control.clone())) { + if let Err(e) = ctx.show_popup(&request) { log::error!("Failed to show popup: {}", e); } } @@ -582,11 +582,6 @@ impl Runtime { &self.compilation_result } - #[must_use] - pub fn popup(&self, component_name: impl Into) -> PopupBuilder<'_> { - PopupBuilder::new(self, component_name.into()) - } - pub fn on(&self, surface_name: &str, callback_name: &str, handler: F) -> Result<()> where F: Fn(ShellControl) -> R + 'static, diff --git a/crates/composition/src/popup_builder.rs b/crates/composition/src/popup_builder.rs index 4cec8aa..19949ae 100644 --- a/crates/composition/src/popup_builder.rs +++ b/crates/composition/src/popup_builder.rs @@ -1,15 +1,26 @@ -use crate::Result; -use crate::shell::Shell; -use layer_shika_adapters::platform::slint_interpreter::Value; -use layer_shika_domain::prelude::AnchorStrategy; use layer_shika_domain::value_objects::popup_positioning_mode::PopupPositioningMode; use layer_shika_domain::value_objects::popup_request::{PopupPlacement, PopupRequest, PopupSize}; -/// Builder for configuring and displaying popup windows +/// Builder for configuring popup windows /// -/// Useful for context menus, tooltips, dropdowns, and other transient UI. -pub struct PopupBuilder<'a> { - shell: &'a Shell, +/// This is a convenience wrapper around `PopupRequest::builder()` that provides +/// a fluent API for configuring popups. Once built, pass the resulting `PopupRequest` +/// to `ShellControl::show_popup()` from within a callback. +/// +/// # Example +/// ```rust,ignore +/// shell.on("Main", "open_menu", |control| { +/// let request = PopupBuilder::new("MenuPopup") +/// .relative_to_cursor() +/// .anchor_top_left() +/// .grab(true) +/// .close_on("menu_closed") +/// .build(); +/// +/// control.show_popup(&request)?; +/// }); +/// ``` +pub struct PopupBuilder { component: String, reference: PopupPlacement, anchor: PopupPositioningMode, @@ -19,11 +30,12 @@ pub struct PopupBuilder<'a> { resize_callback: Option, } -impl<'a> PopupBuilder<'a> { - pub(crate) fn new(shell: &'a Shell, component: String) -> Self { +impl PopupBuilder { + /// Creates a new popup builder for the specified component + #[must_use] + pub fn new(component: impl Into) -> Self { Self { - shell, - component, + component: component.into(), reference: PopupPlacement::AtCursor, anchor: PopupPositioningMode::TopLeft, size: PopupSize::Content, @@ -168,181 +180,11 @@ impl<'a> PopupBuilder<'a> { self } - /// Binds the popup to show when the specified Slint callback is triggered - pub fn bind(self, trigger_callback: &str) -> Result<()> { - let request = self.build_request(); - let control = self.shell.control(); - - self.shell.with_all_surfaces(|_name, instance| { - let request_clone = request.clone(); - let control_clone = control.clone(); - - if let Err(e) = instance.set_callback(trigger_callback, move |_args| { - if let Err(e) = control_clone.show_popup(&request_clone) { - log::error!("Failed to show popup: {}", e); - } - Value::Void - }) { - log::error!( - "Failed to bind popup callback '{}': {}", - trigger_callback, - e - ); - } - }); - - Ok(()) - } - - /// Binds the popup to toggle visibility when the specified callback is triggered - pub fn toggle(self, trigger_callback: &str) -> Result<()> { - let request = self.build_request(); - let control = self.shell.control(); - let component_name = request.component.clone(); - - self.shell.with_all_surfaces(|_name, instance| { - let request_clone = request.clone(); - let control_clone = control.clone(); - let component_clone = component_name.clone(); - - if let Err(e) = instance.set_callback(trigger_callback, move |_args| { - log::debug!("Toggle callback for component: {}", component_clone); - if let Err(e) = control_clone.show_popup(&request_clone) { - log::error!("Failed to toggle popup: {}", e); - } - Value::Void - }) { - log::error!( - "Failed to bind toggle popup callback '{}': {}", - trigger_callback, - e - ); - } - }); - - Ok(()) - } - - #[allow(clippy::too_many_lines)] - pub fn bind_anchored(self, trigger_callback: &str, strategy: AnchorStrategy) -> Result<()> { - let component_name = self.component.clone(); - let grab = self.grab; - let close_callback = self.close_callback.clone(); - let resize_callback = self.resize_callback.clone(); - let control = self.shell.control(); - - self.shell.with_all_surfaces(|_name, instance| { - let component_clone = component_name.clone(); - let control_clone = control.clone(); - let close_cb = close_callback.clone(); - let resize_cb = resize_callback.clone(); - - if let Err(e) = instance.set_callback(trigger_callback, move |args| { - if args.len() < 4 { - log::error!( - "bind_anchored callback expects 4 arguments (x, y, width, height), got {}", - args.len() - ); - return Value::Void; - } - - let anchor_x = args - .first() - .and_then(|v| v.clone().try_into().ok()) - .unwrap_or(0.0); - let anchor_y = args - .get(1) - .and_then(|v| v.clone().try_into().ok()) - .unwrap_or(0.0); - let anchor_w = args - .get(2) - .and_then(|v| v.clone().try_into().ok()) - .unwrap_or(0.0); - let anchor_h = args - .get(3) - .and_then(|v| v.clone().try_into().ok()) - .unwrap_or(0.0); - - log::debug!( - "Anchored popup triggered for '{}' at rect: ({}, {}, {}, {})", - component_clone, - anchor_x, - anchor_y, - anchor_w, - anchor_h - ); - - let (reference_x, reference_y, mode) = match strategy { - AnchorStrategy::CenterBottom => { - let center_x = anchor_x + anchor_w / 2.0; - let bottom_y = anchor_y + anchor_h; - (center_x, bottom_y, PopupPositioningMode::TopCenter) - } - AnchorStrategy::CenterTop => { - let center_x = anchor_x + anchor_w / 2.0; - (center_x, anchor_y, PopupPositioningMode::BottomCenter) - } - AnchorStrategy::RightBottom => { - let right_x = anchor_x + anchor_w; - let bottom_y = anchor_y + anchor_h; - (right_x, bottom_y, PopupPositioningMode::TopRight) - } - AnchorStrategy::LeftTop => { - (anchor_x, anchor_y, PopupPositioningMode::BottomLeft) - } - AnchorStrategy::RightTop => { - let right_x = anchor_x + anchor_w; - (right_x, anchor_y, PopupPositioningMode::BottomRight) - } - AnchorStrategy::LeftBottom => { - let bottom_y = anchor_y + anchor_h; - (anchor_x, bottom_y, PopupPositioningMode::TopLeft) - } - AnchorStrategy::Cursor => (anchor_x, anchor_y, PopupPositioningMode::TopLeft), - }; - - log::debug!( - "Resolved anchored popup reference for '{}' -> ({}, {}), mode: {:?}", - component_clone, - reference_x, - reference_y, - mode - ); - - let mut builder = PopupRequest::builder(component_clone.clone()) - .placement(PopupPlacement::at_position(reference_x, reference_y)) - .size(PopupSize::Content) - .grab(grab) - .mode(mode); - - if let Some(ref close_cb) = close_cb { - builder = builder.close_on(close_cb.clone()); - } - - if let Some(ref resize_cb) = resize_cb { - builder = builder.resize_on(resize_cb.clone()); - } - - let request = builder.build(); - - if let Err(e) = control_clone.show_popup(&request) { - log::error!("Failed to show anchored popup: {}", e); - } - - Value::Void - }) { - log::error!( - "Failed to bind anchored popup callback '{}': {}", - trigger_callback, - e - ); - } - }); - - Ok(()) - } - - fn build_request(&self) -> PopupRequest { + /// Builds the popup request + /// + /// After building, pass the request to `ShellControl::show_popup()` to display the popup. + #[must_use] + pub fn build(self) -> PopupRequest { let mut builder = PopupRequest::builder(self.component.clone()) .placement(self.reference) .size(self.size) diff --git a/crates/composition/src/shell.rs b/crates/composition/src/shell.rs index 194c6b0..f001762 100644 --- a/crates/composition/src/shell.rs +++ b/crates/composition/src/shell.rs @@ -1,6 +1,5 @@ use crate::event_loop::{EventLoopHandle, FromAppState}; use crate::layer_surface::LayerSurfaceHandle; -use crate::popup_builder::PopupBuilder; use crate::shell_config::{CompiledUiSource, ShellConfig}; use crate::shell_runtime::ShellRuntime; use crate::surface_registry::{SurfaceDefinition, SurfaceEntry, SurfaceRegistry}; @@ -580,11 +579,11 @@ impl Shell { fn handle_popup_command( command: PopupCommand, ctx: &mut EventDispatchContext<'_>, - control: &ShellControl, + _control: &ShellControl, ) { match command { PopupCommand::Show(request) => { - if let Err(e) = ctx.show_popup(&request, Some(control.clone())) { + if let Err(e) = ctx.show_popup(&request) { log::error!("Failed to show popup: {}", e); } } @@ -963,12 +962,6 @@ impl Shell { &self.compilation_result } - /// Creates a popup builder for showing a popup window - #[must_use] - pub fn popup(&self, component_name: impl Into) -> PopupBuilder<'_> { - PopupBuilder::new(self, component_name.into()) - } - /// Returns the registry of all connected outputs pub fn output_registry(&self) -> OutputRegistry { let system = self.inner.borrow(); diff --git a/crates/composition/src/system.rs b/crates/composition/src/system.rs index d4217b9..e9990d5 100644 --- a/crates/composition/src/system.rs +++ b/crates/composition/src/system.rs @@ -156,6 +156,59 @@ impl CallbackContext { self.control .surface_by_name_and_output(&self.surface_name, self.output_handle()) } + + /// Shows a popup from a popup request + /// + /// Convenience method that forwards to the underlying `ShellControl`. + /// See [`ShellControl::show_popup`] for full documentation. + pub fn show_popup(&self, request: &PopupRequest) -> Result<()> { + self.control.show_popup(request) + } + + /// Shows a popup at the current cursor position + /// + /// Convenience method that forwards to the underlying `ShellControl`. + /// See [`ShellControl::show_popup_at_cursor`] for full documentation. + pub fn show_popup_at_cursor(&self, component: impl Into) -> Result<()> { + self.control.show_popup_at_cursor(component) + } + + /// Shows a popup centered on screen + /// + /// Convenience method that forwards to the underlying `ShellControl`. + /// See [`ShellControl::show_popup_centered`] for full documentation. + pub fn show_popup_centered(&self, component: impl Into) -> Result<()> { + self.control.show_popup_centered(component) + } + + /// Shows a popup at the specified absolute position + /// + /// Convenience method that forwards to the underlying `ShellControl`. + /// See [`ShellControl::show_popup_at_position`] for full documentation. + pub fn show_popup_at_position( + &self, + component: impl Into, + x: f32, + y: f32, + ) -> Result<()> { + self.control.show_popup_at_position(component, x, y) + } + + /// Closes a specific popup by its handle + /// + /// Convenience method that forwards to the underlying `ShellControl`. + /// See [`ShellControl::close_popup`] for full documentation. + pub fn close_popup(&self, handle: PopupHandle) -> Result<()> { + self.control.close_popup(handle) + } + + /// Resizes a popup to the specified dimensions + /// + /// Convenience method that forwards to the underlying `ShellControl`. + /// See [`ShellControl::resize_popup`] for full documentation. + pub fn resize_popup(&self, handle: PopupHandle, width: f32, height: f32) -> Result<()> { + self.control.resize_popup(handle, width, height) + } } /// Handle for runtime control of shell operations @@ -172,6 +225,41 @@ impl ShellControl { } /// Shows a popup from a popup request + /// + /// This is the primary API for showing popups from Slint callbacks. Popups are + /// transient windows that appear above the main surface, commonly used for menus, + /// tooltips, dropdowns, and other temporary UI elements. + /// + /// # Content-Based Sizing + /// + /// When using `PopupSize::Content`, you must configure a resize callback via + /// `resize_on()` to enable automatic resizing. The popup component should use a + /// `Timer` with `interval: 1ms` to invoke the resize callback after initialization, + /// ensuring the component is initialized before callback invocation. This allows the + /// popup to reposition itself to fit the content. See the `popup-demo` example. + /// + /// # Example + /// + /// ```rust,ignore + /// shell.on("Main", "open_menu", |control| { + /// let request = PopupRequest::builder("MenuPopup") + /// .placement(PopupPlacement::at_cursor()) + /// .grab(true) + /// .close_on("menu_closed") + /// .build(); + /// + /// control.show_popup(&request)?; + /// Value::Void + /// }); + /// ``` + /// + /// # See Also + /// + /// - [`show_popup_at_cursor`](Self::show_popup_at_cursor) - Convenience method for cursor-positioned popups + /// - [`show_popup_centered`](Self::show_popup_centered) - Convenience method for centered popups + /// - [`show_popup_at_position`](Self::show_popup_at_position) - Convenience method for absolute positioning + /// - [`PopupRequest`] - Full popup configuration options + /// - [`PopupBuilder`] - Fluent API for building popup requests pub fn show_popup(&self, request: &PopupRequest) -> Result<()> { self.sender .send(ShellCommand::Popup(PopupCommand::Show(request.clone()))) @@ -183,6 +271,19 @@ impl ShellControl { } /// Shows a popup at the current cursor position + /// + /// Convenience method for showing a popup at the cursor with default settings. + /// For more control over popup positioning, sizing, and behavior, use + /// [`show_popup`](Self::show_popup) with a [`PopupRequest`]. + /// + /// # Example + /// + /// ```rust,ignore + /// shell.on("Main", "context_menu", |control| { + /// control.show_popup_at_cursor("ContextMenu")?; + /// Value::Void + /// }); + /// ``` pub fn show_popup_at_cursor(&self, component: impl Into) -> Result<()> { let request = PopupRequest::builder(component.into()) .placement(PopupPlacement::AtCursor) @@ -191,6 +292,18 @@ impl ShellControl { } /// Shows a popup centered on screen + /// + /// Convenience method for showing a centered popup. Useful for dialogs + /// and modal content that should appear in the middle of the screen. + /// + /// # Example + /// + /// ```rust,ignore + /// shell.on("Main", "show_dialog", |control| { + /// control.show_popup_centered("ConfirmDialog")?; + /// Value::Void + /// }); + /// ``` pub fn show_popup_centered(&self, component: impl Into) -> Result<()> { let request = PopupRequest::builder(component.into()) .placement(PopupPlacement::AtCursor) @@ -199,7 +312,19 @@ impl ShellControl { self.show_popup(&request) } - /// Shows a popup at the specified position + /// Shows a popup at the specified absolute position + /// + /// Convenience method for showing a popup at an exact screen coordinate. + /// The position is in logical pixels relative to the surface origin. + /// + /// # Example + /// + /// ```rust,ignore + /// shell.on("Main", "show_tooltip", |control| { + /// control.show_popup_at_position("Tooltip", 100.0, 50.0)?; + /// Value::Void + /// }); + /// ``` pub fn show_popup_at_position( &self, component: impl Into, @@ -212,7 +337,23 @@ impl ShellControl { self.show_popup(&request) } - /// Closes a popup by its handle + /// Closes a specific popup by its handle + /// + /// Use this when you need to close a specific popup that you opened previously. + /// The handle is returned by [`show_popup`](Self::show_popup) and related methods. + /// + /// For closing popups from within the popup itself, consider using the + /// `close_on` callback configuration in [`PopupRequest`] instead. + /// + /// # Example + /// + /// ```rust,ignore + /// // Store handle when showing popup + /// let handle = context.show_popup(&request)?; + /// + /// // Later, close it + /// control.close_popup(handle)?; + /// ``` pub fn close_popup(&self, handle: PopupHandle) -> Result<()> { self.sender .send(ShellCommand::Popup(PopupCommand::Close(handle))) @@ -224,6 +365,22 @@ impl ShellControl { } /// Resizes a popup to the specified dimensions + /// + /// Dynamically changes the size of an active popup. This is typically used + /// in response to content changes or user interaction. + /// + /// For automatic content-based sizing, use `PopupSize::Content` with the + /// `resize_on` callback configuration in [`PopupRequest`] instead. + /// + /// # Example + /// + /// ```rust,ignore + /// shell.on("Main", "expand_menu", |control| { + /// // Assuming we have the popup handle stored somewhere + /// control.resize_popup(menu_handle, 400.0, 600.0)?; + /// Value::Void + /// }); + /// ``` pub fn resize_popup(&self, handle: PopupHandle, width: f32, height: f32) -> Result<()> { self.sender .send(ShellCommand::Popup(PopupCommand::Resize { @@ -722,11 +879,11 @@ impl EventDispatchContext<'_> { } /// Shows a popup from a popup request - pub fn show_popup( - &mut self, - req: &PopupRequest, - resize_control: Option, - ) -> Result { + /// + /// Resize callbacks (if configured via `resize_on()`) will operate directly + /// on the popup manager for immediate updates. + #[allow(clippy::too_many_lines, clippy::cognitive_complexity)] + pub fn show_popup(&mut self, req: &PopupRequest) -> Result { log::info!("show_popup called for component '{}'", req.component); let compilation_result = self.compilation_result().ok_or_else(|| { @@ -779,32 +936,67 @@ impl EventDispatchContext<'_> { }) })?; + // For content-based sizing, we need to query the component's preferred size first let initial_dimensions = match req.size { PopupSize::Fixed { w, h } => { log::debug!("Using fixed popup size: {}x{}", w, h); (w, h) } PopupSize::Content => { - log::debug!("Using content-based sizing - will measure after instance creation"); + log::debug!("Using content-based sizing - starting at 2×2"); + // Start with minimal size. Consumer app should register a callback to + // call resize_popup() with the desired dimensions. (2.0, 2.0) } }; + let resolved_placement = match req.placement { + PopupPlacement::AtCursor => { + let cursor_pos = active_surface.current_pointer_position(); + log::debug!( + "Resolving AtCursor placement to actual cursor position: ({}, {})", + cursor_pos.x, + cursor_pos.y + ); + PopupPlacement::AtPosition { + x: cursor_pos.x, + y: cursor_pos.y, + } + } + other => other, + }; + + let (ref_x, ref_y) = resolved_placement.position(); + log::debug!( "Creating popup for '{}' with dimensions {}x{} at position ({}, {}), mode: {:?}", req.component, initial_dimensions.0, initial_dimensions.1, - req.placement.position().0, - req.placement.position().1, + ref_x, + ref_y, req.mode ); - let popup_handle = - popup_manager.request_popup(req.clone(), initial_dimensions.0, initial_dimensions.1); + // Create a new request with resolved placement + let resolved_request = PopupRequest { + component: req.component.clone(), + placement: resolved_placement, + size: req.size, + mode: req.mode, + grab: req.grab, + close_callback: req.close_callback.clone(), + resize_callback: req.resize_callback.clone(), + }; + + let popup_handle = popup_manager.request_popup( + resolved_request, + initial_dimensions.0, + initial_dimensions.1, + ); let (instance, popup_key_cell) = - Self::create_popup_instance(&definition, &popup_manager, resize_control, req)?; + Self::create_popup_instance(&definition, &popup_manager, req)?; popup_key_cell.set(popup_handle.key()); @@ -894,7 +1086,6 @@ impl EventDispatchContext<'_> { fn create_popup_instance( definition: &ComponentDefinition, popup_manager: &Rc, - resize_control: Option, req: &PopupRequest, ) -> Result<(ComponentInstance, Rc>)> { let instance = definition.create().map_err(|e| { @@ -905,13 +1096,7 @@ impl EventDispatchContext<'_> { let popup_key_cell = Rc::new(Cell::new(0)); - Self::register_popup_callbacks( - &instance, - popup_manager, - resize_control, - &popup_key_cell, - req, - )?; + Self::register_popup_callbacks(&instance, popup_manager, &popup_key_cell, req)?; instance.show().map_err(|e| { Error::Domain(DomainError::Configuration { @@ -925,7 +1110,6 @@ impl EventDispatchContext<'_> { fn register_popup_callbacks( instance: &ComponentInstance, popup_manager: &Rc, - resize_control: Option, popup_key_cell: &Rc>, req: &PopupRequest, ) -> Result<()> { @@ -934,10 +1118,9 @@ impl EventDispatchContext<'_> { } if let Some(resize_callback_name) = &req.resize_callback { - Self::register_resize_callback( + Self::register_resize_direct( instance, popup_manager, - resize_control, popup_key_cell, resize_callback_name, )?; @@ -966,59 +1149,6 @@ impl EventDispatchContext<'_> { }) } - fn register_resize_callback( - instance: &ComponentInstance, - popup_manager: &Rc, - resize_control: Option, - popup_key_cell: &Rc>, - callback_name: &str, - ) -> Result<()> { - if let Some(control) = resize_control { - Self::register_resize_with_control(instance, popup_key_cell, &control, callback_name) - } else { - Self::register_resize_direct(instance, popup_manager, popup_key_cell, callback_name) - } - } - - fn register_resize_with_control( - instance: &ComponentInstance, - popup_key_cell: &Rc>, - control: &ShellControl, - callback_name: &str, - ) -> Result<()> { - let key_cell = Rc::clone(popup_key_cell); - let control = control.clone(); - instance - .set_callback(callback_name, move |args| { - let dimensions = extract_dimensions_from_callback(args); - let popup_key = key_cell.get(); - - log::info!( - "Resize callback invoked: {}x{} for key {}", - dimensions.width, - dimensions.height, - popup_key - ); - - if control - .resize_popup( - PopupHandle::from_raw(popup_key), - dimensions.width, - dimensions.height, - ) - .is_err() - { - log::error!("Failed to resize popup through control"); - } - Value::Void - }) - .map_err(|e| { - Error::Domain(DomainError::Configuration { - message: format!("Failed to set '{}' callback: {}", callback_name, e), - }) - }) - } - fn register_resize_direct( instance: &ComponentInstance, popup_manager: &Rc, diff --git a/src/lib.rs b/src/lib.rs index 4ab0c4a..0cf5b2a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -143,7 +143,7 @@ pub use shell::{ }; pub use window::{ - AnchorEdges, AnchorStrategy, KeyboardInteractivity, Layer, PopupHandle, PopupPlacement, + AnchorEdges, AnchorStrategy, KeyboardInteractivity, Layer, PopupBuilder, PopupHandle, PopupPlacement, PopupPositioningMode, PopupRequest, PopupSize, }; diff --git a/src/window.rs b/src/window.rs index a57fb33..3e6472e 100644 --- a/src/window.rs +++ b/src/window.rs @@ -1,4 +1,4 @@ pub use layer_shika_composition::{ - AnchorEdges, AnchorStrategy, KeyboardInteractivity, Layer, PopupHandle, PopupPlacement, - PopupPositioningMode, PopupRequest, PopupSize, + AnchorEdges, AnchorStrategy, KeyboardInteractivity, Layer, PopupBuilder, PopupHandle, + PopupPlacement, PopupPositioningMode, PopupRequest, PopupSize, };