Skip to content

[web] Workaround for removing aria-hidden leftovers#580

Merged
dgdavid merged 5 commits intomasterfrom
popup-siblings-workaround
May 18, 2023
Merged

[web] Workaround for removing aria-hidden leftovers#580
dgdavid merged 5 commits intomasterfrom
popup-siblings-workaround

Conversation

@dgdavid
Copy link
Contributor

@dgdavid dgdavid commented May 17, 2023

Problem

When an isOpen={true} PF4/Modal is unmounted instead of set as isOpen={false}, its siblings remain as aria-hidden. To know more, read following links

We have been told by the Patternfly team that the patternfly-react repo is code frozen and the fix will be reviewed for the next release, in Q3 at the earliest.

Solution

To use a workaround meanwhile: removing the aria-hidden attributes from the document.body children when the Agama/Popup will be unmounted.

Why not using a ref to ask for its siblings instead, as we did in #572 for the similar behavior of the Sidebar? Because PF4/Modal does not forward the ref. Read https://react.dev/learn/manipulating-the-dom-with-refs#accessing-another-components-dom-nodes

There are other tricky ways to workaround the issue (such as looking for the element with backdrop open CSS class 😵 😶), but since Agama is not using the appendTo PF4/Modal prop it is enough removing the attribute from the default target children.

Testing

  • Added a new unit test
  • Tested manually

Screenshots

N/A

dgdavid added 2 commits May 17, 2023 16:47
When a PF4/Modal is unmounted instead of set as `isOpen={false}`, its
siblings remain as `aria-hidden`. To know more read
patternfly/patternfly-react#9096
@dgdavid dgdavid force-pushed the popup-siblings-workaround branch from 09a942f to 8ca1cbc Compare May 17, 2023 15:47
@github-actions
Copy link

Pull Request Test Coverage Report for Build 5005255927

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 74.706%

Totals Coverage Status
Change from base Build 4993079353: 0.01%
Covered Lines: 5056
Relevant Lines: 6529

💛 - Coveralls

@dgdavid dgdavid marked this pull request as ready for review May 17, 2023 16:03
Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

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

LGTM

@dgdavid dgdavid merged commit e2fff39 into master May 18, 2023
@dgdavid dgdavid deleted the popup-siblings-workaround branch May 18, 2023 14:44
dgdavid added a commit that referenced this pull request Oct 19, 2023
dgdavid added a commit that referenced this pull request Oct 19, 2023
dgdavid added a commit that referenced this pull request Oct 19, 2023
[web] Remove Popup#aria-hidden patch added in #580
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.

2 participants