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

[regression/8.0.0-preview.5.8529] FlyoutPage Navigation broken in Android #18050

Closed
FlavioGoncalves-Cayas opened this issue Oct 16, 2023 · 3 comments · Fixed by #18684
Closed
Assignees
Labels
area-controls-flyout Flyout fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! fixed-in-8.0.10 fixed-in-8.0.14 fixed-in-8.0.40 i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 t/bug Something isn't working
Milestone

Comments

@FlavioGoncalves-Cayas
Copy link

FlavioGoncalves-Cayas commented Oct 16, 2023

Description

2 major issues with FlyoutPage Navigation in .NET 8 RC2, breaking existing applications using FlyoutPage:

  1. Changing detail page does not update Title
  2. Pushing a page with PushAsync after changing Detail page of FlyoutPage, breaks back navigation Button (Android OS back button still works)
Bildschirmaufnahme.2023-10-16.um.14.45.45.mov

Steps to Reproduce

See repro project

Link to public reproduction project repository

https://github.com/FlavioGoncalves-Cayas/FlyoutNavigationTest

Version with bug

8.0.0-preview.5.8529

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.0-preview.4.8333

Affected platforms

Android

Affected platform versions

Android 13

Did you find any workaround?

No response

Relevant log output

No response

@FlavioGoncalves-Cayas FlavioGoncalves-Cayas added the t/bug Something isn't working label Oct 16, 2023
@jsuarezruiz jsuarezruiz added area-controls-flyout Flyout platform/android 🤖 potential-regression This issue described a possible regression on a currently supported version., verification pending labels Oct 16, 2023
@PureWeen PureWeen added this to the .NET 8 GA milestone Oct 16, 2023
@samhouts samhouts changed the title FlyoutPage Navigation broken in Android .NET 8 RC2 [regression/8.0.0] FlyoutPage Navigation broken in Android Oct 16, 2023
@samhouts
Copy link
Member

Confirmed that this regressed between 8.0.0-preview.4.8333 and 8.0.0-preview.5.8529. #14867 seems sus.

@samhouts samhouts added i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Oct 16, 2023
@samhouts samhouts modified the milestones: .NET 8 GA, .NET 8 SR1 Oct 16, 2023
@samhouts samhouts changed the title [regression/8.0.0] FlyoutPage Navigation broken in Android [regression/8.0.0-preview.5.8529] FlyoutPage Navigation broken in Android Oct 16, 2023
@FlavioGoncalves-Cayas
Copy link
Author

Do I unterstand this correctly, that the milestone-change to .NET 8 SR1 means this regression will not be fixed with GA?

@PureWeen PureWeen removed their assignment Nov 3, 2023
@emaf emaf self-assigned this Nov 6, 2023
emaf added a commit that referenced this issue Nov 10, 2023
This fix complements fb82b83 to fix toolbar issues when swapping Detail pages. That commit was necessary, but on Android we also need to make sure the `Toolbar.Handler` is disconnected, otherwise the old toolbar is never removed and we start stacking them.

This was already happening for `Window`  but not for `Page` . This code is now shared between both to avoid this problem where only one place is changed.

Fixes #18050
emaf added a commit that referenced this issue Nov 14, 2023
This fix complements fb82b83 to fix toolbar issues when swapping Detail pages. That commit was necessary, but on Android we also need to make sure the `Toolbar.Handler` is disconnected, otherwise the old toolbar is never removed and we start stacking them.

This was already happening for `Window`  but not for `Page` . This code is now shared between both to avoid this problem where only one place is changed.

Fixes #18050
PureWeen added a commit that referenced this issue Nov 15, 2023
* [Android] Fix Flyout toolbar

This fix complements fb82b83 to fix toolbar issues when swapping Detail pages. That commit was necessary, but on Android we also need to make sure the `Toolbar.Handler` is disconnected, otherwise the old toolbar is never removed and we start stacking them.

This was already happening for `Window`  but not for `Page` . This code is now shared between both to avoid this problem where only one place is changed.

Fixes #18050

* Add missing condition

* Make test run on all platforms and fix iOS back button check

- Moved test to FlyoutPageTests since it tests both Flyout and Navigation.
- Changed iOS HasBackButton check to check if there is a BackItem, otherwise we were checking for any button in the navigation bar, so it was returning true even for the flyout button.

* Fix condition

* Skip MacCatalyst

* - fix test timings

* Stop skipping MacCatalyst

---------

Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
@FlavioGoncalves-Cayas
Copy link
Author

Glad to see this issue fixed. Thanks @emaf.
Will this make GA then?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-flyout Flyout fixed-in-8.0.6 Look for this fix in 8.0.6 SR1! fixed-in-8.0.10 fixed-in-8.0.14 fixed-in-8.0.40 i/regression This issue described a confirmed regression on a currently supported version platform/android 🤖 t/bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants