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

feat: MenuFlyout support for keyboard accelerators #17228

Closed
wants to merge 28 commits into from
Closed

feat: MenuFlyout support for keyboard accelerators #17228

wants to merge 28 commits into from

Conversation

MartinZikmund
Copy link
Member

@MartinZikmund MartinZikmund commented Jun 19, 2024 •
edited
Loading

GitHub Issue (If applicable): closes #17133, closes #15195

PR Type

What kind of change does this PR introduce?

  • Feature

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@github-actions github-actions bot added area/skia ✏️ Categorizes an issue or PR as relevant to Skia area/build Categorizes an issue or PR as relevant to build infrastructure platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform labels Jun 19, 2024
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@MartinZikmund MartinZikmund force-pushed the dev/mazi/keyboard-accelerator-menu branch 2 times, most recently from 26d5000 to bd6fd7a Compare June 20, 2024 11:31
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@MartinZikmund MartinZikmund force-pushed the dev/mazi/keyboard-accelerator-menu branch from a2e5c77 to ec4a06a Compare June 21, 2024 08:55
@github-actions github-actions bot removed area/skia ✏️ Categorizes an issue or PR as relevant to Skia area/build Categorizes an issue or PR as relevant to build infrastructure labels Jun 21, 2024
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@MartinZikmund MartinZikmund marked this pull request as ready for review June 21, 2024 12:02
@MartinZikmund MartinZikmund changed the title [WIP] feat: MenuFlyout support for keyboard accelerators feat: MenuFlyout support for keyboard accelerators Jun 21, 2024
@github-actions github-actions bot added the platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform label Jun 21, 2024
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

1 similar comment
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@MartinZikmund MartinZikmund force-pushed the dev/mazi/keyboard-accelerator-menu branch from 522f71d to 8903703 Compare June 24, 2024 17:02
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@MartinZikmund MartinZikmund requested review from jeromelaban and Youssef1313 and removed request for jeromelaban June 25, 2024 10:01
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-17228/index.html

@MartinZikmund MartinZikmund mentioned this pull request Jun 25, 2024
Draft
6 tasks
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

1 similar comment
@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-17228/index.html

jeromelaban
jeromelaban reviewed Jun 26, 2024
View reviewed changes
src/Uno.UI/UI/Xaml/Controls/AppBar/AppBarButton.Partial.cs Show resolved Hide resolved
src/Uno.UI/UI/Xaml/ParentVisualTreeListener.cs
Comment on lines +20 to +21
_onLoaded = onLoaded;
_onUnloaded = onUnloaded;
Copy link
Member

Choose a reason for hiding this comment

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

Could this cause memory leaks? We may need additional leak testing for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, will have to improve that still

Copy link
Member Author

Choose a reason for hiding this comment

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

@jeromelaban is there a blocker for #15540?

@MartinZikmund
feat: ToggleMenuFlyoutItemAutomationPeer
3399595
@MartinZikmund
feat: Update ToggleMenuFlyoutItem to winui3/release/1.5.4
64a5c2d
@MartinZikmund
chore: Add helper MenuFlyout classes
accab48
@MartinZikmund
feat: Update MenuFlyoutSubItem to winui3/release/1.5.4
6acd57b
@MartinZikmund
feat: MenuFlyoutSubItemAutomationPeer
4551ec5
@MartinZikmund
chore: In progress adjustments of MenuFlyout
130e423
@MartinZikmund
feat: MenuFlyoutItemAutomationPeer
b12bb9d
@MartinZikmund
feat: MenuFlyoutPresenterAutomationPeer
9b21bee
@MartinZikmund
feat: Update MenuFlyoutItem to winui3/release/1.5.4
7c481e7
@MartinZikmund
chore: Updating MenuFlyout
45cba9a
@MartinZikmund
feat: Update MenuFlyout to winui3/release/1.5.4
dad45d8
@MartinZikmund
chore: Adjustments to resolve errors
93041b0
@MartinZikmund
test: Re-enable MenuFlyout Keyboard Accelerator tests
f92f97b
@MartinZikmund
chore: Ensure KeyboardAccelerators are registered in OnPropertyChanged
f109756
@MartinZikmund
chore: Use Invoke instead of InvokeClick
a5c575f
@MartinZikmund
feat: CascadingMenuHelper update to winui3/release/1.5.4
f9c308d
@MartinZikmund
chore: Mark additional files updated
1be2d2e
@MartinZikmund
feat: ElevationHelper
842defd
@MartinZikmund
feat: Update MenuFlyoutPresenter to winui3/release/1.5.4
0017c4e
@MartinZikmund
chore: Cleanup unused code
3d94bf0
@MartinZikmund
test: Enable additional KA tests
279dfec
@MartinZikmund
chore: Leave KACollection on change
5424ebb
@MartinZikmund
chore: Unused members
db788fa
@MartinZikmund
chore: Adjust build
2182678
@MartinZikmund
chore: New hiding
83f6063
@MartinZikmund MartinZikmund force-pushed the dev/mazi/keyboard-accelerator-menu branch from b32344f to 83f6063 Compare August 25, 2024 13:25
@unodevops
Copy link
Contributor

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-17228/index.html

@unodevops
Copy link
Contributor

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-17228/index.html

@MartinZikmund
Copy link
Member Author

Replaced by #17292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeromelaban jeromelaban jeromelaban left review comments

@Youssef1313 Youssef1313 Awaiting requested review from Youssef1313

Assignees
No one assigned
Labels
platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform
Projects
None yet
Milestone
No milestone
3 participants
@MartinZikmund @unodevops @jeromelaban