-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add an attr
function to make outputting HTML attributes easier
#3930
base: 3.x
Are you sure you want to change the base?
Changes from all commits
03a5de1
e3a74bd
aef2551
9722108
717f872
09ff3b4
799fb28
0f694fd
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 |
---|---|---|
|
@@ -12,8 +12,10 @@ | |
namespace Twig\Extra\Html; | ||
|
||
use Symfony\Component\Mime\MimeTypes; | ||
use Twig\Environment; | ||
use Twig\Error\RuntimeError; | ||
use Twig\Extension\AbstractExtension; | ||
use Twig\Runtime\EscaperRuntime; | ||
use Twig\TwigFilter; | ||
use Twig\TwigFunction; | ||
|
||
|
@@ -30,6 +32,7 @@ public function getFilters(): array | |
{ | ||
return [ | ||
new TwigFilter('data_uri', [$this, 'dataUri']), | ||
new TwigFilter('html_attr_merge', [self::class, 'htmlAttrMerge']), | ||
]; | ||
} | ||
|
||
|
@@ -38,6 +41,7 @@ public function getFunctions(): array | |
return [ | ||
new TwigFunction('html_classes', [self::class, 'htmlClasses']), | ||
new TwigFunction('html_cva', [self::class, 'htmlCva']), | ||
new TwigFunction('html_attr', [self::class, 'htmlAttr'], ['needs_environment' => true, 'is_safe' => ['html']]), | ||
]; | ||
} | ||
|
||
|
@@ -124,4 +128,96 @@ public static function htmlCva(array|string $base = [], array $variants = [], ar | |
{ | ||
return new Cva($base, $variants, $compoundVariants, $defaultVariant); | ||
} | ||
|
||
public static function htmlAttrMerge(...$arrays): array | ||
{ | ||
$result = []; | ||
|
||
foreach ($arrays as $argNumber => $array) { | ||
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. Not guaranteed that we have a number here. |
||
if (!$array) { | ||
continue; | ||
} | ||
|
||
if (!is_iterable($array)) { | ||
throw new RuntimeError(sprintf('The "attr_merge" filter only works with mappings or "Traversable", got "%s" for argument %d.', \gettype($array), $argNumber + 1)); | ||
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. Should this be " The |
||
} | ||
|
||
$array = (array) ($array); | ||
|
||
foreach (['data', 'aria'] as $flatOutKey) { | ||
if (!isset($array[$flatOutKey])) { | ||
continue; | ||
} | ||
$values = (array) $array[$flatOutKey]; | ||
foreach ($values as $key => $value) { | ||
$result[$flatOutKey.'-'.$key] = $value; | ||
} | ||
unset($array[$flatOutKey]); | ||
} | ||
|
||
foreach (['class', 'style'] as $deepMergeKey) { | ||
if (!isset($array[$deepMergeKey])) { | ||
continue; | ||
} | ||
|
||
$value = $array[$deepMergeKey]; | ||
unset($array[$deepMergeKey]); | ||
|
||
if (!is_iterable($value)) { | ||
$value = (array) $value; | ||
} | ||
|
||
$result[$deepMergeKey] = [...$result[$deepMergeKey] ?? [], ...$value]; | ||
} | ||
|
||
$result = [...$result, ...$array]; | ||
} | ||
|
||
return $result; | ||
} | ||
|
||
public static function htmlAttr(Environment $env, ...$args): string | ||
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. Maybe a |
||
{ | ||
$attr = self::htmlAttrMerge(...$args); | ||
|
||
if (isset($attr['class'])) { | ||
$attr['class'] = trim(implode(' ', $attr['class'])); | ||
} | ||
|
||
if (isset($attr['style'])) { | ||
$style = ''; | ||
foreach ($attr['style'] as $name => $value) { | ||
if (is_numeric($name)) { | ||
$style .= $value.'; '; | ||
} else { | ||
$style .= $name.': '.$value.'; '; | ||
} | ||
Comment on lines
+190
to
+194
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 sure why this specific case for style.. i may have missed the reason earlier in the discussion ? 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. In case 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 have two other questions then (sorry) What if "aria" is something like Does that mean i can do something like this ? {{ html_attr(
{ style: ['color: red;'] },
{ style: {'color': 'blue'} },
{ style: {'color': 'green'} },
{ style: ['', 'color: black;'] },
) }} I'm shared between "i understand each of these usages" and "this could be very un-predictable when/if args are assembled from different layers / templates :| 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. Yes, definetly many edge cases I haven't thought through. Make sure you don't miss the inital idea in #3907, fwiw. I think we should aim for behaviour similar to
I hope I can come up with reasonable test cases for all those situations so we can discuss them before merge. |
||
} | ||
$attr['style'] = trim($style); | ||
} | ||
|
||
if (isset($attr['data'])) { | ||
foreach ($attr['data'] as $name => $value) { | ||
$attr['data-'.$name] = $value; | ||
} | ||
unset($attr['data']); | ||
} | ||
|
||
$result = ''; | ||
$runtime = $env->getRuntime(EscaperRuntime::class); | ||
|
||
foreach ($attr as $name => $value) { | ||
if ($value === false) { | ||
continue; | ||
} | ||
|
||
if ($value === true) { | ||
$value = $name; | ||
} | ||
|
||
$result .= $runtime->escape($name, 'html_attr').'="'.$runtime->escape($value).'" '; | ||
mpdude marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return trim($result); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
--TEST-- | ||
"html_attr_merge" function | ||
--TEMPLATE-- | ||
{% autoescape false %} | ||
#1 - {{ { foo: 'bar' } | html_attr_merge({ bar: 'baz' }) | json_encode }} | ||
#2 - {{ { class: 'foo' } | html_attr_merge({ class: 'bar' }) | json_encode }} | ||
#3 - {{ { class: 'foo' } | html_attr_merge({ class: ['bar', 'baz'] }) | json_encode }} | ||
#4 - {{ { class: { special: 'foo' } } | ||
| html_attr_merge({ class: ['bar', 'baz'] }) | ||
| html_attr_merge({ class: { special: 'qux' } }) | ||
| json_encode }} | ||
{% endautoescape %} | ||
--DATA-- | ||
return [] | ||
--EXPECT-- | ||
#1 - {"foo":"bar","bar":"baz"} | ||
#2 - {"class":["foo","bar"]} | ||
#3 - {"class":["foo","bar","baz"]} | ||
#4 - {"class":{"special":"qux","0":"bar","1":"baz"}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,184 @@ | ||
<?php | ||
|
||
namespace Twig\Extra\Html\Tests; | ||
|
||
use PHPUnit\Framework\TestCase; | ||
use Twig\Extra\Html\HtmlExtension; | ||
|
||
class HtmlAttrMergeTest extends TestCase | ||
{ | ||
/** | ||
* @dataProvider htmlAttrProvider | ||
*/ | ||
public function testMerge(array $expected, array $inputs) | ||
{ | ||
$result = HtmlExtension::htmlAttrMerge(...$inputs); | ||
|
||
self::assertSame($expected, $result); | ||
} | ||
|
||
public function htmlAttrProvider(): \Generator | ||
{ | ||
yield 'merging different attributes from two arrays' => [ | ||
['id' => 'some-id', 'label' => 'some-label'], | ||
[ | ||
['id' => 'some-id'], | ||
['label' => 'some-label'], | ||
] | ||
]; | ||
|
||
yield 'merging different attributes from three arrays' => [ | ||
['id' => 'some-id', 'label' => 'some-label', 'role' => 'main'], | ||
[ | ||
['id' => 'some-id'], | ||
['label' => 'some-label'], | ||
['role' => 'main'], | ||
] | ||
]; | ||
|
||
yield 'merging different attributes from Traversables' => [ | ||
['id' => 'some-id', 'label' => 'some-label', 'role' => 'main'], | ||
[ | ||
new \ArrayIterator(['id' => 'some-id']), | ||
new \ArrayIterator(['label' => 'some-label']), | ||
new \ArrayIterator(['role' => 'main']), | ||
] | ||
]; | ||
|
||
yield 'later keys override previous ones' => [ | ||
['id' => 'other'], | ||
[ | ||
['id' => 'this'], | ||
['id' => 'that'], | ||
['id' => 'other'], | ||
] | ||
]; | ||
|
||
yield 'ignore empty strings or arrays passed as arguments' => [ | ||
['some' => 'attribute'], | ||
[ | ||
['some' => 'attribute'], | ||
[], // empty array | ||
'', // empty string | ||
] | ||
]; | ||
|
||
yield 'keep "true" and "false" boolean values' => [ | ||
['disabled' => true, 'enabled' => false], | ||
[ | ||
['disabled' => true], | ||
['enabled' => false], | ||
] | ||
]; | ||
|
||
yield 'consolidate values for the "class" key' => [ | ||
['class' => ['foo', 'bar', 'baz']], | ||
[ | ||
['class' => ['foo']], | ||
['class' => 'bar'], // string, not array | ||
['class' => ['baz']], | ||
] | ||
]; | ||
|
||
yield 'class values can be overridden when they use names (array keys)' => [ | ||
['class' => ['foo', 'bar', 'importance' => 'high']], | ||
[ | ||
['class' => 'foo'], | ||
['class' => ['bar', 'importance' => 'low']], | ||
['class' => ['importance' => 'high']], | ||
] | ||
]; | ||
|
||
yield 'inline style values with numerical keys are merely collected' => [ | ||
['style' => ['font-weight: light', 'color: green', 'font-weight: bold']], | ||
[ | ||
['style' => ['font-weight: light']], | ||
['style' => ['color: green', 'font-weight: bold']], | ||
] | ||
]; | ||
|
||
yield 'inline style values can be overridden when they use names (array keys)' => [ | ||
['style' => ['font-weight' => 'bold', 'color' => 'red']], | ||
[ | ||
['style' => ['font-weight' => 'light']], | ||
['style' => ['color' => 'green', 'font-weight' => 'bold']], | ||
['style' => ['color' => 'red']], | ||
] | ||
]; | ||
|
||
yield 'no merging happens when mixing numerically indexed inline styles with named ones' => [ | ||
['style' => ['color: green', 'color' => 'red']], | ||
[ | ||
['style' => ['color: green']], | ||
['style' => ['color' => 'red']], | ||
] | ||
]; | ||
|
||
yield 'turning aria attributes from array to flat keys' => [ | ||
['aria-role' => 'banner'], | ||
[ | ||
['aria' => ['role' => 'main']], | ||
['aria' => ['role' => 'banner']], | ||
] | ||
]; | ||
|
||
yield 'using aria attributes from a sub-array' => [ | ||
['aria-role' => 'main', 'aria-label' => 'none'], | ||
[ | ||
['aria' => ['role' => 'main', 'label' => 'none']], | ||
] | ||
]; | ||
|
||
yield 'merging aria attributes, where the array values overrides the flat one' => [ | ||
['aria-role' => 'navigation'], | ||
[ | ||
['aria-role' => 'main'], | ||
['aria' => ['role' => 'banner']], | ||
['aria' => ['role' => 'navigation']], | ||
] | ||
]; | ||
|
||
yield 'merging aria attributes, where the flat ones overrides the array' => [ | ||
['aria-role' => 'navigation'], | ||
[ | ||
['aria' => ['role' => 'main']], | ||
['aria-role' => 'banner'], | ||
['aria-role' => 'navigation'], | ||
] | ||
]; | ||
|
||
yield 'using data attributes in a sub-array' => [ | ||
['data-foo' => 'this', 'data-bar' => 'that'], | ||
[ | ||
['data' => ['foo' => 'this']], | ||
['data' => ['bar' => 'that']], | ||
] | ||
]; | ||
|
||
yield 'turning data attributes from array to flat keys' => [ | ||
['data-test' => 'bar'], | ||
[ | ||
['data' => ['test' => 'foo']], | ||
['data' => ['test' => 'bar']], | ||
] | ||
]; | ||
|
||
yield 'merging data attributes, where the array values overrides the flat one' => [ | ||
['data-test' => 'baz'], | ||
[ | ||
['data-test' => 'foo'], | ||
['data' => ['test' => 'bar']], | ||
['data' => ['test' => 'baz']], | ||
] | ||
]; | ||
|
||
yield 'merging data attributes, where the flat ones overrides the array' => [ | ||
['data-test' => 'baz'], | ||
[ | ||
['data' => ['test' => 'foo']], | ||
['data-test' => 'bar'], | ||
['data-test' => 'baz'], | ||
] | ||
]; | ||
} | ||
} |
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.
We can maybe have a
@param
to better describe$arrays
?