Skip to content
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.

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

It remove aria-hidden attribute for hidden element on dialog closing #97

Closed
ryuran opened this issue Jul 30, 2018 · 10 comments
Closed

It remove aria-hidden attribute for hidden element on dialog closing #97

ryuran opened this issue Jul 30, 2018 · 10 comments

Comments

@ryuran
Copy link

ryuran commented Jul 30, 2018

When we open a dialog, it put aria-hidden attribute to true for each element of document other than dialog itself.

And when we close the dialog box, aria-hidden attribute is removed to each of them.

But when an element is aria-hidden="true" yet before opening, a11y-dialog remove the attribute at closing, but it shouldn't.

@KittyGiraudel
Copy link
Owner

Fair point.

@KittyGiraudel
Copy link
Owner

The lib did support this for a while and eventually I removed it as I thought it was some unnecessary optimisation.

@kmmathis
Copy link

kmmathis commented Oct 9, 2018

Can confirm this is an issue, I reverted to an older version of the script for the time being

@ryuran
Copy link
Author

ryuran commented Oct 10, 2018

In my case it was not a problem because nothing was aria-hidden at the same DOM level than the dialog box.
But in can be dangerous if an other dialog, tooltip or other have a aria-hidden block at same level.
It will be shown when dialog box is closing, but it shouldn’t.

@KittyGiraudel
Copy link
Owner

That’s absolutely true. If someone would like to submit a PR, I’d be glad to review and merge it. :)

@bitboxer
Copy link

Sadly I have 3 dialog boxes on the same page. My workaround was to add surrounding divs to the dialog html

@michaelhugney
Copy link

This issue was causing unwanted behavior when I had multiple dialogs on the same page, in all browsers except Chrome. When hiding one dialog, the others were all having their 'aria-hidden' attributes removed and were therefore incorrectly being displayed.

I was able to fix this issue by updating a11y-dialog.js line 182 and adding a check so that the hide() method only removed the aria-hidden attribute from non-dialog elements.

this._targets.forEach(function(target) { if (target.className !== 'dialog') target.removeAttribute('aria-hidden');});

@kuuak
Copy link

kuuak commented Jun 17, 2020

Still facing this issue almost 2 years after it has been reported.

If I find some time I'll try to propose a fix and Pull Request

@KittyGiraudel
Copy link
Owner

Hello. I’m sorry you’re still facing that issue. That being said, the fix is relatively straightforward and can be done without touching the library at all: have two containers side by side, one for the content, one for the dialog.

<body>
  <div id="root">
    <!-- Main app content here -->
  </div>
  <div id="dialog" aria-hidden="true">
    <!-- Dialog here -->
  </div>
</body>

Wouldn’t that be possible?

@patrickhlauke
Copy link
Contributor

this issue should be fixed by the latest change #117

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

No branches or pull requests

7 participants