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

Option to remove aria-hidden from output #58

Closed
zachleat opened this issue Aug 28, 2019 · 5 comments
Closed

Option to remove aria-hidden from output #58

zachleat opened this issue Aug 28, 2019 · 5 comments
Assignees

Comments

@zachleat
Copy link

I believe that adding aria-hidden is incorrect for these links. These links should be accessible and would be useful to screen reader users too!

Notably this throws an error when using the axe-core accessibility linting tool.

Violation of "aria-hidden-focus" with 6 occurrences!
Ensures aria-hidden elements do not contain focusable elements. Correct invalid elements at:

The options to fix here are to remove aria-hidden (thus, this issue) or to set tabindex="-1" on the link, which wouldn’t be right either. I want these links to be focusable.

Thank you!

zachleat added a commit to 11ty/11ty-website that referenced this issue Aug 29, 2019
@nagaozen nagaozen self-assigned this Oct 15, 2019
@nagaozen
Copy link
Collaborator

Hi @zachleat, thank you for pointing this out!

Initially I thought we made the right call on this because, as you said:

"... elements do not contain focusable elements ..."

So, I went to the to source looking for the aria-hidden on the header tag but instead found it on the a tag. Well, that tag does not contain focusable elements, it is the focusable element! And because of https://webaccessibility.com "Avoid placing inactive elements in the focus order" Best Practice that states:

Elements on the page that are not interactive (those that do not trigger an action) such as labels, headings, etc. should not be in the keyboard focus order. Providing focus to non-active elements may give users of assistive technology the impression that the element is interactive and cause keyboard users to have to use extra keystrokes to navigate. In some specific situations (such as dire warnings or disabled controls that require changes for them to become enabled) it may be acceptable and necessary to place a limited number of non-interactive elements in the focus order. Setting focus programmatically to certain content that is not interactive such as error messages may also be acceptable.

It made sense that for a better UX it was the right call to remove that inactive element from the focus order...

But after further research I found out that MDN clearly says:

According to the fourth rule of ARIA, aria-hidden="true" should not be used on a focusable element. Additionally, since this attribute is inherited by an element's children, it should not be added onto the parent or ancestor of a focusable element.

And an HTMLAnchorElement with a href is, hands down, considered to be a focusable element.

So I think this should be changed.

@nagaozen
Copy link
Collaborator

Any additional words on this topic @valeriangalliat?

@nagaozen
Copy link
Collaborator

While we are still discussing this, what about changing it to: role="presentation"?

@nagaozen nagaozen reopened this Oct 15, 2019
@nagaozen
Copy link
Collaborator

Fixed on 5.2.5

@binyamin
Copy link

binyamin commented Oct 26, 2020

@zachleat @nagaozen I would like to revisit this point. How will screen-readers know to ignore the permalinkSymbol? Also, the devtools tell me that GitHub puts aria-hidden="true" on its markdown permalink thingies.

markdown permalink symbols

I like #58 (comment), though I think it's role="none" currently. Edit: axe cli doesn't like that suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants