-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix for Page OnAppearing triggered twice when navigating via ShellItem change with PresentationMode set to Modal #32582
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
Fix for Page OnAppearing triggered twice when navigating via ShellItem change with PresentationMode set to Modal #32582
Conversation
|
Hey there @@SyedAbdulAzeemSF4852! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
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.
Pull Request Overview
This PR fixes an issue where OnAppearing is called twice on modal pages when navigating between different ShellItems in .NET MAUI Shell. The fix adds a check to prevent non-visible ShellSections from sending lifecycle events to pages they no longer own.
Key changes:
- Modified
PresentedPageDisappearing()inShellSection.csto checkIsVisibleSectionbefore sending disappearing events to modal pages - Added UI test case to verify the fix prevents duplicate
OnAppearingcalls during ShellItem transitions with modal pages
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Controls/src/Core/Shell/ShellSection.cs |
Added IsVisibleSection check in PresentedPageDisappearing() to prevent duplicate lifecycle events on modal pages during ShellItem transitions |
src/Controls/tests/TestCases.HostApp/Issues/Issue31584.cs |
Added HostApp test page demonstrating the bug and verifying the fix with modal navigation between ShellItems |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue31584.cs |
Added NUnit UI test to verify modal page OnAppearing is triggered only once during ShellItem navigation |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
4e5c336 to
fcb97b0
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
PureWeen
left a comment
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.
Can you rework this to just be a unit test vs a UItest?
I think we can validate this purely by being a UITest?
|
Conversation_Summary_PR_32582.md @SyedAbdulAzeemSF4852 @sheiksyedm I had copilot add unit tests based on your UITest I asked it this
and then it generated these tests Please review added tests and delete UITests if they look good |
@PureWeen , I’ve gone through the tests Copilot added. Two of the tests seemed quite similar and were essentially testing the same thing, so I’ve removed one of them. I’ve also removed the original UI test since the remaining unit tests provide the necessary coverage. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
…m change with PresentationMode set to Modal
…d duplicate tests in the unit tests
2e00eb3 to
9e70228
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
3143fac to
bcafb40
Compare
…m change with PresentationMode set to Modal (#32582) * Fix for Page OnAppearing triggered twice when navigating via ShellItem change with PresentationMode set to Modal * - generate tests with copilot * Removed the UI test since unit tests have been added, and also removed duplicate tests in the unit tests --------- Co-authored-by: Shane Neuville <[email protected]>
…m change with PresentationMode set to Modal (#32582) * Fix for Page OnAppearing triggered twice when navigating via ShellItem change with PresentationMode set to Modal * - generate tests with copilot * Removed the UI test since unit tests have been added, and also removed duplicate tests in the unit tests --------- Co-authored-by: Shane Neuville <[email protected]>
…m change with PresentationMode set to Modal (#32582) * Fix for Page OnAppearing triggered twice when navigating via ShellItem change with PresentationMode set to Modal * - generate tests with copilot * Removed the UI test since unit tests have been added, and also removed duplicate tests in the unit tests --------- Co-authored-by: Shane Neuville <[email protected]>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details
Root Cause
Description of Change
Issues Fixed
Fixes #31584
Validated the behaviour in the following platforms
Output
iOS_Before.mov
iOS_After.mov
Android_Before.mov
Android_After.mov