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

Introduce sections for Accessibility tree and aria-hidden #1045

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Jun 20, 2019

Fixes #258


Preview | Diff

aria-hidden.md Outdated Show resolved Hide resolved
aria-hidden.md Outdated Show resolved Hide resolved
@zcorpan
Copy link
Member Author

zcorpan commented Aug 30, 2019

Thank you for the review @a11ydoer! I've addressed the comments and moved the text into aria-practices.html, and rebased on latest master. If this looks OK, I believe this is ready to merge.

aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
<p>Using <code>aria-hidden</code> with the value <code>true</code> excludes the element and its descendants from the accessibility tree, just like hidden elements, but such elements are <em>not</em> hidden visually. The descendants are excluded from the accessibility tree even if an element in the excluded subtree sets <code>aria-hidden</code> to <code>false</code>.</p>
<p><code>aria-hidden</code> can be used to hide decorative or redundant content from screen readers, for example an inline SVG icon.</p>
<pre><code>&lt;a href=&quot;/&quot;&gt;
&lt;svg aria-hidden=&quot;true&quot;&gt;...&lt;/svg&gt;
Copy link
Contributor

Choose a reason for hiding this comment

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

@mcking65 Do we want to add focusable="false" to this SVG example code to remind authors that if they use aria-hidden="true" on an SVG, they will want to force the SVG to not take focus in IE11?

Copy link
Member Author

Choose a reason for hiding this comment

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

That seems like a bug in IE11. If I understand correctly, APG takes the stance to generally not work around bugs in implementation. I'm not sure if that is only implementation of ARIA or in general, though?

Copy link
Contributor

Choose a reason for hiding this comment

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

Completely agree that APG should not work around browser bugs, however since IE11 is never going to change, perhaps this bug needs to be worked around permanently? If you do decide to add focusable="false", it deserves a decent comment in the code to explain why it's there.

@zcorpan
Copy link
Member Author

zcorpan commented Sep 10, 2019

Thanks for the review! I've addressed comments, except I didn't add focusable="false" #1045 (comment)

@JAWS-test
Copy link

JAWS-test commented Sep 10, 2019

That seems like a bug in IE11.

This is a bug of IE 11 and older versions of Edge. I think APG are not responsible for browser bugs. But where a bug is well known and there is a simple solution to avoid it, it would make sense to offer this solution, especially if a bug has such serious consequences as an SVG that is hidden and gets the focus. In addition to unnecessary and invisible navigation steps with the Tab key for keyboard users, this means that the screen reader outputs the role and label of the previous element.

@zcorpan
Copy link
Member Author

zcorpan commented Sep 11, 2019

Thanks @JAWS-test and @carmacleod. I think it is a reasonable balance to include and document simple workarounds for unmaintained and widely used implementations, while in general assume conforming implementations.

@zcorpan
Copy link
Member Author

zcorpan commented Nov 15, 2019

@a11ydoer or @carmacleod, can either of you review this? Thanks!

aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
@carmacleod
Copy link
Contributor

Consider making section 9. Excluding subtrees from the accessibility tree
and section 10. Intentionally Hiding Semantics with the presentation Role
be subsections of section 8. Accessibility tree.

@carmacleod
Copy link
Contributor

Consider adding something to the effect of "don't put aria-hidden on interactive content". to the warning box at the end of section 9. Excluding subtrees from the accessibility tree.
(This may be off-topic for this PR - if so, I can open a new issue).

@zcorpan
Copy link
Member Author

zcorpan commented Dec 6, 2019

Consider making section 9. Excluding subtrees from the accessibility tree
and section 10. Intentionally Hiding Semantics with the presentation Role
be subsections of section 8. Accessibility tree.

Done.

@zcorpan
Copy link
Member Author

zcorpan commented Dec 6, 2019

Consider adding something to the effect of "don't put aria-hidden on interactive content". to the warning box at the end of section 9. Excluding subtrees from the accessibility tree.

Done.

@zcorpan
Copy link
Member Author

zcorpan commented Jan 9, 2020

@carmacleod can you review the latest changes? Thanks!

@carmacleod carmacleod mentioned this pull request Jan 23, 2020
@zcorpan zcorpan removed their assignment Feb 19, 2021
Base automatically changed from master to main March 3, 2021 18:13
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.

Draft guidance on aria-hidden
9 participants