Skip to content

NOT FUNCTIONAL: muda context menu support #8803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

NOT FUNCTIONAL: muda context menu support #8803

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
29e6363
NOT FUNCTIONAL: `muda` context menu support
npwoods Jun 28, 2025
92a37d6
Changes as per feedback
npwoods Jun 29, 2025
d095406
The muda dropdown is now (sort of) functional, with a few problems
npwoods Jul 2, 2025
59f1e73
Fixes to Rust codegen for using native context menu
npwoods Jul 3, 2025
7a9507f
Fixing issue that caused root items to be submenus
npwoods Jul 3, 2025
7a6a588
Various improvements:
npwoods Jul 3, 2025
084d2e4
- Changed `muda` infrastructure to use `VRc<MenuVTable>` instead of `…
npwoods Jul 4, 2025
41f9bf6
[autofix.ci] apply automated fixes
autofix-ci[bot] Jul 4, 2025
12cf3d0
Proper implementation of `VTableMetaDropInPlace` for `MenuVTable`
npwoods Jul 4, 2025
11300e3
Fixing menu bar code
npwoods Jul 4, 2025
086c867
The `internal/interpreter` crate now compiles
npwoods Jul 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions internal/backends/winit/winitwindowadapter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1324,12 +1324,12 @@ impl WindowAdapterInternal for WinitWindowAdapter {
}

#[cfg(muda)]
fn show_context_menu(
fn show_native_popup_menu(
&self,
context_menu: vtable::VBox<i_slint_core::menus::MenuVTable>,
context_menu_item: vtable::VBox<i_slint_core::menus::MenuVTable>,
position: LogicalPosition,
) {
self.context_menu.replace(Some(context_menu));
) -> bool {
self.context_menu.replace(Some(context_menu_item));

if let WinitWindowOrNone::HasWindow { context_menu_muda_adapter, .. } =
&*self.winit_window_or_none.borrow()
Expand All @@ -1343,6 +1343,7 @@ impl WindowAdapterInternal for WinitWindowAdapter {
self.event_loop_proxy.clone(),
)));
}
true
}

#[cfg(enable_accesskit)]
Expand Down
55 changes: 41 additions & 14 deletions internal/compiler/generator/rust.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2764,8 +2764,7 @@ fn compile_builtin_function_call(
}
}
BuiltinFunction::ShowPopupMenu => {
let [Expression::PropertyReference(context_menu_ref), entries, position, Expression::BoolLiteral(no_native)] =
arguments
let [Expression::PropertyReference(context_menu_ref), entries, position] = arguments
else {
panic!("internal error: invalid args to ShowPopupMenu {arguments:?}")
};
Expand Down Expand Up @@ -2836,6 +2835,8 @@ fn compile_builtin_function_call(
matches!(entries.ty(ctx), Type::Array(ty) if matches!(&*ty, Type::Struct{..}))
);
let entries = compile_expression(entries, ctx);
let inner_component_id =
self::inner_component_id(ctx.current_sub_component().unwrap());
let forward_callback = |access, cb| {
let call = context_menu
.clone()
Expand All @@ -2850,9 +2851,44 @@ fn compile_builtin_function_call(
});
)
};
let fw_sub_menu = forward_callback(access_sub_menu, quote!(sub_menu));
let fw_sub_menu = forward_callback(access_sub_menu.clone(), quote!(sub_menu));
let fw_activated = forward_callback(access_activated, quote!(activated));
quote! {
let context_menu_item_tree = if sp::WindowInner::from_pub(#window_adapter_tokens.window()).supports_native_menu_bar() {
// May seem overkill to have an instance of the struct for each call, but there should only be one call per component anyway
struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);
struct ContextMenuWrapper(sp::VWeak<sp::ItemTreeVTable, #popup_id>);

const _ : () = {
use slint::private_unstable_api::re_exports::*;
MenuVTable_static!(static VT for ContextMenuWrapper);
};
impl sp::Menu for ContextMenuWrapper {
fn sub_menu(&self, parent: sp::Option<&sp::MenuEntry>, result: &mut sp::SharedVector<sp::MenuEntry>) {
let Some(self_rc) = self.0.upgrade() else { return };
let _self = self_rc.as_pin_ref();
let model = match parent {
None => #entries,
Some(parent) => {
// PROBLEM - how do I wrie this up to the submenu?
// #access_sub_menu.call(&(parent.clone(),));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that line should work. Do you get a comilation error there? Same for activate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When uncommenting that on sub_menu() I get:

error[E0308]: mismatched types
     |
3946 | ...                   ) . apply_pin (_self) . call (& (parent . clone () ,)) }
     |                           ---------  ^^^^^ expected `Pin<&InnerPopupMenuImpl_root_26>`, found `Pin<&InnerMainWindow>`
     |                           |
     |                           arguments to this method are incorrect
     |
     = note: expected struct `Pin<&InnerPopupMenuImpl_root_26>`
                found struct `Pin<&InnerMainWindow>`
note: method defined here
    --> C:\Users\Nate\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\field-offset-0.3.6\src\lib.rs:264:12
     |
264  |     pub fn apply_pin<'a>(self, x: Pin<&'a T>) -> Pin<&'a U> {
     |            ^^^^^^^^^

For more information about this error, try `rustc --explain E0308`.

I get a virtually identical error when uncommenting that on activate():

error[E0308]: mismatched types
     |
3959 | ...                   ) . apply_pin (_self) . call (& (entry . clone () ,)) }
     |                           ---------  ^^^^^ expected `Pin<&InnerPopupMenuImpl_root_26>`, found `Pin<&InnerMainWindow>`
     |                           |
     |                           arguments to this method are incorrect
     |
     = note: expected struct `Pin<&InnerPopupMenuImpl_root_26>`
                found struct `Pin<&InnerMainWindow>`
note: method defined here
    --> C:\Users\Nate\.cargo\registry\src\index.crates.io-1949cf8c6b5b557f\field-offset-0.3.6\src\lib.rs:264:12
     |
264  |     pub fn apply_pin<'a>(self, x: Pin<&'a T>) -> Pin<&'a U> {
     |            ^^^^^^^^^

For more information about this error, try `rustc --explain E0308`.

I assume there is a straight forward way to walk the object hierarchy to get what we want?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the problem, access_sub_menu / access_activated are in the context of the menu.
So the change i suggest might help.

todo!()
}
};
*result = model.iter().map(|v| v.try_into().unwrap()).collect();
}
fn activate(&self, entry: &sp::MenuEntry) {
let Some(self_rc) = self.0.upgrade() else { return };
let _self = self_rc.as_pin_ref();
// PROBLEM - how do I wire this up to the activated callback?
// #access_activated.call(&(entry.clone(),))
todo!()
}
}
Some(sp::VBox::new(ContextMenuWrapper(_self.self_weak.get().unwrap().clone())))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some(sp::VBox::new(ContextMenuWrapper(_self.self_weak.get().unwrap().clone())))
Some(sp::VBox::new(ContextMenuWrapper(sp::VRc::downgrade(&popup_instance))))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to do something like that; it wasn't clear what I need to change sp::ItemTreeVTable to in this context:

struct ContextMenuWrapper(sp::VWeakMapped<sp::ItemTreeVTable, #inner_component_id>);

}
else {
None
};

let entries = #entries;
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we only need this part when not using the native context menu (fallback)

let _self = popup_instance_vrc.as_pin_ref();
Expand All @@ -2870,24 +2906,15 @@ fn compile_builtin_function_call(
};
let context_menu = context_menu.unwrap();

let native_impl = if *no_native {
quote!()
} else {
quote!(if sp::WindowInner::from_pub(#window_adapter_tokens.window()).supports_native_menu_bar() {
sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_context_menu(sp::VBox::new(popup_instance_menu), #position);
} else)
};

quote!({
let position = #position;
let popup_instance = #popup_id::new(_self.globals.get().unwrap().clone()).unwrap();
let popup_instance_vrc = sp::VRc::map(popup_instance.clone(), |x| x);
{
let parent_weak = _self.self_weak.get().unwrap().clone();
#init_popup
let popup_instance_menu = sp::ContextMenuFromItemTree::new(sp::VRc::into_dyn(popup_instance.clone()), entries.clone());
#native_impl
/* else */ {

if !sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_native_popup_menu(context_menu_item_tree.unwrap(), #position) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the .unwrap() here seems bad, as it would panic when supports_native_menu_bar returns false.

#close_popup
let id = sp::WindowInner::from_pub(#window_adapter_tokens.window()).show_popup(
&sp::VRc::into_dyn(popup_instance.into()),
Expand Down
10 changes: 3 additions & 7 deletions internal/compiler/passes/lower_menus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,14 @@ pub async fn lower_menus(

let mut has_menu = false;
let mut has_menubar = false;
let no_native_menu = type_loader.compiler_config.no_native_menu;

doc.visit_all_used_components(|component| {
recurse_elem_including_sub_components_no_borrow(component, &(), &mut |elem, _| {
if matches!(&elem.borrow().builtin_type(), Some(b) if b.name == "Window") {
has_menubar |= process_window(elem, &useful_menu_component, type_loader.compiler_config.no_native_menu, diag);
}
if matches!(&elem.borrow().builtin_type(), Some(b) if matches!(b.name.as_str(), "ContextMenuArea" | "ContextMenuInternal")) {
has_menu |= process_context_menu(elem, &useful_menu_component, diag, no_native_menu);
has_menu |= process_context_menu(elem, &useful_menu_component, diag);
}
})
});
Expand All @@ -170,8 +169,7 @@ pub async fn lower_menus(
recurse_elem_including_sub_components_no_borrow(&menubar_impl, &(), &mut |elem, _| {
if matches!(&elem.borrow().builtin_type(), Some(b) if matches!(b.name.as_str(), "ContextMenuArea" | "ContextMenuInternal"))
{
has_menu |=
process_context_menu(elem, &useful_menu_component, diag, no_native_menu);
has_menu |= process_context_menu(elem, &useful_menu_component, diag);
}
});
}
Expand Down Expand Up @@ -199,7 +197,7 @@ pub async fn lower_menus(
recurse_elem_including_sub_components_no_borrow(&popup_menu_impl, &(), &mut |elem, _| {
if matches!(&elem.borrow().builtin_type(), Some(b) if matches!(b.name.as_str(), "ContextMenuArea" | "ContextMenuInternal"))
{
process_context_menu(elem, &useful_menu_component, diag, no_native_menu);
process_context_menu(elem, &useful_menu_component, diag);
}
});
doc.popup_menu_impl = popup_menu_impl.into();
Expand All @@ -210,7 +208,6 @@ fn process_context_menu(
context_menu_elem: &ElementRc,
components: &UsefulMenuComponents,
diag: &mut BuildDiagnostics,
no_native_menu: bool,
) -> bool {
let is_internal = matches!(&context_menu_elem.borrow().base_type, ElementType::Builtin(b) if b.name == "ContextMenuInternal");

Expand Down Expand Up @@ -306,7 +303,6 @@ fn process_context_menu(
index: 0,
ty: crate::typeregister::logical_point_type(),
},
Expression::BoolLiteral(no_native_menu),
],
source_location,
};
Expand Down
29 changes: 11 additions & 18 deletions internal/core/window.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,12 @@ pub trait WindowAdapterInternal {

fn setup_menubar(&self, _menubar: vtable::VBox<MenuVTable>) {}

fn show_context_menu(
fn show_native_popup_menu(
&self,
_context_menu: vtable::VBox<MenuVTable>,
_context_menu_item: vtable::VBox<MenuVTable>,
_position: LogicalPosition,
) {
) -> bool {
false
}

/// Re-implement this to support exposing raw window handles (version 0.6).
Expand Down Expand Up @@ -1157,17 +1158,6 @@ impl WindowInner {
}
}

/// Show a native context menu
pub fn show_context_menu(
&self,
context_menu: vtable::VBox<MenuVTable>,
position: LogicalPosition,
) {
if let Some(x) = self.window_adapter().internal(crate::InternalToken) {
x.show_context_menu(context_menu, position);
}
}

/// Show a popup at the given position relative to the `parent_item` and returns its ID.
/// The returned ID will always be non-zero.
/// `is_menu` specifies whether the popup is a popup menu.
Expand Down Expand Up @@ -1295,11 +1285,14 @@ impl WindowInner {
/// Returns false if the native platform doesn't support it
pub fn show_native_popup_menu(
&self,
_context_menu_item: &ItemRc,
_position: LogicalPosition,
context_menu_item: vtable::VBox<MenuVTable>,
position: LogicalPosition,
) -> bool {
// TODO
false
if let Some(x) = self.window_adapter().internal(crate::InternalToken) {
x.show_native_popup_menu(context_menu_item, position)
} else {
false
}
}

// Close the popup associated with the given popup window.
Expand Down
Loading