feat: add aria attributes for MiniToc #244
Conversation
|
Left some comments here
|
src/components/MiniToc/MiniToc.tsx
Outdated
| return ( | ||
| <div className={b()}> | ||
| <div className={b()} aria-label={t('article navigation')}> |
There was a problem hiding this comment.
label should be article-navigation and it need to be aded here and in "ru"
https://github.com/diplodoc-platform/components/blob/master/src/i18n/en.json#L50
add hyphen in aria-label value add Russian and English translation
replace div to nav tag replace div to h1 for title wrap MiniToc links into h2 and h3 tags
There was a problem hiding this comment.
Before using nav and h1-h6 tags:
2024-07-11.09-13-35.mov
After:
2024-07-11.10-37-54.mov
Screen Reader: https://www.nvaccess.org
There was a problem hiding this comment.
Hey, the videos are great, it's great that you made them.
There are a couple of things that need to be fixed.
Let's leave the title only for the dc-mini-toc__title and make a div tag -> h2. We will leave the remaining elements as links in the mini-current itself, it will be more correct, this became more noticeable thanks to the video with a screen reader.
Apparently Scrollspy component already adds an aria-current label for mini-current elements, which means we don’t need to put miniTocContent into a separate variable at all and can leave it as is.
delete h2, h3 tags from MiniToc items delete aria-current from MiniToc items change h1 to h2 for MiniToc title wrap MiniToc title in div
src/components/MiniToc/MiniToc.tsx
Outdated
| <div className={b()}> | ||
| <div className={b('title')}>{t<string>('title')}:</div> | ||
| <nav className={b()} aria-label={t('article-navigation')}> | ||
| <div> |
There was a problem hiding this comment.
for what reason you need extra div here ?
add aria-label to MiniToc and aria-current for MiniToc headings.
Task: https://github.com/orgs/diplodoc-platform/projects/2/views/1?pane=issue&itemId=55495812