NOT FUNCTIONAL: Initial support for menus with checkmarks #9056

npwoods · 2025-08-03T13:04:40Z

This is an initial attempt to support menus with checkmarks. This initial support has the following caveats:

It doesn't work :-). It isn't clear to me how the state on MenuItem objects get sync'd with MenuEntry objects. I expected this to be happening within MenuFromItemTree::update_shadow_tree(), but that seems to not happen in practice. By contrast, enabled gets sync'd just fine.
The graphics for non-muda checkmarks is almost certainly insufficient. I anticipate strong opinions about what this should look like
muda seems to have distinct structs for menu items that support icons versus checkmarks. It isn't clear to me why this would be. This means that icons and checkmarks cannot be used at the same time when muda is used.
muda has some odd default behaviors for CheckMenuItem, like automatically toggling the checkmark. This behavior will likely need to be suppressed.

Separately, I would also like to see accelerator support; I do see key_sequence commented out in builtin_structs.rs. I'm sure that some of the guidance rectifying the above issues will translate for accelerator support.

This is an initial attempt to support menus with checkmarks. This initial support has the following caveats: - It doesn't work :-). It isn't clear to me how the state on `MenuItem` objects get sync'd with `MenuEntry` objects. I expected this to be happening within `MenuFromItemTree::update_shadow_tree()`, but that seems to not happen in practice. By contrast, `enabled` gets sync'd just fine. - The graphics for non-muda checkmarks is almost certainly insufficient. I anticipate strong opinions about what this should look like - `muda` seems to have distinct structs for menu items that support icons versus checkmarks. It isn't clear to me why this would be. This means that icons and checkmarks cannot be used at the same time when `muda` is used. - `muda` has some odd default behaviors for `CheckMenuItem`, like automatically toggling the checkmark. This behavior will likely need to be suppressed. Separately, I would also like to see accelerator support; I do see `key_sequence` commented out in `builtin_structs.rs`. I'm sure that some of the guidance rectifying the above issues will translate for accelerator support.

ogoffart · 2025-08-04T07:12:19Z

Thanks for the contributions.

Note that we would need both a checked and checkable flag.

It isn't clear to me how the state on MenuItem objects get sync'd with MenuEntry objects.

Depending if the menu has if or for, it is done in update_shadow_tree, or here:

slint/internal/compiler/passes/lower_menus.rs

Line 745 in 351324a

