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

Fix violation of Axe rule link-name #1633

Merged
merged 6 commits into from Mar 20, 2024
Merged

Conversation

gabalafou
Copy link
Collaborator

@gabalafou gabalafou commented Jan 6, 2024

This PR is part of a set of fixes for issue #1428.

If you don't provide link-alt to Sphinx Design clickable cards, then:

  • The link may not appear in the list of links provided by assistive tech
  • It may not be presented by the assistive tech as a link to the user (in other words, a screen reader user may have to guess that the card is clickable)
  • Results in an Axe error: Links must have discernible text

link-alt was added to Sphinx Design specifically to address accessibility checkers.

In my opinion, it's not a perfect solution. For example, on our site, I had to duplicate the card text in link-alt because I didn't think any other value made sense. However, that means that not only is that text duplicated in the source code, it also ends up being duplicated for a screen reader too.

The cards probably need to be given an upstream re-think, but as this CSS Tricks article shows, getting clickable cards right isn't easy, so this solution will have to do for now.

@gabalafou
Copy link
Collaborator Author

gabalafou commented Jan 6, 2024

To-do?

  • Update docs to encourage authors to always provide link-alt when using clickable Sphinx Design cards
  • Also update Sphinx Design docs to highlight the important of link-alt (right now it's just presented as a card option, but the option should be noted/highlighted in the section about clickable cards)

@trallard trallard added the tag: accessibility Issues related to accessibility issues or efforts label Jan 8, 2024
@trallard
Copy link
Collaborator

Thanks @gabalafou

The cards probably need to be given an upstream re-think, but as this CSS Tricks article shows, getting clickable cards right isn't easy, so this solution will have to do for now.

Agree, I am not sure the current approach is optimal and from executablebooks/sphinx-design#89 the text is only "hidden" using a quick CSS hack/trick but remains visible for screen readers.
We might want to look at some of the techniques in https://www.scottohara.me/blog/2017/04/14/inclusively-hidden.html and see if we could use any to make this better.

@@ -6,38 +6,50 @@
# This file should only be modified by maintainer, contributors should add their project
# to the list of `gallery.md`.
- title: ArviZ
link-alt: ArviZ
Copy link
Collaborator

Choose a reason for hiding this comment

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

My first reaction was to modify this to link-alt: Arviz home
per the recommendations in https://www.w3.org/WAI/tutorials/images/functional/#example-1-image-used-alone-as-a-linked-logo

But then I wondered if it should instead be link-alt: "" per https://www.w3.org/WAI/tutorials/images/functional/#logo-image-within-link-text
Ultimately the card title Arviz provides some of that information? Or is it because it does not indicate where the link takes the user per se in which case then we should adopt the link-alt: Arviz home

Copy link
Collaborator Author

@gabalafou gabalafou Feb 27, 2024

Choose a reason for hiding this comment

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

I think it might be helpful if I start by showing the HTML structure that Sphinx Design outputs for link cards. It looks basically like this:

div
  div yaml.title
  img yaml.img-bottom
  a href=yaml.link
    span yaml.link-alt

The way that Sphinx Design makes the whole card clickable is by absolutely positioning the last element (the <a> tag) in the (relatively positioned) card div. But that <a> tag has no content unless you specify link-alt, which means it doesn't get surfaced in a nice way to assistive tech if it gets surfaced at all.

So a screen reader user may end up just hearing a bunch of link card titles with no indication whatsoever that they represent links. They will have to guess from the context that these are links and then to verify that hunch by doing what is essentially the equivalent of clicking on completely unmarked text. (I can verify that this is the behavior I observed with VoiceOver.)

What this PR does is surface the links to the screen reader at the cost of repeating the same info twice (card title = link-alt), since I don't think it makes sense to make the card title text different from the link text.

As for putting "home" in "link-alt", I think this is nearly inconsequential either way but I generally shy away from putting "home" in alt text from fear of creating screen reader noise. What's kind of funny is that the W3C page you shared with me is out of date with itself. I mean, if you examine the page at that URL and look at the accessibility tree of the W3C logo in the top left, you'll see that it's just "W3C", which is generated by an svg with role="image" and aria-label="W3C".

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep I verified the HTML output when originally reviewing the PR.

What seems a bit odd is now having the duplicate Text + alt text such as Arviz + Arviz as the latter is meant to provide additional context around the link (and where users will be redirected to).

Since we already use the PyData Sphinx Theme - home pattern for our own icon (on the navbar) I'd prefer adding the home to these alt-text to at least provide more information regarding the hyperlink

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure. Happy to change it. :)

Just for accuracy, though, we don't do that with our own icon. (That's because our top-left link is logo + text, in which case we made the decision to set the logo alt text to empty, see #1472, so the accessible name for the entire logo link is "PyData Theme.")

Copy link
Collaborator

Choose a reason for hiding this comment

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

I forgot about that last PR. Before that we did have the home text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh wait. Actually, some of these links do not go to the home page, for example, Bokeh goes to docs.bokeh.org, and bokeh.org is a separate page from / does not redirect to docs.bokeh.org. (Fairlearn's link goes neither to docs nor to home but to their about page.) Should we go through each link and state whether the link goes to the home page or to the docs or some other page? Should we update the links to make sure they all take the user to the docs home page? (I don't think we should update the links because I assume those were chosen specifically to take the user to a page built with the PyData theme, whereas other pages on the same site might not be built with PyData theme.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should indicate where the link is directing the user/reader instead of changing them all to point to the project's Homepage per the specific reasons you mentioned before

Copy link
Collaborator Author

@gabalafou gabalafou Mar 20, 2024

Choose a reason for hiding this comment

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

I appended "docs" to the each of them, because I realized that's essentially where all of these links are going (and that makes sense, given that this is a theme for docs sites)

docs/user_guide/web-components.rst Outdated Show resolved Hide resolved
Co-authored-by: Tania Allard <taniar.allard@gmail.com>
@gabalafou
Copy link
Collaborator Author

I agree that the cards need to be refactored upstream but the description in Sphinx Design PR #89, about needing to override aspects of the Sphinx HTML builder, made me think this would be a deep rabbit hole.

Maybe in the short run we merge this PR, and in the long run we work with Sphinx Design to make a more screen reader-friendly link card?

The only reason I can think of not to merge this PR would be that it will make the Axe errors go away, which might reduce our sense of urgency around the problem.

@trallard
Copy link
Collaborator

Thanks @drammock, merging now 🚀

@trallard trallard merged commit 69f827d into pydata:main Mar 20, 2024
18 checks passed
@gabalafou
Copy link
Collaborator Author

To close the loop on this, I created a PR in the Sphinx Design repo to improve the docs and the default behavior:

If the PR is merged, it won't change how we've updated the cards in this PR, but it might be useful for other folks.

@gabalafou gabalafou deleted the fix-link-name branch April 18, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: accessibility Issues related to accessibility issues or efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants