Skip to content
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

Convert all uses of UICommand to UIAction #9332

Closed
Tracked by #9139
PureWeen opened this issue Aug 10, 2022 · 2 comments · Fixed by #16174
Closed
Tracked by #9139

Convert all uses of UICommand to UIAction #9332

PureWeen opened this issue Aug 10, 2022 · 2 comments · Fixed by #16174
Labels
area-architecture Issues with code structure, SDK structure, implementation details area-controls-menubar Desktop MenuBarItems fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/macOS 🍏 macOS / Mac Catalyst t/desktop The issue relates to desktop scenarios (MacOS/MacCatalyst/Windows/WinUI/WinAppSDK)

Comments

@PureWeen
Copy link
Member

PureWeen commented Aug 10, 2022

Description

OnHold until: https://github.com/dotnet/maui/pull/14740/files/49f5f1f0685a17e332d3d67e4f654a605ccdae47#diff-f1798675642557df37437c1b2586c65818edf78c04b02740ff587e2d3486fc0fR50

Currently for Menus on Catalyst we are using UICommand's which requires selectors. This leads to a lot of awkward extra code. It appears that we can just use UIAction instead.

@PureWeen PureWeen added the area-architecture Issues with code structure, SDK structure, implementation details label Aug 10, 2022
@PureWeen PureWeen added this to the .NET 7 milestone Aug 10, 2022
@Eilon
Copy link
Member

Eilon commented Aug 10, 2022

One thing I'm not sure of is that with the selector stuff and UICommand, it might be easier to integrate with the default MacOS menus that nearly every app has, like File/Edit/View. I'm not sure if that works with UIAction. Definitely something to consider.

@PureWeen
Copy link
Member Author

I did a basic test just plopping in UIAction instead of UICommand and it seems like it works

AFAICT the only difference between the two is that one uses a closure and the other uses a selector

image

Still haven't found anything indicating the why of doing one vs the other.
The one advantage I could see with UICommand is that the actions all execute through the MauiAppDelegate menu override so in theory a user could hook into that for every menu item even ones they've added

@mattleibow mattleibow mentioned this issue Sep 2, 2022
22 tasks
@samhouts samhouts added platform/iOS 🍎 legacy-area-desktop Windows / WinUI / Project Reunion & Mac Catalyst / macOS specifics (Menus & other Controls)) area-controls-menubar Desktop MenuBarItems platform/macOS 🍏 macOS / Mac Catalyst partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with and removed platform/iOS 🍎 labels Jan 6, 2023
@PureWeen PureWeen added the good first issue Good for newcomers label Mar 2, 2023
@dustin-wojciechowski dustin-wojciechowski self-assigned this Mar 10, 2023
@PureWeen PureWeen removed the good first issue Good for newcomers label May 26, 2023
@samhouts samhouts modified the milestones: .NET 8 Planning, .NET 8 Jul 28, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! label Aug 8, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2023
@Eilon Eilon added t/desktop The issue relates to desktop scenarios (MacOS/MacCatalyst/Windows/WinUI/WinAppSDK) and removed legacy-area-desktop Windows / WinUI / Project Reunion & Mac Catalyst / macOS specifics (Menus & other Controls)) labels May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-architecture Issues with code structure, SDK structure, implementation details area-controls-menubar Desktop MenuBarItems fixed-in-8.0.0-preview.7.8842 Look for this fix in 8.0.0-preview.7.8842! partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/macOS 🍏 macOS / Mac Catalyst t/desktop The issue relates to desktop scenarios (MacOS/MacCatalyst/Windows/WinUI/WinAppSDK)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants