Skip to content

Conversation

@pictos
Copy link
Member

@pictos pictos commented Oct 28, 2025

Description of Change

Removed the NavigationBar and make sure the Close will work. When you open a Popup inside a ModalPage the Popup will not show on ModalStack, what shows up is a NavigationPage and the PopupPage will be its Content.

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls

Additional information

Copilot AI review requested due to automatic review settings October 28, 2025 03:14
Copy link
Contributor

Copilot AI left a 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 addresses an issue where popups opened inside modal pages fail to display correctly due to navigation bar interference. The fix removes the navigation bar from popup pages and implements proper modal stack traversal to locate and close popups that are wrapped in navigation containers.

Key Changes:

  • Explicitly disables the navigation bar for popup pages to prevent UI conflicts
  • Adds a FindPopupPage() method to handle closing popups that are nested within page containers (e.g., NavigationPage)
  • Removes unused imports to clean up the code

@pictos pictos force-pushed the pj/fix-modal-on-popups branch from f69560f to d70a248 Compare October 31, 2025 03:15
{
throw new PopupNotFoundException();
await FindPopupPage();
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we call PopupClosed event here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes! My bad

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 Pedro! Could you add some UnitTests before we merge this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Popup V2 within NavigationPage

3 participants