-
-
Notifications
You must be signed in to change notification settings - Fork 369
[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 all 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,7 +139,45 @@ 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']); | ||
|
||
$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); | ||
} | ||
|
||
foreach ($attributes as $name => $value) { | ||
if (false === $value) { | ||
continue; | ||
} | ||
|
@@ -159,7 +197,7 @@ public function toHtml(): string | |
$htmlAttributes .= '="'.$value.'"'; | ||
} | ||
|
||
return '<svg'.$htmlAttributes.'>'.$this->innerSvg.'</svg>'; | ||
xDeSwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return '<svg' . $htmlAttributes . '>' . $a11yContent . $innerSvg . '</svg>'; | ||
} | ||
|
||
public function getInnerSvg(): string | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
<?php | ||
|
||
namespace Symfony\UX\Icons\Tests\Unit; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Symfony\UX\Icons\Icon; | ||
|
||
class IconAccessibilityTest extends TestCase | ||
{ | ||
public function testTitleIsIncludedInOutput() | ||
{ | ||
$icon = new Icon('<path d="M0 0h24v24H0z" fill="none"/>', ['title' => 'Test Icon']); | ||
$html = $icon->toHtml(); | ||
$this->assertMatchesRegularExpression('/<title( id="[^"]*")?>Test Icon<\/title>/', $html); | ||
} | ||
|
||
public function testDescIsIncludedInOutput() | ||
{ | ||
$icon = new Icon('<circle cx="12" cy="12" r="10"/>', ['desc' => 'This is a test circle']); | ||
$html = $icon->toHtml(); | ||
$this->assertMatchesRegularExpression('/<desc( id="[^"]*")?>This is a test circle<\/desc>/', $html); | ||
} | ||
|
||
public function testTitleAndDescWithCustomAriaLabelledBy() | ||
{ | ||
$attributes = [ | ||
'title' => 'My Line', | ||
'desc' => 'This is a diagonal line', | ||
'aria-labelledby' => 'custom-id', | ||
]; | ||
$icon = new Icon('<line x1="0" y1="0" x2="10" y2="10"/>', $attributes); | ||
|
||
$html = $icon->toHtml(); | ||
$this->assertStringContainsString('<title>My Line</title>', $html); | ||
$this->assertStringContainsString('<desc>This is a diagonal line</desc>', $html); | ||
$this->assertStringContainsString('aria-labelledby="custom-id"', $html); | ||
} | ||
|
||
public function testToStringReturnsHtml() | ||
{ | ||
$icon = new Icon('<path d="M0 0h24v24H0z"/>'); | ||
$this->assertSame($icon->toHtml(), (string) $icon); | ||
} | ||
} |
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