Skip to content

Commit 3a2937e

Browse files
kbondkbond
committed
feature #1797 [Icons] Improve aria attributes rendering (smnandre)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Icons] Improve aria attributes rendering Fix: #1796 * Convert aria boolean values to "true/~~false~~" strings (as previously done in TwigComponent) * Fix aria-hidden is added when source icon contains aria label * Add pre-rendering filter chain in IconRenderer (internal) Update: After discussion in this issue, i suggest we keep the current behaviour: "boolean false removes an attribute" | Attribute \ Value | true | false | "true" | "false" | | - | - | - | - | - | | required | required | | required="true" | required=false" | | aria-required | aria-required="true" | | aria-required="true" | aria-required="false" | So just something to ease the DX but that let the developer responsible of setting a correct value. Commits ------- aeded4a [Icons] Improve aria attributes rendering
2 parents 955764b + aeded4a commit 3a2937e

File tree

5 files changed

+74
-21
lines changed

5 files changed

+74
-21
lines changed

‎src/Icons/src/Icon.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,20 @@ public function toHtml(): string
143143
if (false === $value) {
144144
continue;
145145
}
146+
147+
// Special case for aria-* attributes
148+
// https://www.w3.org/TR/wai-aria-1.1/#state_prop_def
149+
if (true === $value && str_starts_with($name, 'aria-')) {
150+
$value = 'true';
151+
}
152+
146153
$htmlAttributes .= ' '.$name;
147-
if (true !== $value) {
148-
$value = htmlspecialchars($value, \ENT_QUOTES | \ENT_SUBSTITUTE, 'UTF-8');
149-
$htmlAttributes .= '="'.$value.'"';
154+
if (true === $value) {
155+
continue;
150156
}
157+
158+
$value = htmlspecialchars($value, \ENT_QUOTES | \ENT_SUBSTITUTE, 'UTF-8');
159+
$htmlAttributes .= '="'.$value.'"';
151160
}
152161

153162
return '<svg'.$htmlAttributes.'>'.$this->innerSvg.'</svg>';

