-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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] Fix FlyoutPage ShouldShowToolbarButton when overridden to return false, still shows button in title bar #25857
base: main
Are you sure you want to change the base?
Conversation
Hey there @devanathan-vaithiyanathan! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
{ | ||
|
||
} | ||
public partial class Issue24547PopoverPage : FlyoutPage |
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.
D:\a\_work\1\s\src\Controls\tests\TestCases.HostApp\Issues\Issue24547.cs(4,15): error CS0260: Missing partial modifier on declaration of type 'Issue24547'; another partial declaration of this type exists [D:\a\_work\1\s\src\Controls\tests\TestCases.HostApp\Controls.TestCases.HostApp.csproj::TargetFramework=net9.0-windows10.0.20348.0]
D:\a\_work\1\s\src\Controls\tests\TestCases.HostApp\Issues\Issue24547.cs(4,15): error CS0260: Missing partial modifier on declaration of type 'Issue24547'; another partial declaration of this type exists [D:\a\_work\1\s\src\Controls\tests\TestCases.HostApp\Controls.TestCases.HostApp.csproj::TargetFramework=net9.0-windows10.0.19041.0]
8570 Warning(s)
2 Error(s)
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.
I have removed the partial keyword, could you please check once and let me know.
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
App.WaitForElement("PopButton"); | ||
App.Tap("PopButton"); | ||
App.WaitForElement("DetailButton"); | ||
VerifyScreenshot(); |
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.
Triggered build. There are some pending snapshots.
Issue details
When Popover behavior is applied to a FlyoutPage with ShouldShowToolbarButton set to false, the hamburger icon still appears in the Windows title bar.
Root Cause
In Windows, When the ShouldToolBarButton is override as false, hardly setting WinUI IsPaneToggleButtonVisible as True based on FlyoutBehavior to control navigation visibility with no handling to restrict the visibility of the ToolBarButton.
Description of Change
Since we are controlling the DrawerToggleVisible value based on the visibility of the ToolBarButton, which in turn manages the visibility of the TogglePaneButton in WinUI. This ensures that the toolbar now correctly reflects the DrawerToggleVisible state, improving the consistency of navigation element.
Issues Fixed
Fixes #24547
Tested the behaviour in the following platforms
Output Screenshot