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

Reconsider using aria-hidden instead of aria-modal #408

Closed
fymmot opened this issue Sep 7, 2022 · 6 comments
Closed

Reconsider using aria-hidden instead of aria-modal #408

fymmot opened this issue Sep 7, 2022 · 6 comments

Comments

@fymmot
Copy link

fymmot commented Sep 7, 2022

First of all, thank you for your great work with this project!

But I noticed that v7.0 dropped the use of aria-hidden in favor of aria-modal. Would you be open to reconsider that decision?

Aria-modal still has very limited support in Talkback and iOS VoiceOver, meaning that it is possible to access the background content when swiping through the modal. This can lead to pretty confusing experience for mobile screen reader users. See the following screenshot:

Modal displayed on iPhone with VoiceOver, with background content selected

Here is the a11ysupport page for the aria-modal technique: https://a11ysupport.io/tech/aria/aria-modal_attribute

@KittyGiraudel
Copy link
Owner

KittyGiraudel commented Sep 7, 2022

Hello Tommy, thank you for opening an issue.

That’s certainly a little inconvenient… The thing with the aria-hidden attribute on the main containers method is that it makes the library significantly more difficult to use, to the point where many users ended up implementing it incorrectly (myself included 🙀), despite the documentation. It’s easy not to build the DOM the right way and to have the library toggle the attribute on the wrong elements.

That being said, you can still use version 6. It’s fine, it works and it will rely on aria-hidden forever. The documentation is still up and running and the feature set remains essentially the same.

In the meantime, I can add this limitation to the known issues documentation page so it’s made clear. What do you think?

@chalkygames123
Copy link
Contributor

@fymmot FYI, you can easily address this issue by hooking a library like aria-hidden to the show and hide events.

@fymmot
Copy link
Author

fymmot commented Sep 8, 2022

@KittyGiraudel
I see, that is a real challenge..! I know there has been similar problems with the accessible react-modal library, where users of the library failed to set the root element correctly and instead ended up completely hiding the app for screen readers with without noticing. That's no good either.

But with that said, aria-modal really does cause serious usability problems for mobile screen reader users - I have seen it first hand in usability tests. So if you keep aria-modal I believe the documentation needs to be quite clear in describing these limitations. Perhaps you might even provide an example implementation using the aria-hidden library that @chalkygames123 mentioned, if it works out well?

@KittyGiraudel
Copy link
Owner

@fymmot Thank you for coming back to me and for sharing you experience with usability testing. At this stage, I would recommend authoring significant documentation about this limitation, as you suggested. I will also recommend the workaround from @chalkygames123, since I feel like it’s a great and rather un-intrusive solution to this problem.

@fymmot
Copy link
Author

fymmot commented Sep 8, 2022

@KittyGiraudel
I think that is a good compromise! Thank you for being so proactive :)

@KittyGiraudel
Copy link
Owner

Addressed in #409. I would love your eyes on this though. :)

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

3 participants