-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
[macOS] Implement IsEnabled property in MenuFlyoutItems #14920
Conversation
Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines. |
src/Core/src/Handlers/MenuFlyoutSubItem/MenuFlyoutSubItemHandler.iOS.cs
Outdated
Show resolved
Hide resolved
Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines. |
src/Core/src/Handlers/MenuFlyoutSubItem/MenuFlyoutSubItemHandler.iOS.cs
Outdated
Show resolved
Hide resolved
I just tested this, and when the Command's CanExecute changes, the state of the menu item changes. |
It doesn't appear to propagate; setting a parent Menu Item to IsEnabled=False does not modify the child item in any way. On Windows, it doesn't matter - disabling a menu item with sub items disables it completely, to the point that you can't even reveal the sub items. So their state becomes moot. So we need to answer the question of what should happen here - should disabling a menu with sub items propagate the xplat property? Or should we just handle that on the platform side (which is doable in Catalyst, it just takes a little extra code). @PureWeen @mattleibow thoughts? |
We probably should use the same method that I did for VisualElement. We do propagate that enabled value, but not actually - otherwise we end up not knowing if we should re-enable. On the base menu element, you have a protected property: maui/src/Controls/src/Core/VisualElement/VisualElement.cs Lines 574 to 591 in 34b481d
The real IsEnabled property will coerce the value and make sure to do the math but preserve the original value: maui/src/Controls/src/Core/VisualElement/VisualElement.cs Lines 1294 to 1303 in 34b481d
The coersion will fire for all controls and disable if something is disabled, but caches the real value in a field. And it will propagate the enabled value: maui/src/Controls/src/Core/VisualElement/VisualElement.cs Lines 1387 to 1390 in 34b481d
In the derived menu items, it just overrides the IsEnabledCore with any additional things - like the command: maui/src/Controls/src/Core/Button/Button.cs Lines 462 to 466 in 34b481d
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe separate blocks?
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
src/Core/src/Handlers/MenuFlyoutSubItem/MenuFlyoutSubItemHandler.cs
Outdated
Show resolved
Hide resolved
src/Core/src/Handlers/MenuFlyoutSubItem/MenuFlyoutSubItemHandler.iOS.cs
Outdated
Show resolved
Hide resolved
woohoo! |
Description of Change
Implement IsEnabled property in MenuFlyoutItems on Catalyst.
(Work in progress)
Menu
Flyout
Issues Fixed
Fixes #9583
Fixes #15896