‎src/Icons/src/IconRenderer.php

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,21 +37,34 @@ public function __construct(
3737
*/
3838
public function renderIcon(string $name, array $attributes = []): string
3939
{
40-
return $this->registry->get($name)
41-
->withAttributes($this->getIconAttributes($name, $attributes))
42-
->toHtml()
43-
;
40+
$icon = $this->registry->get($name)
41+
->withAttributes($this->defaultIconAttributes)
42+
->withAttributes($attributes);
43+
44+
foreach ($this->getPreRenderers() as $preRenderer) {
45+
$icon = $preRenderer($icon);
46+
}
47+
48+
return $icon->toHtml();
4449
}
4550

46-
private function getIconAttributes(string $name, array $attributes): array
51+
/**
52+
* @return iterable<callable(Icon): Icon>
53+
*/
54+
private function getPreRenderers(): iterable
4755
{
48-
$iconAttributes = $this->defaultIconAttributes;
56+
yield self::setAriaHidden(...);
57+
}
4958

50-
// Add aria-hidden attribute
51-
if ([] === array_intersect(['aria-hidden', 'aria-label', 'aria-labelledby', 'title'], array_keys($attributes))) {
52-
$iconAttributes['aria-hidden'] = 'true';
59+
/**
60+
* Set `aria-hidden=true` if not defined & no textual alternative provided.
61+
*/
62+
private static function setAriaHidden(Icon $icon): Icon
63+
{
64+
if ([] === array_intersect(['aria-hidden', 'aria-label', 'aria-labelledby', 'title'], array_keys($icon->getAttributes()))) {
65+
return $icon->withAttributes(['aria-hidden' => 'true']);
5366
}
5467

55-
return [...$iconAttributes, ...$attributes];
68+
return $icon;
5669
}
5770
}

‎src/Icons/tests/Integration/RenderIconsInTwigTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function testRenderIcons(): void
2828
<ul class="svg">
2929
<li id="first"><svg viewBox="0 0 24 24" fill="currentColor" class="h-8 w-8" aria-hidden="true"><path fill-rule="evenodd" d="M7.5 6a4.5 4.5 0 1 1 9 0 4.5 4.5 0 0 1-9 0ZM3.751 20.105a8.25 8.25 0 0 1 16.498 0 .75.75 0 0 1-.437.695A18.683 18.683 0 0 1 12 22.5c-2.786 0-5.433-.608-7.812-1.7a.75.75 0 0 1-.437-.695Z" clip-rule="evenodd"></path></svg></li>
3030
<li id="second"><svg viewBox="0 0 24 24" fill="currentColor" class="w-6 h-6" aria-label="AriaLabel"><path fill-rule="evenodd" d="M7.5 6a4.5 4.5 0 1 1 9 0 4.5 4.5 0 0 1-9 0ZM3.751 20.105a8.25 8.25 0 0 1 16.498 0 .75.75 0 0 1-.437.695A18.683 18.683 0 0 1 12 22.5c-2.786 0-5.433-.608-7.812-1.7a.75.75 0 0 1-.437-.695Z" clip-rule="evenodd"></path></svg></li>
31-
<li id="third"><svg viewBox="0 0 24 24" fill="currentColor" class="w-6 h-6" aria-hidden="true" data-action="string &quot;with&quot; quotes"><path fill-rule="evenodd" d="M19.916 4.626a.75.75 0 0 1 .208 1.04l-9 13.5a.75.75 0 0 1-1.154.114l-6-6a.75.75 0 0 1 1.06-1.06l5.353 5.353 8.493-12.74a.75.75 0 0 1 1.04-.207Z" clip-rule="evenodd"></path></svg></li>
31+
<li id="third"><svg viewBox="0 0 24 24" fill="currentColor" class="w-6 h-6" data-action="string &quot;with&quot; quotes" aria-hidden="true"><path fill-rule="evenodd" d="M19.916 4.626a.75.75 0 0 1 .208 1.04l-9 13.5a.75.75 0 0 1-1.154.114l-6-6a.75.75 0 0 1 1.06-1.06l5.353 5.353 8.493-12.74a.75.75 0 0 1 1.04-.207Z" clip-rule="evenodd"></path></svg></li>
3232
<li id="fifth"><svg viewBox="0 0 24 24" fill="currentColor" class="h-8 w-8" aria-hidden="true"><path fill-rule="evenodd" d="M7.5 6a4.5 4.5 0 1 1 9 0 4.5 4.5 0 0 1-9 0ZM3.751 20.105a8.25 8.25 0 0 1 16.498 0 .75.75 0 0 1-.437.695A18.683 18.683 0 0 1 12 22.5c-2.786 0-5.433-.608-7.812-1.7a.75.75 0 0 1-.437-.695Z" clip-rule="evenodd"></path></svg></li>
3333
<li id="sixth"><svg viewBox="0 0 24 24" fill="currentColor" class="w-6 h-6" aria-labelledby="foo"><path fill-rule="evenodd" d="M19.916 4.626a.75.75 0 0 1 .208 1.04l-9 13.5a.75.75 0 0 1-1.154.114l-6-6a.75.75 0 0 1 1.06-1.06l5.353 5.353 8.493-12.74a.75.75 0 0 1 1.04-.207Z" clip-rule="evenodd"></path></svg></li>
3434
<li id="seventh"><svg viewBox="0 0 24 24" fill="currentColor" class="w-6 h-6" aria-hidden="true"><path fill-rule="evenodd" d="M19.916 4.626a.75.75 0 0 1 .208 1.04l-9 13.5a.75.75 0 0 1-1.154.114l-6-6a.75.75 0 0 1 1.06-1.06l5.353 5.353 8.493-12.74a.75.75 0 0 1 1.04-.207Z" clip-rule="evenodd"></path></svg></li>

‎src/Icons/tests/Unit/IconRendererTest.php

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function testRenderIconWithAttributes(): void
6666

6767
$svg = $iconRenderer->renderIcon('foo', $attributes);
6868

69-
$this->assertSame('<svg aria-hidden="true" viewBox="0 0 24 24" class="icon" id="FooBar"><path d="M0 0L12 12"/></svg>', $svg);
69+
$this->assertSame('<svg viewBox="0 0 24 24" class="icon" id="FooBar" aria-hidden="true"><path d="M0 0L12 12"/></svg>', $svg);
7070
}
7171

