Skip to content

[OSX] Refactor Native App Menu. Move default menu initialization code from ObjC to C#. #6909

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

Merged
merged 8 commits into from
Dec 11, 2021

Conversation

Mikolaytis
Copy link
Contributor

What does the pull request do?

  • refactoring of default native app menu initialization code. Moved from native lib to C#.
  • Exposed three macOS commands - hide, show all, hide other over INativeApplicationCommands.
  • Implemented DisableDefaultApplicationMenuItems in C# side.

What is the current behavior?

MacOS default menu initialization is hard-coded in native lib. No ability to localize. No ability to use mac specific commands.

What is the updated/expected behavior with this PR?

Any user now can add their own app menu in macOS including default macOS specific menu commands.

Checklist

Fixed issues

#6903

@Mikolaytis
Copy link
Contributor Author

Mikolaytis commented Nov 10, 2021

After this PR our app menu looks like:

<NativeMenuItem Command="{Binding ...}"
				Gesture="{Gesture About}"
				Header="{Loc MainMenu_AboutLunacy}" />
<NativeMenuItemSeparator />
<NativeMenuItem Command="{Binding ...}" Header="{Loc MainMenu_CheckForUpdates}" />
<NativeMenuItemSeparator />
<NativeMenuItem Header="{Loc MainMenu_Services}">
	<NativeMenu IsServicesSubmenu="True" />
</NativeMenuItem>
<NativeMenuItemSeparator />
<NativeMenuItem Command="{x:Static controls:AvaloniaCommands.HideAppCommand}" Header="{Loc MainMenu_HideLunacy}" Gesture="Meta+H" />
<NativeMenuItem Command="{x:Static controls:AvaloniaCommands.HideOthersCommand}" Header="{Loc MainMenu_HideOthers}" Gesture="Meta+Alt+H" />
<NativeMenuItem Command="{x:Static controls:AvaloniaCommands.ShowAllCommand}" Header="{Loc MainMenu_ShowAll}" />
<NativeMenuItemSeparator />
<NativeMenuItem Command="{Binding ...}"
				Gesture="{Gesture Quit}"
				Header="{Loc MainMenu_QuitLunacy}" />

Notice: IsServicesSubmenu usage.

public static class AvaloniaCommands
{
    public static ICommand HideAppCommand = new RelayCommand(() =>
        AvaloniaLocator.Current.GetService<INativeApplicationCommands>().HideApp());
    public static ICommand HideOthersCommand = new RelayCommand(() =>
        AvaloniaLocator.Current.GetService<INativeApplicationCommands>().HideOthers());
    public static ICommand ShowAllCommand = new RelayCommand(() =>
        AvaloniaLocator.Current.GetService<INativeApplicationCommands>().ShowAll());
}

@kekekeks
Copy link
Member

@danwalmsley

@kekekeks
Copy link
Member

I haven't looked at the code diff, but I see some problems just from looking at the API:

The problem with the API is that it exposes properties in a way that doesn't indicate them being OSX-specific. While it's not generally a problem for the main app menu since we only use it for OSX, NativeMenuItem itself is also used for Linux global menu and for TrayIcon across all platforms.

So I'd suggest to rename AvaloniaCommands to MacOSNativeMenuCommands and make IsServicesSubmenu to be an attached property provided by that class.

@maxkatz6 maxkatz6 enabled auto-merge (squash) December 11, 2021 00:14
@maxkatz6 maxkatz6 merged commit 8578160 into AvaloniaUI:master Dec 11, 2021
@robloo robloo mentioned this pull request Dec 11, 2021
danwalmsley added a commit that referenced this pull request Dec 13, 2021
… from ObjC to C#. (#6909)

* [NativeMenu] [Refactoring] Move Default Menu creation from native lib to C# (C# side of code)

* fix return type for IAvnApplicationCommands

* [Native] menu refactoring (ObjC side)

* fix nullref

* minor refactor

Co-authored-by: Jumar Macato <[email protected]>
Co-authored-by: Dan Walmsley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants