-
Notifications
You must be signed in to change notification settings - Fork 644
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
[WIP] CommandBar in PageHeader #1692
base: main
Are you sure you want to change the base?
Conversation
71a30dd
to
0a1bfda
Compare
0a1bfda
to
f2ff047
Compare
a259a65
to
e9fa837
Compare
Hey @Jay-o-Way Thank you for taking the time to create and submit your PR. We’ve carefully reviewed it, and while we appreciate the effort, we’ve decided not to proceed with it in its current form. Here's our thinking:
We value the effort you’ve put into this contribution, but for future submissions involving significant UX changes, we should discuss the concept in an issue first. This ensures we can collaborate on the best possible solution and are aligned on the direction. That will avoid unnecessary development work. |
Why is that review not public? I would like to see that.
Those apps are different already. Not a strong argument to me.
I would assume a developer would know to find these buttons. Especially one who used the app before. Note that this is still a work in progress, it's easy to change - like, moving more buttons out of the SecondaryCommands part.
Now this is where I just get angry. You know very well I've been trying to draw attention to that issue for nine months and it has had no response at all! You all ignored it (and therefore me) and telling me this now is hypocritical and insulting! |
Description
This PR shows the use of a CommandBar in the PageHeader, to accommodate several buttons. It includes @niels9001 his new button for "About API", and I also started to add a Share button, that's supposed to open the OS Share window (ShowShareUIForWindow). The idea is that this can replace the CopyLinkButton. However, at the moment this is not functioning yet, because the (example*) code assumes this CS to be in MainWindow.xaml.cs and it uses a
this
keyword that doesn't work in the current place. #needhelpCode example from https://learn.microsoft.com/windows/apps/develop/ui-input/display-ui-objects#for-classes-that-implement-idatatransfermanagerinterop
WinUI-Gallery/WinUIGallery/Controls/PageHeader.xaml.cs
Lines 116 to 117 in 4505036
Tasks
Motivation and Context
This is a nice demonstration of a CommandBar. It is also adaptive in terms of width, so can't have any issues regarding size.
Issues:
Note: Incompatible with PR #1679
How Has This Been Tested?
Locally.
Screenshots (if appropriate):
Types of changes