Skip to content

Cleanup Popup Resources When Closed#2971

Merged
TheCodeTraveler merged 11 commits intoCommunityToolkit:mainfrom
AlejoTorresLeon:fix/2962-popup-android-accessibility
Nov 28, 2025
Merged

Cleanup Popup Resources When Closed#2971
TheCodeTraveler merged 11 commits intoCommunityToolkit:mainfrom
AlejoTorresLeon:fix/2962-popup-android-accessibility

Conversation

@AlejoTorresLeon
Copy link
Contributor

@AlejoTorresLeon AlejoTorresLeon commented Nov 28, 2025

The OnNavigatedFrom method in PopupPage.shared.cs has been updated to verify that the popup is actually closing before performing cleanup operations. The appropriate modal stack (ModalStack) is now used depending on whether Shell is present. This avoids unnecessarily cleaning up GestureRecognizers and unsubscribing from the PropertyChanged event, improving stability and preventing unexpected behavior.

What this PR solves

Fixes an issue where Popup A cannot be dismissed by tapping outside when Popup B was opened and closed on top of it.

Root Cause

After Popup B closed, Popup A was not receiving inside/outside touch events correctly.

Solution

When Popup is shown via PopupExtensions, we ensure PopupPage receives the correct gesture handling and event routing.

Testing

  • Popup A opens → tap outside → closes ✔
  • Popup A opens → Popup B opens → Popup B closes → tap outside → Popup A closes ✔
  • Tested on Android 14 (emulator + device)

Linked Issues

Fixes #2962

The `OnNavigatedFrom` method in `PopupPage.shared.cs` has been updated to verify that the popup is actually closing before performing cleanup operations. The appropriate modal stack (`ModalStack`) is now used depending on whether `Shell` is present. This avoids unnecessarily cleaning up `GestureRecognizers` and unsubscribing from the `PropertyChanged` event, improving stability and preventing unexpected behavior.
@AlejoTorresLeon
Copy link
Contributor Author

@dotnet-policy-service agree

Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AlejoTorresLeon! Good catch that OnNavigatedFrom also fires when a second Popup is overlayed onto a visible Popup 🙌

Just FYI - I moved the cleanup of Popup resources out of PopupPage.OnNavigatedFrom() and into PopupPage.CloseAsync() to make things a bit cleaner and to avoid the modalStack.Contains(this) improving performance a bit.

@TheCodeTraveler TheCodeTraveler added the approved This Proposal has been approved and is ready to be added to the Toolkit label Nov 28, 2025
@AlejoTorresLeon
Copy link
Contributor Author

@TheCodeTraveler Excellent observation, thank you very much.

@TheCodeTraveler TheCodeTraveler changed the title Fixing #2962 Conditional Cleanup on Popup Closing Cleanup Popup Resources When Closed Nov 28, 2025
@TheCodeTraveler TheCodeTraveler merged commit 3ba59e6 into CommunityToolkit:main Nov 28, 2025
10 checks passed
@AlejoTorresLeon AlejoTorresLeon deleted the fix/2962-popup-android-accessibility branch November 28, 2025 21:56
@github-actions github-actions bot locked and limited conversation to collaborators Nov 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved This Proposal has been approved and is ready to be added to the Toolkit

Projects

None yet

2 participants