doc - Fix some accessibility issues#28322
Conversation
TODO: on dashboard the + icon for save-reports generate an empty link
```
<span>Saved reports</span>
<a class="d-flex align-items-center text-muted" href="#">
<span data-feather="plus-circle"></span>
</a>
```
patrickhlauke
left a comment
There was a problem hiding this comment.
- conflicted about the use of
<nav> - change of the
<h6>is (from memory) not a good idea as it will make it show up in the TOC (currently, until we switch to a different site generator solution) - that one heading level change in the markdown - yup, that's right
- shorter/simpler to add
aria-labelto links around those<svg>s ?
| <div class="col-4 d-flex justify-content-end align-items-center"> | ||
| <a class="text-muted" href="#"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" class="mx-3" role="img" viewBox="0 0 24 24" focusable="false"><title>Search</title><circle cx="10.5" cy="10.5" r="7.5"/><path d="M21 21l-5.2-5.2"/></svg> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" class="mx-3" role="img" viewBox="0 0 24 24" focusable="false" aria-labelledby="svg-search-title"><title id="svg-search-title">Search</title><circle cx="10.5" cy="10.5" r="7.5"/><path d="M21 21l-5.2-5.2"/></svg> |
There was a problem hiding this comment.
advantage of this over just adding an aria-label="..." on the link?
| <div class="container d-flex flex-column flex-md-row justify-content-between"> | ||
| <a class="py-2" href="#"> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" class="d-block mx-auto" role="img" viewBox="0 0 24 24" focusable="false"><title>Product</title><circle cx="12" cy="12" r="10"/><path d="M14.31 8l5.74 9.94M9.69 8h11.48M7.38 12l5.74-9.94M9.69 16L3.95 6.06M14.31 16H2.83m13.79-4l-5.74 9.94"/></svg> | ||
| <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" fill="none" stroke="currentColor" stroke-linecap="round" stroke-linejoin="round" stroke-width="2" class="d-block mx-auto" role="img" viewBox="0 0 24 24" focusable="false" aria-labelledby="svg-product-title"><title id="svg-product-title">Product</title><circle cx="12" cy="12" r="10"/><path d="M14.31 8l5.74 9.94M9.69 8h11.48M7.38 12l5.74-9.94M9.69 16L3.95 6.06M14.31 16H2.83m13.79-4l-5.74 9.94"/></svg> |
There was a problem hiding this comment.
advantage of this over just adding an aria-label="..." on the link?
There was a problem hiding this comment.
I've not tested with aria-label. I will made a codepen.
I've propose that solution based on https://developer.paciellogroup.com/blog/2013/12/using-aria-enhance-svg-accessibility/ but that doesn't discuss about svg used in a link
There was a problem hiding this comment.
First tests give the advantage to aria-label on the link that prevent to vocalize the type of the content, but impose to duplicate the title (no impact in our case)
test NVDA 2016.2.1 / chrome 72 : test NVDA 2016.2.1 / firefox ESR 52.7.2 : test Jaws 17 / IE11 : Same result
labelledby on the SVG say : Search graphique lien (in french)
aria-label on the link say : Search lien (in french)
If you've more feedback with voiceover/talkback/narrator let me know
There was a problem hiding this comment.
anyone had made some more test?
What solution have your favour @patrickhlauke ? labelledby or aria-label? Let me know if I need to update the PR
|
Please do not change any headers. I'll tackle this later myself. |
|
But this will need to be adapted after the Hugo switch, and I'll probably need to do it from scratch then. |
|
I could remove the Hx update from my PR to prevent conflict during hugo migration. And I will update the PR with aria-label on nav elements and make some tests about aria-label on link versus aria-labelledby on svg element |
This reverts commit 224c762.
Lausselloic
left a comment
There was a problem hiding this comment.
I've update my PR wonder it's better now?
|
Nope, there are conflicts. You need to rebase properly. |
|
A merge is not a rebase FYI. |
|
Yes I know that a merge isn't a rebase, but it works like a charm no? |
|
I've split that PR into 2 smallest one, with clean branch |
|
No it doesn't work, because it doesn't allow us to rebase and merge. We can do squash and merge or a merge commit. The later we don't do anymore. |
This PR goal is to fix some accessibility issues in actual documentation :
navlandmark in documentation pageslet @patrickhlauke check it