Associate PopupWindows with an ID for their active popup (#6693)

Popups are stored in a HashMap and are assigned an ID so popup.close(); closes the correct popup and so a single PopupWindow cannot be opened multiple times

Author: BrandonXLF

api/cpp/include/slint_window.h

@@ -106,18 +106,23 @@ class WindowAdapterRc
     }
 
     template<typename Component, typename Parent, typename PosGetter>
-    void show_popup(const Parent *parent_component, PosGetter pos,
-                    cbindgen_private::PopupClosePolicy close_policy,
-                    cbindgen_private::ItemRc parent_item) const
+    uint32_t show_popup(const Parent *parent_component, PosGetter pos,
+                        cbindgen_private::PopupClosePolicy close_policy,
+                        cbindgen_private::ItemRc parent_item) const
     {
         auto popup = Component::create(parent_component);
         cbindgen_private::Point p = pos(popup);
         auto popup_dyn = popup.into_dyn();
-        cbindgen_private::slint_windowrc_show_popup(&inner, &popup_dyn, p, close_policy,
-                                                    &parent_item);
+        return cbindgen_private::slint_windowrc_show_popup(&inner, &popup_dyn, p, close_policy,
+                                                           &parent_item);
     }
 
-    void close_popup() const { cbindgen_private::slint_windowrc_close_popup(&inner); }
+    void close_popup(uint32_t popup_id) const
+    {
+        if (popup_id > 0) {
+            cbindgen_private::slint_windowrc_close_popup(&inner, popup_id);
+        }
+    }
 
     template<std::invocable<RenderingState, GraphicsAPI> F>
     std::optional<SetRenderingNotifierError> set_rendering_notifier(F callback) const

internal/backends/qt/qt_window.rs

@@ -165,7 +165,7 @@ cpp! {{
                 void *parent_window = p->rust_window;
                 bool inside = rect().contains(event->pos());
                 bool close_on_click = rust!(Slint_mouseReleaseEventPopup [parent_window: &QtWindow as "void*", inside: bool as "bool"] -> bool as "bool" {
-                    let close_policy = parent_window.close_policy();
+                    let close_policy = parent_window.top_close_policy();
                     close_policy == PopupClosePolicy::CloseOnClick || (close_policy == PopupClosePolicy::CloseOnClickOutside && !inside)
                 });
                 if (close_on_click) {
@@ -182,7 +182,7 @@ cpp! {{
             });
             if (parent_of_popup_to_close) {
                 rust!(Slint_mouseReleaseEventClosePopup [parent_of_popup_to_close: &QtWindow as "void*"] {
-                    parent_of_popup_to_close.close_popup();
+                    parent_of_popup_to_close.close_top_popup();
                 });
             }
         }
@@ -1746,12 +1746,12 @@ impl QtWindow {
         timer_event();
     }
 
-    fn close_popup(&self) {
-        WindowInner::from_pub(&self.window).close_popup();
+    fn close_top_popup(&self) {
+        WindowInner::from_pub(&self.window).close_top_popup();
     }
 
-    fn close_policy(&self) -> PopupClosePolicy {
-        WindowInner::from_pub(&self.window).close_policy()
+    fn top_close_policy(&self) -> PopupClosePolicy {
+        WindowInner::from_pub(&self.window).top_close_policy()
     }
 
     fn window_state_event(&self) {

internal/compiler/generator/cpp.rs

@@ -1969,6 +1969,17 @@ fn generate_sub_component(
         ));
     }
 
+    for (i, _) in component.popup_windows.iter().enumerate() {
+        target_struct.members.push((
+            field_access,
+            Declaration::Var(Var {
+                ty: ident("mutable uint32_t"),
+                name: format_smolstr!("popup_id_{}", i),
+                ..Default::default()
+            }),
+        ));
+    }
+
     for (prop1, prop2) in &component.two_way_bindings {
         init.push(format!(
             "slint::private_api::Property<{ty}>::link_two_way(&{p1}, &{p2});",
@@ -3613,15 +3624,28 @@ fn compile_builtin_function_call(
                 let position = compile_expression(&popup.position.borrow(), &popup_ctx);
                 let close_policy = compile_expression(close_policy, ctx);
                 format!(
-                    "{window}.show_popup<{popup_window_id}>({component_access}, [=](auto self) {{ return {position}; }}, {close_policy}, {{ {parent_component} }})"
+                    "{window}.close_popup({component_access}->popup_id_{popup_index}); {component_access}->popup_id_{popup_index} = {window}.show_popup<{popup_window_id}>({component_access}, [=](auto self) {{ return {position}; }}, {close_policy}, {{ {parent_component} }})"
                 )
             } else {
                 panic!("internal error: invalid args to ShowPopupWindow {:?}", arguments)
             }
         }
         BuiltinFunction::ClosePopupWindow => {
-            let window = access_window_field(ctx);
-            format!("{window}.close_popup()")
+            if let [llr::Expression::NumberLiteral(popup_index), llr::Expression::PropertyReference(parent_ref)] = arguments {
+                let mut parent_ctx = ctx;
+                let mut component_access = "self".into();
+
+                if let llr::PropertyReference::InParent { level, .. } = parent_ref {
+                    for _ in 0..level.get() {
+                        component_access = format!("{}->parent", component_access);
+                        parent_ctx = parent_ctx.parent.as_ref().unwrap().ctx;
+                    }
+                };
+                let window = access_window_field(ctx);
+                format!("{window}.close_popup({component_access}->popup_id_{popup_index})")
+            } else {
+                panic!("internal error: invalid args to ClosePopupWindow {:?}", arguments)
+            }
         }
         BuiltinFunction::SetSelectionOffsets => {
             if let [llr::Expression::PropertyReference(pr), from, to] = arguments {

internal/compiler/generator/rust.rs

@@ -1001,6 +1001,9 @@ fn generate_sub_component(
         sub_component_types.push(sub_component_id);
     }
 
+    let popup_id_names =
+        component.popup_windows.iter().enumerate().map(|(i, _)| internal_popup_id(i));
+
     for (prop1, prop2) in &component.two_way_bindings {
         let p1 = access_member(prop1, &ctx);
         let p2 = access_member(prop2, &ctx);
@@ -1116,6 +1119,7 @@ fn generate_sub_component(
         struct #inner_component_id {
             #(#item_names : sp::#item_types,)*
             #(#sub_component_names : #sub_component_types,)*
+            #(#popup_id_names : ::core::cell::Cell<sp::Option<::core::num::NonZeroU32>>,)*
             #(#declared_property_vars : sp::Property<#declared_property_types>,)*
             #(#declared_callbacks : sp::Callback<(#(#declared_callbacks_types,)*), #declared_callbacks_ret>,)*
             #(#repeated_element_names : sp::Repeater<#repeated_element_components>,)*
@@ -1819,6 +1823,12 @@ fn inner_component_id(component: &llr::SubComponent) -> proc_macro2::Ident {
     format_ident!("Inner{}", ident(&component.name))
 }
 
+fn internal_popup_id(index: usize) -> proc_macro2::Ident {
+    let mut name = index.to_string();
+    name.insert_str(0, "popup_id_");
+    ident(&name)
+}
+
 fn global_inner_name(g: &llr::GlobalComponent) -> TokenStream {
     if g.is_builtin {
         let i = ident(&g.name);
@@ -2660,27 +2670,51 @@ fn compile_builtin_function_call(
 
                 let close_policy = compile_expression(close_policy, ctx);
                 let window_adapter_tokens = access_window_adapter_field(ctx);
+                let popup_id_name = internal_popup_id(*popup_index as usize);
                 quote!({
                     let popup_instance = #popup_window_id::new(#component_access_tokens.self_weak.get().unwrap().clone()).unwrap();
                     let popup_instance_vrc = sp::VRc::map(popup_instance.clone(), |x| x);
                     #popup_window_id::user_init(popup_instance_vrc.clone());
                     let position = { let _self = popup_instance_vrc.as_pin_ref(); #position };
-                    sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_popup(
-                        &sp::VRc::into_dyn(popup_instance.into()),
-                        position,
-                        #close_policy,
-                        #parent_component
-                    )
+                    if let Some(current_id) = #component_access_tokens.#popup_id_name.take() {
+                        sp::WindowInner::from_pub(#window_adapter_tokens.window()).close_popup(current_id);
+                    }
+                    #component_access_tokens.#popup_id_name.set(Some(
+                        sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_popup(
+                            &sp::VRc::into_dyn(popup_instance.into()),
+                            position,
+                            #close_policy,
+                            #parent_component
+                        ))
+                    );
                 })
             } else {
                 panic!("internal error: invalid args to ShowPopupWindow {:?}", arguments)
             }
         }
         BuiltinFunction::ClosePopupWindow => {
-            let window_adapter_tokens = access_window_adapter_field(ctx);
-            quote!(
-                sp::WindowInner::from_pub(#window_adapter_tokens.window()).close_popup()
-            )
+            if let [Expression::NumberLiteral(popup_index), Expression::PropertyReference(parent_ref)] =
+                arguments
+            {
+                let mut parent_ctx = ctx;
+                let mut component_access_tokens = quote!(_self);
+                if let llr::PropertyReference::InParent { level, .. } = parent_ref {
+                    for _ in 0..level.get() {
+                        component_access_tokens =
+                            quote!(#component_access_tokens.parent.upgrade().unwrap().as_pin_ref());
+                        parent_ctx = parent_ctx.parent.as_ref().unwrap().ctx;
+                    }
+                }
+                let window_adapter_tokens = access_window_adapter_field(ctx);
+                let popup_id_name = internal_popup_id(*popup_index as usize);
+                quote!(
+                    if let Some(current_id) = #component_access_tokens.#popup_id_name.take() {
+                        sp::WindowInner::from_pub(#window_adapter_tokens.window()).close_popup(current_id);
+                    }
+                )
+            } else {
+                panic!("internal error: invalid args to ClosePopupWindow {:?}", arguments)
+            }
         }
         BuiltinFunction::SetSelectionOffsets => {
             if let [llr::Expression::PropertyReference(pr), from, to] = arguments {

internal/compiler/llr/lower_expression.rs

@@ -122,11 +122,7 @@ pub fn lower_expression(
                 lower_show_popup(arguments, ctx)
             }
             tree_Expression::BuiltinFunctionReference(BuiltinFunction::ClosePopupWindow, _) => {
-                // FIXME: right now, `popup.close()` will close any visible popup, as the popup argument is ignored
-                llr_Expression::BuiltinFunctionCall {
-                    function: BuiltinFunction::ClosePopupWindow,
-                    arguments: vec![],
-                }
+                lower_close_popup(arguments, ctx)
             }
             tree_Expression::BuiltinFunctionReference(f, _) => {
                 let mut arguments =
@@ -399,6 +395,38 @@ fn lower_show_popup(args: &[tree_Expression], ctx: &ExpressionContext) -> llr_Ex
     }
 }
 
+fn lower_close_popup(args: &[tree_Expression], ctx: &ExpressionContext) -> llr_Expression {
+    if let [tree_Expression::ElementReference(e)] = args {
+        let popup_window = e.upgrade().unwrap();
+        let pop_comp = popup_window.borrow().enclosing_component.upgrade().unwrap();
+        let parent_component = pop_comp
+            .parent_element
+            .upgrade()
+            .unwrap()
+            .borrow()
+            .enclosing_component
+            .upgrade()
+            .unwrap();
+        let popup_list = parent_component.popup_windows.borrow();
+        let (popup_index, popup) = popup_list
+            .iter()
+            .enumerate()
+            .find(|(_, p)| Rc::ptr_eq(&p.component, &pop_comp))
+            .unwrap();
+        let item_ref = lower_expression(
+            &tree_Expression::ElementReference(Rc::downgrade(&popup.parent_element)),
+            ctx,
+        );
+
+        llr_Expression::BuiltinFunctionCall {
+            function: BuiltinFunction::ClosePopupWindow,
+            arguments: vec![llr_Expression::NumberLiteral(popup_index as _), item_ref],
+        }
+    } else {
+        panic!("invalid arguments to ShowPopupWindow");
+    }
+}
+
 pub fn lower_animation(a: &PropertyAnimation, ctx: &ExpressionContext<'_>) -> Animation {
     fn lower_animation_element(a: &ElementRc, ctx: &ExpressionContext<'_>) -> llr_Expression {
         llr_Expression::Struct {