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.

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

[Windows] MenuBarItems are now rendered when Shell.Title is not set #6497

Merged
merged 8 commits into from
Apr 29, 2022
Merged

[Windows] MenuBarItems are now rendered when Shell.Title is not set #6497

merged 8 commits into from
Apr 29, 2022

Conversation

SotoiGhost
Copy link
Contributor

Description of Change

  • MenuBarItems are now rendered when Shell.Title is not set on Windows

Issues Fixed

Fixes #6303

@jsuarezruiz jsuarezruiz changed the title [UWP] MenuBarItems are now rendered when Shell.Title is not set [Windows] MenuBarItems are now rendered when Shell.Title is not set Apr 26, 2022
@jsuarezruiz jsuarezruiz added platform/windows 🪟 area-controls-shell Shell Navigation, Routes, Tabs, Flyout labels Apr 26, 2022
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@rmarinho rmarinho requested a review from PureWeen April 26, 2022 10:57
src/Controls/src/Core/Toolbar.cs Outdated Show resolved Hide resolved
src/Controls/src/Core/ShellToolbar.cs Outdated Show resolved Hide resolved
Copy link
Member

@PureWeen PureWeen left a comment

Choose a reason for hiding this comment

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

  • So, I think we need to restrict this check to windows only. iOS/Catalyst/Android shouldn't show the toolbar if the MenuBarItems are present. That behavior is only relevant on Windows
  • Given that I think it makes sense to move your tests to ShellTests.cs and then if you look at the NavigationPageTests there are checks in there for IsNavigationBarVisible which check at the platform level to see if the NavigationBar is visible. We can probably move those methods up to HandlerTestBase

@PureWeen PureWeen merged commit 2d83ad9 into dotnet:main Apr 29, 2022
@SotoiGhost SotoiGhost deleted the issoto/fix/6303 branch May 2, 2022 14:59
@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout platform/windows 🪟
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MenuBarItems are not rendered if Title is not set
3 participants