fix(Modal): exclude Poppers in aria-hidden change#12424
Conversation
Popper based components like Select or Dropdown get mounted to the DOM on the same level as Modal. Modal then on update can set aria-hidden label to true via function toggleSiblingsFromScreenReaders, which adds the label to all the elements that are siblings of the Modal, including these components.This causes the opened Popper based component to become invisible for screen readers and/or Playwright. Fix this by excluding all the Popper components from this function's behaviour.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughModal's ChangesModal aria-hidden Popper exclusion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Preview: https://pf-react-pr-12424.surge.sh A11y report: https://pf-react-pr-12424-a11y.surge.sh |
|
Hey @tkoscieln , this looks good for resolving the issue with Poppers specifically. We have a PR in our component groups repo that is looking to introduce tearsheets, basically stacked Modals. When modals gets stackeds, they end up having the problem noted in the linked issue to your PR (all open Modals get aria-hidden of true, even the "active" one). I was wondering if you would want to try expanding this PR to take into account additional Modals as well? Likely the logic should be that only the "active"/most recently opened Modal shouldn't have aria hidden applied, all other Modals behind the active one/that are inactive should be aria-hidden true. |
What: Closes #12422
Popper based components like Select or Dropdown get mounted to the DOM on the same level as Modal. Modal then on update can set aria-hidden label to true via function toggleSiblingsFromScreenReaders, which adds the label to all the elements that are siblings of the Modal, including these components.This causes the opened Popper based component to become invisible for screen readers and/or Playwright.
Fix this by excluding all the Popper components from this function's behaviour.
Additional issues:
N/A
Summary by CodeRabbit