You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When a Modal's container prop changes while the modal stays open, the modal is moved into the new container but the previous container keeps the aria-hidden state that was computed for the previous sibling set. For example, a modal mounted in document.body (default container) marks #root as aria-hidden; switching container to a node inside #root leaves #root hidden, making the dialog inaccessible to assistive technologies (getByRole('dialog') fails).
Changes
useModal: track the container the modal is registered with (registeredContainerRef) and re-register the modal with the ModalManager when the resolved container changes while open.
ModalManager: when removing the last modal of a container, restore siblings using the container itself as the blacklist entry instead of modal.mount. The mount node is already updated to the new container by the portal before the manager is notified, which previously kept the new container in the blacklist and left it aria-hidden.
Tests
ModalManager.test.ts: unit test covering the re-registration flow to a nested container.
Modal.test.js: regression test that changes the container prop after mount and asserts the previous container loses aria-hidden.
Hi @ZeeshanTamboli, could you take a look when you get a chance? This fixes #48882 (stale aria-hidden when a Modal's container changes). All checks are passing, including a regression test that swaps the container after mount. A maintainer label on the PR would also let the remaining label check go green.
Hi @ZeeshanTamboli@silviuaavram, quick nudge on this one. The fix is ready, all CI checks pass, and I added a regression test for the stale aria-hidden case. The test-label-applied check needs a maintainer to apply the label before this can merge. Would appreciate a review when you have a moment. Thanks!
@ZeeshanTamboli this seems to be in the same area as your current transition fix, that also checks whether the portal container changes. Can you sync the work and make sure there's no duplicated effort?
@ZeeshanTamboli this seems to be in the same area as your current transition fix, that also checks whether the portal container changes. Can you sync the work and make sure there's no duplicated effort?
@silviuaavram This is a bit different than the fix in the transition PR (#48881). I will review this PR later after the transition one is accepted and merged, incase if there is any duplication.
Hi @ZeeshanTamboli, I rebased this branch on the latest master and resolved the conflicts that came up after your transition fix (#48881) landed. Both tests are kept: yours for the exit transition and mine for the stale aria-hidden case, they cover different scenarios. useModal.ts rebased cleanly with no duplicated logic. The PR is mergeable again and ready for review when you have a moment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
PR: out-of-date
Fixes #48882
Root cause
When a
Modal'scontainerprop changes while the modal stays open, the modal is moved into the new container but the previous container keeps thearia-hiddenstate that was computed for the previous sibling set. For example, a modal mounted indocument.body(default container) marks#rootasaria-hidden; switchingcontainerto a node inside#rootleaves#roothidden, making the dialog inaccessible to assistive technologies (getByRole('dialog')fails).Changes
useModal: track the container the modal is registered with (registeredContainerRef) and re-register the modal with theModalManagerwhen the resolved container changes while open.ModalManager: when removing the last modal of a container, restore siblings using the container itself as the blacklist entry instead ofmodal.mount. The mount node is already updated to the new container by the portal before the manager is notified, which previously kept the new container in the blacklist and left itaria-hidden.Tests
ModalManager.test.ts: unit test covering the re-registration flow to a nested container.Modal.test.js: regression test that changes thecontainerprop after mount and asserts the previous container losesaria-hidden.