-
-
Notifications
You must be signed in to change notification settings - Fork 370
[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 2 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,47 @@ | |
|
||
<twig:ux:icon name="user-profile" aria-hidden="false" /> | ||
|
||
## Accessibility: Descriptive Titles and Descriptions | ||
|
||
.. versionadded:: NEXT\_VERSION | ||
Check failure on line 486 in src/Icons/doc/index.rst
|
||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The `ux_icon()` function and the `<twig:ux:icon>` component now support accessible SVG metadata via the `title` and `desc` attributes. | ||
|
||
These are automatically injected into the `<svg>` markup as child elements, and properly referenced using `aria-labelledby` for improved screen reader support. | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
**How it works:** | ||
|
||
When you pass a `title` and/or `desc` attribute, they are rendered inside the `<svg>` as follows: | ||
|
||
.. code-block:: html+twig | ||
|
||
``` | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{{ 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> | ||
Comment on lines
+510
to
+512
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. The |
||
<!-- inner SVG content --> | ||
</svg> | ||
``` | ||
|
||
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,53 @@ public function __construct( | |
public function toHtml(): string | ||
{ | ||
$htmlAttributes = ''; | ||
foreach ($this->attributes as $name => $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']); | ||
|
||
// Prepare <title> and <desc> elements | ||
$labelledByIds = []; | ||
$a11yContent = ''; | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if ($title) { | ||
$titleId = 'title-' . bin2hex(random_bytes(4)); | ||
$labelledByIds[] = $titleId; | ||
$a11yContent .= sprintf('<title id="%s">%s</title>', $titleId, htmlspecialchars((string) $title, ENT_QUOTES)); | ||
} | ||
|
||
if ($desc) { | ||
$descId = 'desc-' . 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. I feel like it's a not a good idea to generate random strings for eacg title/desc, especially when you render a lot of icons in a single page. 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. "IDs for the <title> and elements are generated only when the aria-labelledby attribute is automatically added to reference them." 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. Yeah I know, but I don't think generating random values like that is a good thing, it can be CPU intensive and it's untestable. Instead, either we generate these ids based on the icon (name, title, description, ...), either we use an internal incremental counter or something similar |
||
$labelledByIds[] = $descId; | ||
$a11yContent .= sprintf('<desc id="%s">%s</desc>', $descId, htmlspecialchars((string) $desc, ENT_QUOTES)); | ||
} | ||
|
||
// Only add aria-labelledby if not already present and we have content | ||
if ($a11yContent !== '' && !isset($attributes['aria-labelledby'])) { | ||
$attributes['aria-labelledby'] = implode(' ', $labelledByIds); | ||
} | ||
|
||
// Build final attributes string | ||
foreach ($attributes as $name => $value) { | ||
if (false === $value) { | ||
continue; | ||
} | ||
|
||
// Special case for aria-* attributes | ||
// https://www.w3.org/TR/wai-aria-1.1/#state_prop_def | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (true === $value && str_starts_with($name, 'aria-')) { | ||
$value = 'true'; | ||
} | ||
|
||
$htmlAttributes .= ' '.$name; | ||
|
||
if (true === $value) { | ||
continue; | ||
} | ||
|
||
$value = htmlspecialchars($value, \ENT_QUOTES | \ENT_SUBSTITUTE, 'UTF-8'); | ||
$value = htmlspecialchars((string) $value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); | ||
$htmlAttributes .= '="'.$value.'"'; | ||
} | ||
|
||
return '<svg'.$htmlAttributes.'>'.$this->innerSvg.'</svg>'; | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public function getInnerSvg(): string | ||
|
Uh oh!
There was an error while loading. Please reload this page.