7272
public function testRenderIconWithDefaultAttributes(): void
@@ -163,12 +163,14 @@ public static function provideAttributesWithDefaultAttributesCases()
163163
/**
164164
* @dataProvider provideAriaHiddenCases
165165
*
166-
* @param array<string, string> $attributes
166+
* @param string|array{string, array<string, string|bool>} $icon
167+
* @param array<string, string|bool> $attributes
167168
*/
168-
public function testRenderIconWithAutoAriaHidden(array $attributes, string $expectedSvg): void
169+
public function testRenderIconWithAutoAriaHidden(string|array $icon, array $attributes, string $expectedSvg): void
169170
{
171+
$icon = (array) $icon;
170172
$registry = $this->createRegistry([
171-
'foo' => '<path d="M0 0L12 12"/>',
173+
'foo' => $icon,
172174
]);
173175
$iconRenderer = new IconRenderer($registry);
174176

@@ -177,31 +179,52 @@ public function testRenderIconWithAutoAriaHidden(array $attributes, string $expe
177179
}
178180

179181
/**
180-
* @return iterable<array{array<string, string>, string}>
182+
* @return iterable<array{string|array{string, array<string, string|bool>}, array<string, string|bool>, string}>
181183
*/
182184
public static function provideAriaHiddenCases(): iterable
183185
{
184186
yield 'no attributes' => [
187+
'<path d="M0 0L12 12"/>',
185188
[],
186189
'<svg aria-hidden="true"><path d="M0 0L12 12"/></svg>',
187190
];
191+
yield 'no attributes and icon attribute' => [
192+
['<path d="M0 0L12 12"/>', ['aria-hidden' => 'false']],
193+
[],
194+
'<svg aria-hidden="false"><path d="M0 0L12 12"/></svg>',
195+
];
196+
yield 'no attributes and icon label' => [
197+
['<path d="M0 0L12 12"/>', ['aria-label' => 'foo']],
198+
[],
199+
'<svg aria-label="foo"><path d="M0 0L12 12"/></svg>',
200+
];
188201
yield 'aria-hidden attribute' => [
202+
'<path d="M0 0L12 12"/>',
203+
['aria-hidden' => 'true'],
204+
'<svg aria-hidden="true"><path d="M0 0L12 12"/></svg>',
205+
];
206+
yield 'aria-hidden attribute and icon attribute' => [
207+
['<path d="M0 0L12 12"/>', ['aria-hidden' => 'false']],
189208
['aria-hidden' => 'true'],
190209
'<svg aria-hidden="true"><path d="M0 0L12 12"/></svg>',
191210
];
192211
yield 'aria-hidden false + aria-label' => [
212+
'<path d="M0 0L12 12"/>',
193213
['aria-hidden' => 'false', 'aria-label' => 'foo'],
194214
'<svg aria-hidden="false" aria-label="foo"><path d="M0 0L12 12"/></svg>',
195215
];
196216
yield 'title attribute' => [
217+
'<path d="M0 0L12 12"/>',
197218
['title' => 'foo'],
198219
'<svg title="foo"><path d="M0 0L12 12"/></svg>',
199220
];
200221
yield 'aria-labelledby attribute' => [
222+
'<path d="M0 0L12 12"/>',
201223
['aria-labelledby' => 'foo'],
202224
'<svg aria-labelledby="foo"><path d="M0 0L12 12"/></svg>',
203225
];
204226
yield 'aria-label attribute' => [
227+
'<path d="M0 0L12 12"/>',
205228
['aria-label' => 'foo'],
206229
'<svg aria-label="foo"><path d="M0 0L12 12"/></svg>',
207230
];

‎src/Icons/tests/Unit/IconTest.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -241,10 +241,18 @@ public static function provideRenderAttributesTestCases(): iterable
241241
['foo' => true],
242242
'<svg foo>',
243243
];
244-
yield 'it_does_not_render_attribute_with_false_value' => [
245-
['foo' => false],
244+
yield 'it_does_not_render_attributes_with_false_value' => [
245+
['foo' => false, 'aria-foo' => false],
246246
'<svg>',
247247
];
248+
yield 'it_converts_aria_attribute_with_true_value' => [
249+
['aria-bar' => true],
250+
'<svg aria-bar="true">',
251+
];
252+
yield 'it_does_not_convert_aria_attribute_with_string_value' => [
253+
['aria-foo' => '0', 'aria-bar' => 'true', 'aria-baz' => 'false'],
254+
'<svg aria-foo="0" aria-bar="true" aria-baz="false">',
255+
];
248256
}
249257

250258
public static function provideWithAttributesTestCases(): iterable

0 commit comments

Comments
 (0)