[Android] Fix Flickering issue when calling Navigation.PopAsync#24887
[Android] Fix Flickering issue when calling Navigation.PopAsync#24887PureWeen merged 19 commits intodotnet:mainfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
16c0e0d to
22d7398
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Azure Pipelines successfully started running 3 pipeline(s). |
This is the failing test NavigatedFiresAfterSwitchingFlyoutItems. |
|
will this ptach be merged into dotnet 8 maui? |
|
@ygl-rg I think at this point with .NET 9 being close to being released this will be .NET 9 only |
|
/rebase |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Changes Applied
| InternalChildren.Remove(page); | ||
|
|
||
| // TODO For NET9 we should remove this because the DisconnectHandlers will take care of it | ||
| page.Handler = null; |
There was a problem hiding this comment.
Hi! @devanathan-vaithiyanathan can you please fix it? It is an annoying bug that would be great to have fixed as soon as possible. I can help you if you want :)
There was a problem hiding this comment.
@PureWeen I have updated the code to include null checks for MauiContext to address the WinUI device test crashes. Could you please review the changes and let me know if you have any further concerns?
…sLayout is set for Tablet but not on mobile devices (dotnet#26152)" This reverts commit 0ddc794.
…msLayout is set for Tablet but not on mobile devices (dotnet#26152)" This reverts commit 4b9074c.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| bool _connectedToHandler; | ||
| WFrame NavigationFrame => _navigationFrame ?? throw new ArgumentNullException(nameof(NavigationFrame)); | ||
| IMauiContext MauiContext => this.Handler?.MauiContext ?? throw new InvalidOperationException("MauiContext cannot be null here"); | ||
| IMauiContext? MauiContext => this.Handler?.MauiContext; |
There was a problem hiding this comment.
I think we want to leave the exception
Can you just add a method to check nullability?
bool HasHandler => this.Handler?.MauiContext is not null
There was a problem hiding this comment.
@PureWeen MauiContext reference is used in multiple places in this file, and I have ensured that null checks are added to prevent exceptions. Do we need to assign this.Handler?.MauiContext based on HasHandler in each place?
There was a problem hiding this comment.
There was a problem hiding this comment.
@PureWeen Upon checking further, the SelectionChanged event was unhooked properly during navigation, but the _navigationFrame.Navigated event was not. Since I am unable to reproduce the exact crash, based on the call stack from the dump files, I suspect that the issue occurs because the Navigated event was not unhooked properly. I have now unhooked the Navigated event correctly. If the Navigated event is properly unhooked during the disconnect handling, it will prevent the UpdateCurrentPageContent method from being called, even when triggered from the selection changing event. Could you please review and share your concerns?
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |





Root Cause
The flickering during navigation occurred because, when performing PopAsync, the removed page’s handlers were set to null. This caused flickering as the navigation was processed.
Description of Change
The fix involves avoiding the removal of page handlers during PopAsync, which eliminates the flickering issue during navigation.
Issues Fixed
Fixes #13810
Validated the behaviour in the following platforms
Output Screenshot
Before
366413807-f0592d6d-6b89-48b0-bab8-eed46d20336c.mp4
After
366413828-b47e24f4-68f0-44a0-b1bf-ed8dd24d1dcf.mp4