for prop in ["title", "enabled"] {

muda seems to have distinct structs for menu items that support icons versus checkmarks. It isn't clear to me why this would be. This means that icons and checkmarks cannot be used at the same time when muda is used.

Then priority to check mark.
It may be either a limitation of muda, or the way it is in some platform API.

Separately, I would also like to see accelerator support

Some work for that has started (eg in #8906)

- Handling `checkable` and `checked` when copying into `MenuEntry` - Changing `muda` code to honor `checkable` and give it priority over `icon`

ogoffart · 2025-08-14T08:05:01Z

Is it ready to be reviewed?

npwoods · 2025-08-14T10:37:50Z

It's ready with the caveats acknowledged in the original PR. I think you provided feedback that allowed me to ddress the first and third.

npwoods · 2025-08-14T23:38:38Z

The only definite problem I'm aware about is the last problem; muda (at least on Windows) seems to unilaterally decide to toggle checkmarks on checked menu items

Relevant code on Windows: https://github.com/tauri-apps/muda/blob/ff2cecc76b69b54ed253fd526320f9c50ddb6236/src/platform_impl/windows/mod.rs#L1193

It isn't clear to me why muda does this. I suspect that you guys might have stronger opinions on what should be done here.

ogoffart · 2025-08-15T10:08:31Z

I made a change in #9159 that is causing conflict but should simplify the implementation a bit

npwoods · 2025-08-15T12:08:58Z

Merged!

ogoffart · 2025-08-15T14:06:42Z

Thanks!

Could you please add documentation in contextmenuarea.mdx.

Also maybe have a checkable entry in the gallery as an example.

The only definite problem I'm aware about is the last problem; muda (at least on Windows) seems to unilaterally decide to toggle checkmarks on checked menu items

I'm not exactly sure what behavior we want here.
I see a few option:

Do the same as in muda in our code: make checked an in-out property and let people do binding like this

   some-item := MenuItem {
       title: "Check me";
       checkable: true;
   } 

   // ... elsewhere, use the property in a binding or something
   background: some-item.checked ? blue : red;

Don't automatically change the checkable as you implemented. User would be expected to do a activated => { self.checked = !self.checked } themselves (and can potentially add more checking code)

I'm tempted do go with the option 1. I think we should change the value of checked automaticallty (This can be done in MenuFromItemTree::activate, I'd say.)

1. Adding documentation to `contextmenuarea.mdx` 2. Adding logic to toggle checkmarks

ogoffart

Nice!

Could you also add a tests?
Prehaps you can extend this test
https://github.com/slint-ui/slint/blob/master/tests/cases/widgets/menubar.slint

and this one
https://github.com/slint-ui/slint/blob/master/tests/cases/widgets/contextmenu.slint

to include a checkable menu and checked that the property is correctly updated.

internal/compiler/builtins.slint

Co-authored-by: Olivier Goffart <[email protected]>

npwoods · 2025-08-16T14:16:50Z

I've created two new test cases, but it isn't clear to me how these test cases are executed (the code behind seems to just be comments in the Slint code; presumably there is a test harness?)

Also in the logic that actually does the toggling, I query for the status of the checkable/checked properties with a method called get_internal(). This does not look like the proper solution; I defer to you

ogoffart · 2025-08-16T15:41:36Z

it isn't clear to me how these test cases are executed

There are test driver that run these. See https://github.com/slint-ui/slint/blob/master/docs/testing.md#driver-tests

with a method called get_internal()

Indeed that's not good. You need to get a Pin. You can do that with
menu_item.as_pin_ref().checkable() or something like that.

npwoods · 2025-08-20T09:04:49Z

Are there any more outstanding issues related to this PR?

ogoffart · 2025-08-20T13:09:21Z

Sorry for the delay.

This is looking good.
The menubar c++ test is failing. Please edit the cpp part of the test with the same thing as rust.

Also please add a entry to the gallery's menu

- Fixing C++ test

ogoffart

Thanks!

tronical

API looks good to me, too.

docs/astro/src/content/docs/reference/window/contextmenuarea.mdx

Co-authored-by: Simon Hausmann <[email protected]>

tronical · 2025-08-21T07:16:58Z

internal/core/menus.rs

        {
            if let Some(menu_item) = menu_item.downcast::<MenuItem>() {
+                if menu_item.as_pin_ref().checkable() {
+                    menu_item.checked.set(!menu_item.as_pin_ref().checked());


Ah, I suspect this is what I’m looking for?

npwoods and others added 3 commits August 4, 2025 21:35

- Added checkable property to MenuItem and MenuEntry

31ce91e

- Handling `checkable` and `checked` when copying into `MenuEntry` - Changing `muda` code to honor `checkable` and give it priority over `icon`

[autofix.ci] apply automated fixes

2e300be

A better check mark when muda is not available

2fd5fc0

Merge branch 'master' into menus_with_checkmarks

136ae1a

Feedback from ogoffart:

dcb8cd0

1. Adding documentation to `contextmenuarea.mdx` 2. Adding logic to toggle checkmarks

ogoffart reviewed Aug 16, 2025

View reviewed changes

internal/compiler/builtins.slint Outdated Show resolved Hide resolved

npwoods and others added 2 commits August 16, 2025 08:52

Update internal/compiler/builtins.slint

d3ab433

Co-authored-by: Olivier Goffart <[email protected]>

Adding test cases for checkable/checked

328e319

npwoods added 2 commits August 16, 2025 12:17

Fixing property access

07527e2

Fixing menubar.slint test

53a2368

- Adding a checkmark item to gallery example

224d2c3

- Fixing C++ test

ogoffart approved these changes Aug 21, 2025

View reviewed changes

tronical reviewed Aug 21, 2025

View reviewed changes

docs/astro/src/content/docs/reference/window/contextmenuarea.mdx Outdated Show resolved Hide resolved

Update docs/astro/src/content/docs/reference/window/contextmenuarea.mdx

99a4bf9

Co-authored-by: Simon Hausmann <[email protected]>

ogoffart merged commit 19d8350 into slint-ui:master Aug 21, 2025
38 checks passed

tronical reviewed Aug 21, 2025

View reviewed changes

NOT FUNCTIONAL: Initial support for menus with checkmarks #9056

NOT FUNCTIONAL: Initial support for menus with checkmarks #9056

Uh oh!

Conversation

npwoods commented Aug 3, 2025

Uh oh!

ogoffart commented Aug 4, 2025

Uh oh!

ogoffart commented Aug 14, 2025

Uh oh!

npwoods commented Aug 14, 2025

Uh oh!

npwoods commented Aug 14, 2025

Uh oh!

ogoffart commented Aug 15, 2025

Uh oh!

npwoods commented Aug 15, 2025

Uh oh!

ogoffart commented Aug 15, 2025

Uh oh!

ogoffart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

npwoods commented Aug 16, 2025

Uh oh!

ogoffart commented Aug 16, 2025

Uh oh!

npwoods commented Aug 20, 2025

Uh oh!

ogoffart commented Aug 20, 2025

Uh oh!

ogoffart left a comment

Choose a reason for hiding this comment

Uh oh!

tronical left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tronical Aug 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!