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.

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

Jump to bottom

Introduce MenuItem.ToggleType #11441

Merged
merged 14 commits into from
Feb 14, 2024
Merged

Introduce MenuItem.ToggleType #11441

merged 14 commits into from
Feb 14, 2024

Conversation

maxkatz6
Copy link
Member

@maxkatz6 maxkatz6 commented May 19, 2023 •
edited
Loading

What does the pull request do?

  • Adds MenuItem.ToggleType APIs
  • Polish radio button functionality (i.e. it should work with hierarchy property).
  • Add fluent styles for radio/simple menu items
  • Add simple styles for radio/simple menu items
  • Integrate into NativeMenuItem
  • Ensure that it's usable with binding menus
  • Add tests
  • (bonus) Add icon and separator support for the top-level Menu
  • (optional) Remove support for Header="-", but instead generate Separator container depending on the data context
demo.mp4

Checklist

Fixed issues

tbd
Fixes #7663
Fixes #224
Fixes #1902
Fixes #5397
Maybe #10794

@maxkatz6 maxkatz6 marked this pull request as draft May 19, 2023 03:45
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0035049-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@grokys
Copy link
Member

grokys commented May 19, 2023

Remove support for Header="-", but instead generate Separator container depending on the data context

Yep, this should now be possible with the recent changes to how container generation works.

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0035163-beta. (feed url: https://pkgs.dev.azure.com/AvaloniaUI/AvaloniaUI/_packaging/avalonia-all/nuget/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 changed the title [WIP] Init work with Toggle and RadioMenuItem [WIP] Init work with MenuItem.ToggleType May 21, 2023
@amwx amwx mentioned this pull request May 29, 2023
Closed
@IsaacMarovitz
Copy link

Any update on this? Since the release of Avalonia 11, the old workaround of adding a checkbox as a MenuItem Icon doesn't work properly amwx/FluentAvalonia#367

@jgcodes2020
Copy link
Contributor

Is this PR still being worked on?

@maxkatz6 maxkatz6 mentioned this pull request Oct 26, 2023
Closed
@jmacato
Copy link
Member

jmacato commented Nov 17, 2023

Closing this PR temporarily due to inactivity. Please ping me if this needs to be reopened.

@jmacato jmacato closed this Nov 17, 2023
@toomasz
Copy link

toomasz commented Nov 23, 2023

@jmacato Why wasn't it merged? Would be nice to have this feature

@timunie
Copy link
Contributor

timunie commented Nov 23, 2023

Closing this PR temporarily due to inactivity. Please ping me if this needs to be reopened.

that is why. Lack of time.

@maxkatz6 maxkatz6 reopened this Nov 23, 2023
@maxkatz6 maxkatz6 added this to the 11.x milestone Nov 23, 2023
@maxkatz6 maxkatz6 changed the title [WIP] Init work with MenuItem.ToggleType Introduce MenuItem.ToggleType Dec 19, 2023
@maxkatz6 maxkatz6 marked this pull request as ready for review December 19, 2023 10:59
@maxkatz6 maxkatz6 added feature and removed inactive labels Dec 19, 2023
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0043062-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 requested a review from a team January 12, 2024 04:55
grokys
grokys requested changes Jan 29, 2024
View reviewed changes
Copy link
Member

@grokys grokys left a comment

Choose a reason for hiding this comment

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

A few questions inline, and what might be a bug: the native menu in ControlCatalog doesn't seem to display any check icons on win32:

image

samples/ControlCatalog/ViewModels/MainWindowViewModel.cs Outdated
@@ -43,7 +42,7 @@ public MainWindowViewModel()

ToggleMenuItemCheckedCommand = MiniCommand.Create(() =>
{
IsMenuItemChecked = !IsMenuItemChecked;
// handle
Copy link
Member

Choose a reason for hiding this comment

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

Not sure what this comment means. Should this command be removed or does something need to be implemented here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's unused now, removed

Copy link
Member

Choose a reason for hiding this comment

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

Seems to still be there?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an outdated file, I don't see this command in the code

src/Avalonia.Controls/MenuBase.cs Show resolved Hide resolved
src/Avalonia.Controls/MenuItem.cs Outdated Show resolved Hide resolved
src/Avalonia.Controls/MenuItem.cs Outdated Show resolved Hide resolved
src/Avalonia.Controls/RadioButtonGroupManager.cs Outdated Show resolved Hide resolved
src/Avalonia.Diagnostics/Diagnostics/Views/MainWindow.xaml.cs Outdated Show resolved Hide resolved
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0044129-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 requested a review from grokys February 3, 2024 06:23
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0044411-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

grokys
grokys requested changes Feb 6, 2024
View reviewed changes
Copy link
Member

@grokys grokys left a comment

Choose a reason for hiding this comment

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

Mostly looking good, but I'm still seeing NativeMenuBar isn't displaying toggles as in the last review. It should be, right?

image

samples/ControlCatalog/ViewModels/MainWindowViewModel.cs Outdated
@@ -43,7 +42,7 @@ public MainWindowViewModel()

ToggleMenuItemCheckedCommand = MiniCommand.Create(() =>
{
IsMenuItemChecked = !IsMenuItemChecked;
// handle
Copy link
Member

Choose a reason for hiding this comment

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

Seems to still be there?

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.1.999-cibuild0044537-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 requested a review from grokys February 7, 2024 08:55
@grokys grokys added this pull request to the merge queue Feb 14, 2024
Merged via the queue into master with commit 6c1341e Feb 14, 2024
7 checks passed
@grokys grokys deleted the menu-item-new-api branch February 14, 2024 14:39
@rabbitism rabbitism mentioned this pull request Apr 18, 2024
Closed
@Numpsy Numpsy mentioned this pull request Jul 31, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grokys grokys grokys approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
11.x
9 participants
@maxkatz6 @avaloniaui-team @grokys @IsaacMarovitz @jgcodes2020 @jmacato @toomasz @timunie @avaloniaui-bot