- [Android] Fix ShouldShowToolbarButton for FlyoutPage#18522
- [Android] Fix ShouldShowToolbarButton for FlyoutPage#18522rmarinho merged 1 commit intodotnet:mainfrom bradencohen:fix-15111
Conversation
|
Hey there @bradencohen! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
Any idea on when this could be reviewed @mattleibow @jsuarezruiz ? No rush, there is a decent workaround posted, just wanted to see if there was anything else required on my end. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
I see that some checks were unsuccessful. Let me know if there's anything I can do to help resolve that! |
|
/rebase |
…n setting the drawer toggle visibility.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
I want to include some tests in this PR. The same where reproduce the issue is this one https://github.com/GODston/Public right? |
Yes it is. I'm still learning and would be super happy to write some tests, are there any good examples to follow? I can take a stab at it this weekend 😄. @jsuarezruiz |
|
@jsuarezruiz @bradencohen created a issue to follow up adding a global test for this on all platforms. |
Description of Change
Fixes an issue where overriding
ShouldShowToolbarButton()in aFlyoutPagedoes not hide the hamburger icon (drawer toggle) on Android.In the
NavigationPageToolbar, the drawer toggle is marked visible if the parent is aFlyoutPageand there are no pages pushed on to the stack (here).There should also be a call made to
ShouldShowToolbarButtonin cases where you want to explicitly hide the toolbar button, which is the hamburger icon.This has no effect on the back button, which is handled separately.
On iOS,
ShouldShowToolbarButtonis called in the compatibilityNavigationRenderer, which is still used as the primary handler today and explains why it's only broken in Android.Issues Fixed
Fixes #15111
Hope this helps! Let me know if you need further adjustments.