♿️(frontend) add aria-hidden to decorative icons in dropdown menu#2093
♿️(frontend) add aria-hidden to decorative icons in dropdown menu#2093
Conversation
|
Size Change: +9 B (0%) Total Size: 4.24 MB
|
Ovgodd
force-pushed
the
fix/more-optns-decorative-icons
branch
from
d89bd67 to
eb43bae
Compare
Ovgodd
force-pushed
the
fix/more-optns-decorative-icons
branch
from
eb43bae to
79d362e
Compare
| <Box | ||
| $theme="neutral" | ||
| $variation={isDisabled ? 'tertiary' : 'primary'} | ||
| aria-hidden="true" |
There was a problem hiding this comment.
You add aria-hidden="true" here, is it necessary to add it on the icon as well ?
There was a problem hiding this comment.
You add
aria-hidden="true"here, is it necessary to add it on the icon as well ?
Yes you are right, the aria-hidden on the Box alone would work, but putting it directly on each icon is cleaner (and respect RGAA rules ),
it's explicit about what we're hiding and why.
If someone refactors the layout later, the icons stay hidden.
But I I've removed it from the Box to avoid redundancy.
There was a problem hiding this comment.
Yes but in every rerender we are recloning a element, to add a aria that is not totally necessary, so I am mitigate about this code snippet. Is it not cleaner so to add the aria-hidden directly on the icon when it is used ?
| const customIconElement = | ||
| option.icon && typeof option.icon !== 'string' | ||
| ? isValidElement(option.icon) | ||
| ? cloneElement( | ||
| option.icon as ReactElement<{ | ||
| 'aria-hidden'?: boolean; | ||
| }>, | ||
| { 'aria-hidden': true }, | ||
| ) | ||
| : option.icon | ||
| : null; | ||
|
|
There was a problem hiding this comment.
Not sure about this snippet, comment here: #2093 (comment)
I think we should either, let the Box forcing the aria-hidden (parent level), or adding it to the icon directly.
There was a problem hiding this comment.
Hey !
Yes, adding it to the icon directly is a good compromise, I agree! I've updated all SVG icons in DocsGridActions and DocToolBox with aria-hidden="true", and removed the cloneElement logic from DropdownMenu.
Ovgodd
force-pushed
the
fix/more-optns-decorative-icons
branch
3 times, most recently
from
5bfb21a to
c5ce738
Compare
Mark decorative SVG icons with aria-hidden.
Ovgodd
force-pushed
the
fix/more-optns-decorative-icons
branch
from
c5ce738 to
b8e1d12
Compare
Added - 🚸(frontend) hint min char search users #2064 Changed - 💄(frontend) improve comments highlights #1961 - ♿️(frontend) improve BoxButton a11y and native button semantics #2103 - ♿️(frontend) improve language picker accessibility #2069 - ♿️(frontend) add aria-hidden to decorative icons in dropdown menu #2093 Fixed - 🐛(y-provider) destroy Y.Doc instances after each convert request #2129 - 🐛(backend) remove deleted sub documents in favorite_list endpoint #2083
Added - 🚸(frontend) hint min char search users #2064 Changed - 💄(frontend) improve comments highlights #1961 - ♿️(frontend) improve BoxButton a11y and native button semantics #2103 - ♿️(frontend) improve language picker accessibility #2069 - ♿️(frontend) add aria-hidden to decorative icons in dropdown menu #2093 Fixed - 🐛(y-provider) destroy Y.Doc instances after each convert request #2129 - 🐛(backend) remove deleted sub documents in favorite_list endpoint #2083
2 participants
Description
Add
aria-hiddento decorative SVG icons in the dropdown menu so they are ignored by assistive technologies. ( NVDA / VO )Proposal
aria-hiddenandfocusable="false"to custom SVG icons rendered inDropdownMenuoptionscustomIconElementconstant for clarity