[Icons][DX] Accessibility helpers
Based on the proposed documentation update here from @smnandre, what do you think about the following helpers:
ux_icon_accessible(icon, label, attributes)for informative and functional icons, will enforce the label parameter and always add thearia-labelattributeux_icon_decorative(icon, attributes)for obviously decorative icons, will always add thearia-hiddenattribute set totrue
I can try to come with a PR if the idea get some traction, but I'm fine if someone else takes it :)
I think this could be automatic, based on the arguments passed.
Something like
- you pass a label -> aria-label,
- else we set it aria-hidden
WDYT ?
I'm not sure we should add multiple functions, as this is harder to dynamise in templates.
(poke @kbond)
I would love that the unique ux_icon supports that! but then I thought about what would be its API, and I found 2 options:
ux_icon(icon, label, attributes)but not sure it's OK to have the label before attributes is OK for general purpose, do we want to enforce it? should it be passed as null for decorative? should named argument be used for attributes so skip label?ux_icon(icon, attributes, label)could work for decorative ones but I find it a bit weird to have the label after attributes for informative/functional ones (but clearly a personal feeling here)
Another idea that pops on my mind could be to make ux_icon kind of polymorphic, supporting both ux_icon(string icon, array attributes) and ux_icon(string icon, string label, array attributes) and adapt the behavior depending on the type of the second argument.
But do we accept such "magic"?
We should add a bit of magic there if we want people to follow best practices :)
What about this :
{# A- Without label #}
<twig:Ux:Icon name="foo:bar" />
{# B- With label #}
<twig:Ux:Icon name="foo:bar" label="Profile" />
{# C- With aria-label #}
<twig:Ux:Icon name="foo:bar" aria-label="Profile" />
<!-- A- No accessible text: hidden -->
<svg aria-hidden="true" role="presentation none">
<!--...-->
</svg>
<!-- B- Label -> aria-label -->
<svg aria-label="Profile">
<!--...-->
</svg>
<!-- C- Same result -->
<svg aria-label="Profile">
<!--...-->
</svg>
Some readings i'd need to check more in depth :
- https://www.w3.org/TR/graphics-aria-1.0/#roles
- https://svgwg.org/svg2-draft/struct.html#implicit-aria-semantics
Update: same thing for Twig functions
{{ ux_icon('foo:bar', {label: "Profile"}) }}
My initial intent with dedicated functions is to enforce to pass the label to follow accessibility best practices when using an icon in an informative/functional purpose. That's why I went for dedicated functions, to throw an error if you don't pass the label when using the accessible variant, and not supporting a label at all for decorative ones.
Alternative idea could be to have the existing ux_icon require the label and have a single new function ux_icon_decorative (or ux_icon_hidden) without label. Or the other way around.
What do you think?
I don't really like the idea of adding additional functions. Is there an example elsewhere of twig enforcing accessibility best practices? My feeling is that this (and twig in general) is a lower level.
That being said, this issue is still very valid. I wonder if the attribute customization system @smnandre is working on could accommodate.
This is maybe something we could suggest in a more DX-friendly-way, via the WDT / profiler. It could be pretty easy to parse the DOM and find icons that are only child of links and have no label / title / etc.
My position, for now, would be
- having multiple methods will not encourage people to use them, as they will stay with what they use the most
- the same logic could justify a lot of other variants, and same thing i don't think it's something we want
I also think that
- accessibility is an important matter
- as package developpers, we have a responsability to allow / ease / encourage best practices
And in the same time i do think that
- we cannot enforce something against the will of the developpers
- there are a loooot of things more important to do in Twig "usage/practices" to improve on this matter
- and as @kbond said Twig is a low-level tool, not an end-user product
So yeah, let's try to find smart ways to educate & motivate people (ourselves first certainely), while keeping things simple and future-proof :)
I agree with all your points!
Just one thing, the proposed support for accessibility is not on Twig, that is indeed low-level, but on UX Icons, and I believe accessibility is part of UX 🙂
I find the solution proposed by @smnandre in https://github.com/symfony/ux/issues/1629#issuecomment-2005308626 a very good option: always add the aria-hidden="true" except if a label or aria-label is explicitly passed.
but on UX Icons, and I believe accessibility is part of UX 🙂
Fair point!
I find the solution proposed by @smnandre in #1629 (comment) a very good option: always add the aria-hidden="true" except if a label or aria-label is explicitly passed.
I'm good with this.
thanks @kbond! @smnandre do you want to build your solution or can I give it a try (I never contributed PRs to Symfony UX yet 😅)
I started some things, but i'll need your feedback / vision @kbond
I want to (when all done) have several "attributesRenderer" and "innerSvgRender" (or even one Interface with those two method" and a RendererChain (that's the idea not the perfect name i guess)
But i'm not too sure on how to deal with the subtle difference (IconRenderer currently get the "name" ... and some of what i just suggested currently cannot have it)....
So we could
- A) add name and prefix (or just name) in Icon (personal vision: it makes sense.... dont know what could be sides effects.. but like that an icon does not know they are identifies in ur system.
- B) create a IconMetadata (that would at a short-but-still-longer-than-A to keep "icon" / "prefix" / ... in a value we have access to.. but for now the data would only be "name" or "name:prefix" so .... (later on i'd like to have cachestatus, file info, iconset info (or a short version of) and initial storage of icon (locale, iconify, ...)
- C) it's already blurry enough in my head ^^
So @kbond if/when you have some time (even next week) poke me :))