-
-
Notifications
You must be signed in to change notification settings - Fork 383
[Icons] Add support for <title> and <desc> elements in SVG for accessibility #2904
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
Changes from 5 commits
b9f41c9
423937e
317f732
8b8296d
27d316b
3dead42
59703cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -481,6 +481,51 @@ of the following attributes: ``aria-label``, ``aria-labelledby`` or ``title``. | |
|
||
<twig:ux:icon name="user-profile" aria-hidden="false" /> | ||
|
||
**Accessibility: Descriptive Titles and Descriptions** | ||
|
||
.. versionadded:: 2.28 | ||
|
||
The `ux_icon()` function and the `<twig:ux:icon>` component now support accessible SVG metadata via the `title` and `desc` attributes in 2.28. | ||
|
||
These are automatically injected into the ``<svg>`` markup as child elements, and properly referenced using ``aria-labelledby`` for improved screen reader support. | ||
|
||
**How it works:** | ||
|
||
When you pass a `title` and/or `desc` attribute, they are rendered inside the `<svg>` as follows: | ||
|
||
.. code-block:: twig | ||
|
||
{{ ux_icon('bi:plus-square-dotted', { | ||
width: '16px', | ||
height: '16px', | ||
class: 'text-success', | ||
title: 'Add Stock', | ||
desc: 'This icon indicates stock entry functionality.' | ||
}) }} | ||
|
||
Renders: | ||
|
||
.. code-block:: html | ||
|
||
<svg class="text-success" width="16px" height="16px" aria-labelledby="icon-title-abc icon-desc-def"> | ||
<title id="icon-title-abc">Add Stock</title> | ||
<desc id="icon-desc-def">This icon indicates stock entry functionality.</desc> | ||
<!-- inner SVG content --> | ||
</svg> | ||
|
||
.. note:: | ||
|
||
- If ``aria-labelledby`` is already defined in your attributes, it will **not** be overwritten. | ||
- ``role="img"`` is **not added automatically**. You may choose to include it if your use case requires. | ||
- When neither ``title``, ``desc``, ``aria-label``, nor ``aria-labelledby`` are provided, ``aria-hidden="true"`` will still be automatically applied. | ||
|
||
This feature brings UX Icons in line with modern accessibility recommendations and helps developers build more inclusive user interfaces. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not so sure about that..
https://design-system.w3.org/styles/svg-icons.html#non-decorative-svg-accessibility I mean, "then it is not being used as an icon" is pretty much outside the scope of this bundle. Please correct me if i'm outdated on this :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also
So let's push for this recommendation no ? |
||
|
||
To learn more about accessible SVG elements: | ||
|
||
- `MDN: <title>`\_ — [https://developer.mozilla.org/en-US/docs/Web/SVG/Element/title](https://developer.mozilla.org/en-US/docs/Web/SVG/Element/title) | ||
- `MDN: <desc>`\_ — [https://developer.mozilla.org/en-US/docs/Web/SVG/Element/desc](https://developer.mozilla.org/en-US/docs/Web/SVG/Element/desc) | ||
|
||
Performance | ||
----------- | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,27 +139,68 @@ public function __construct( | |
public function toHtml(): string | ||
{ | ||
$htmlAttributes = ''; | ||
foreach ($this->attributes as $name => $value) { | ||
if (false === $value) { | ||
$innerSvg = $this->innerSvg; | ||
$attributes = $this->attributes; | ||
|
||
// Extract and remove title/desc attributes if present | ||
$title = $attributes['title'] ?? null; | ||
$desc = $attributes['desc'] ?? null; | ||
unset($attributes['title'], $attributes['desc']); | ||
|
||
$labelledByIds = []; | ||
$a11yContent = ''; | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Check if aria-labelledby should be added automatically | ||
$shouldSetLabelledBy = !isset($attributes['aria-labelledby']) && ($title || $desc); | ||
|
||
if ($title) { | ||
if ($shouldSetLabelledBy) { | ||
$titleId = 'title-' . bin2hex(random_bytes(4)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will create unpredictable HTML |
||
$labelledByIds[] = $titleId; | ||
$a11yContent .= sprintf('<title id="%s">%s</title>', $titleId, htmlspecialchars((string) $title, ENT_QUOTES)); | ||
} else { | ||
$a11yContent .= sprintf('<title>%s</title>', htmlspecialchars((string) $title, ENT_QUOTES)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will add another <title> tag in the SVG if one is already present, generating invalid markup |
||
} | ||
} | ||
|
||
if ($desc) { | ||
if ($shouldSetLabelledBy) { | ||
$descId = 'desc-' . bin2hex(random_bytes(4)); | ||
$labelledByIds[] = $descId; | ||
$a11yContent .= sprintf('<desc id="%s">%s</desc>', $descId, htmlspecialchars((string) $desc, ENT_QUOTES)); | ||
} else { | ||
$a11yContent .= sprintf('<desc>%s</desc>', htmlspecialchars((string) $desc, ENT_QUOTES)); | ||
} | ||
} | ||
|
||
if ($shouldSetLabelledBy) { | ||
$attributes['aria-labelledby'] = implode(' ', $labelledByIds); | ||
} | ||
|
||
// Build final attributes string | ||
foreach ($attributes as $name => $value) { | ||
if ($value === false) { | ||
continue; | ||
} | ||
|
||
// Special case for aria-* attributes | ||
// https://www.w3.org/TR/wai-aria-1.1/#state_prop_def | ||
if (true === $value && str_starts_with($name, 'aria-')) { | ||
if ($value === true && str_starts_with($name, 'aria-')) { | ||
$value = 'true'; | ||
} | ||
|
||
$htmlAttributes .= ' '.$name; | ||
if (true === $value) { | ||
|
||
$htmlAttributes .= ' ' . $name; | ||
|
||
if ($value === true) { | ||
continue; | ||
} | ||
|
||
$value = htmlspecialchars($value, \ENT_QUOTES | \ENT_SUBSTITUTE, 'UTF-8'); | ||
$htmlAttributes .= '="'.$value.'"'; | ||
$htmlAttributes .= '="' . $value . '"'; | ||
} | ||
|
||
return '<svg'.$htmlAttributes.'>'.$this->innerSvg.'</svg>'; | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Inject <title> and <desc> before inner content | ||
return '<svg' . $htmlAttributes . '>' . $a11yContent . $innerSvg . '</svg>'; | ||
xDeSwa marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
} | ||
|
||
public function getInnerSvg(): string | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
id
attributes do no match the actual behaviour