-
Notifications
You must be signed in to change notification settings - Fork 711
Replace NavigationRootPage with MainWindow #1908
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
Conversation
| { | ||
| item.IsSelected = true; | ||
| } | ||
| //if (e.Parameter is NavigationRootPageArgs args && args.NavigationRootPage.NavigationView.MenuItems.OfType<NavigationViewItem>().FirstOrDefault(item => item.Name == "AllControlsItem") is NavigationViewItem item) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oversight, I guess! Would you mind taking a look at this to get this PR in?
|
I'll update this PR with a custom TitleBar so we are not blocked by the known issues. (Or if someone else can do it if they want to jump in :). The titlebar from Windows Community Toolkit Labs would be a great start! |
|
What exactly was the problem up til now? And cant we choose to work around it, or just use it with a relatively small imperfection? |
|
I think for now it is fine to keep the titlebar implementation we currently have and focus on decluttering the files and their purpose. WinUI 3 titlebar has some bugs including making resizing the window sometimes hide window content so we should not introduce known regressions like that. |
|
As a workaround, could we avoid using the IconSource property of the TitleBar control and instead place an icon in the left header of the TitleBar? |
Yeah that works. Would you mind create a Pr against this branch so we can merge that in and get this PR ready for review too? |
|
…the GalleryIcon.ico (#1915) <!--- Provide a general summary of your changes in the Title above --> ## Description This PR targets #1908. ## Motivation and Context - It provides a temporary workaround for microsoft/microsoft-ui-xaml#10427. ## How Has This Been Tested? Manually tested ## Screenshots (if appropriate): ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) --------- Co-authored-by: Niels Laute <niels.laute@live.nl>
|
Thanks @Zakariathr22 ! This PR is now ready for review (cc @marcelwgn). |
1 similar comment
|
Thanks @Zakariathr22 ! This PR is now ready for review (cc @marcelwgn). |
marcelwgn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work and cool that we can also use the new TitleBar control (though I certainly need some time to get used to that centered search bar)
|
/azp run |
|
Hmm, looks like the UI tests are failing because of some of these changes. |
This comment has been minimized.
This comment has been minimized.
|
I will look at the test failure, the search box got renamed and the new UIA name of the search field seems to be "TextBox"... |
|
/azp run |
|
/azp run |
That seemed to work 🚀🚀 |
Description
This PR creates a XAML-defined
MainWindowfor improved maintaining of the shell UX going forward. It also uses the newTitleBarcontrol and moving the searchbox to the top of the window, following UX-guidance and saving some vertical space in the NavView.Closes: #1736
[DO NOT MERGE]: Blocked onTitleBarbugsFor TitleBar samples, see: #1797