Skip to content

fix(modal): allow interaction with parent content through sheet modals in child routes #30839

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.

Sign up for GitHub

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

Jump to bottom
Open
ShaneK wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix(modal): allow interaction with parent content through sheet modals in child routes #30839

ShaneK wants to merge 7 commits into from

Conversation

@ShaneK
Copy link
Member

@ShaneK ShaneK commented Dec 5, 2025 •
edited
Loading

Issue number: resolves #30700

What is the current behavior?

When a sheet modal with showBackdrop=false is rendered in a child route (nested ion-router-outlet), the parent content becomes non-interactive. Clicks on buttons or other interactive elements in the parent component are blocked, even though showBackdrop=false should allow background interaction.

Two separate issues contributed to this bug:

  1. Root locking with backdropBreakpoint: The shouldLockRoot logic in overlays.ts didn't account for backdropBreakpoint. Modals with backdropBreakpoint > 0 were still locking the root with aria-hidden, even though developers expect background interaction when the modal is below the backdrop breakpoint.
  2. Child route wrapper blocking: When a modal is in a child route, the child route's page wrapper (ion-page) and its parent ion-router-outlet remain in the DOM with position: absolute covering the viewport. Even after the modal is moved to ion-app and has pointer-events: none, these wrapper elements block clicks to the parent page's content.

This issue stems from #30563, which added root-locking behavior that didn't account for modals that allow background interaction. A partial fix in #30689 partially addressed showBackdrop=false and focusTrap=false, but missed backdropBreakpoint.

What is the new behavior?

Sheet modals with showBackdrop=false or focusTrap=false now correctly allow interaction with parent content when the modal is in a child route.
Improvements:

  • Recalculates isSheetModal in present() to handle Angular binding timing
  • Sets pointer-events: none on the modal element and its original parent elements when background interaction should be allowed
  • Cleans up pointer-events on dismiss
  • Adds regression tests

Does this introduce a breaking change?

  • Yes
  • No

Other information

Dev build:

8.7.12-dev.11765060985.14ad27fb

@ShaneK
chore(angular): reproducing issue where modals with showBackdrop=fals…
ea8a22d 
…e aren't allowing clicks through the background
@ShaneK ShaneK requested a review from a team as a code owner December 5, 2025 02:49
@ShaneK ShaneK requested a review from BenOsodrac December 5, 2025 02:49
@vercel
Copy link

vercel bot commented Dec 5, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
ionic-framework Ready Ready Preview Comment Dec 8, 2025 11:08pm

@github-actions github-actions bot added package: core @ionic/core package package: angular @ionic/angular package labels Dec 5, 2025
@ShaneK ShaneK removed the request for review from BenOsodrac December 5, 2025 02:50
@ShaneK ShaneK mentioned this pull request Dec 5, 2025
Closed
2 tasks
@github-actions github-actions bot added the package: react @ionic/react package label Dec 5, 2025
brandyscarney
brandyscarney reviewed Dec 5, 2025
View reviewed changes
@ShaneK ShaneK marked this pull request as draft December 5, 2025 18:48
@ShaneK ShaneK marked this pull request as ready for review December 5, 2025 19:19
@ShaneK ShaneK changed the title fix(modal): allow interaction with parent content when sheet modal has showBackdrop=false in child routes fix(modal): allow interaction with parent content through sheet modals in child routes Dec 5, 2025
@ShaneK ShaneK requested a review from thetaPC December 7, 2025 02:56
thetaPC
thetaPC requested changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

The main thing is that I couldn't get the Angular page to interact for me. Please let me know if I just missed something.

core/src/components/modal/modal.tsx Outdated
* passthrough on child route page wrappers and nested router outlets.
*/
private setupChildRoutePassthrough() {
const pageParent = this.getOriginalPageParent();
Copy link
Contributor

@thetaPC thetaPC Dec 8, 2025

Choose a reason for hiding this comment

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

I noticed that getOriginalPageParent() gets called twice, here and cleanupChildRoutePassthrough(). Is it possible to save the pageParent as a private variable instead? So we don't have to run the while loop more than once?

Copy link
Member Author

@ShaneK ShaneK Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Done!

packages/angular/test/base/e2e/src/standalone/modal-child-route.spec.ts
Comment on lines +21 to +26
test('should allow interacting with parent content while modal is open in child route', async ({ page }) => {
await expect(page.locator('ion-modal.show-modal')).toBeVisible();

await page.locator('#increment-btn').click();
await expect(page.locator('#background-action-count')).toHaveText('1');
});
Copy link
Contributor

@thetaPC thetaPC Dec 8, 2025

Choose a reason for hiding this comment

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

I was testing this scenario manually and I'm not able to interact with the background. This is happening on Firefox, Chrome, and Safari.

Screen.Recording.2025-12-08.at.11.49.19.AM.mov

Copy link
Member Author

@ShaneK ShaneK Dec 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This works great for me in my testing, and worked for kumibrr here, so I'm unsure of what's going on 🤔

Maybe pull and try with my latest changes? That's real weird though!

Screenshot.2025-12-08.at.15.10.34.mp4

Copy link
Contributor

@thetaPC thetaPC Dec 9, 2025

Choose a reason for hiding this comment

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

I still had issues with the new code but I've approved it since the issue seems to be only on my end.

@ShaneK
fix(modal): cleaning up some repetitive code, adding links for test p…
6780fad 
…ages, caching page parent to prevent excessive searching
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandyscarney brandyscarney brandyscarney left review comments

@thetaPC thetaPC thetaPC approved these changes

Assignees

No one assigned

Labels

package: angular @ionic/angular package package: core @ionic/core package package: react @ionic/react package

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: sheet modals can't skip focus-traps on child routes

4 participants

@ShaneK @brandyscarney @thetaPC