New @attributes blade directive. #52783

danmatthews · 2024-09-13T16:03:18Z

I'm back, pinching neat ideas from people smarter than me again! - @tpetry had a great idea for an attributes directive on twitter so I thought i'd follow on the heels of the when helper and PR this in.

Note: I've updated the PR description to better reflect what's being proposed, but i've preserved the original description below.

Progress

@attributes helper compiles on normal HTML components.
Supports empty string values for empty attributes
null or false values will result in the attribute not being rendered
Supports lazy evaluation by passing a closure
Custom blade components support @attributes directive.
Slots support @attributes directive.
Tests for @attributes directive
Tests for @attributes directive on custom components
Tests for new Arr::toHtmlAttributes method.

The `@attributes()` blade directive.

Can be used to neatly, and conditionally compose attributes on your HTML elements or components in blade files.

@attributes([
   'class' => "mb-6",
   'disabled' => $disabled ? 'disabled' : null, // if null or false, won't be printed at all
   'role' => fn() => $isButton ? 'button' : null, // Lazily evaluated 
   'required' => when($isRequired, 'required'),
]);

You can also pass the $attributes variable that is automatically created for blade component files straight in, and it will traverse and render them, useful for passing attributes down to sub-components if needed.


{{-- Welcome.blade.php --}}
<x-button class="p-4 ml-4 text-center text-sm" disabled="true">Click Me</x-button>

{{-- button.blade.php --}}
<div>
   <button @attributes([$attributes, 'type' => 'button'])>
       {{ $slot }}
   </button>
</div>

{{-- Result --}}
<div>
    <button class="p-4 ml-4 text-center text-sm" disabled="true" type="button">Click Me</button> 
</div>

Original PR Description

Why

Can be used to conditionally render attributes on your HTML elements or components in blade files.

<div

@attributes([
    'wire:poll.5s="myMethod"' => $shouldPoll,
    'class="p-3 text-xs"' => $size === 'small'
    'class="p-16 text-base"' => $size !== 'small'
]);

><!-- ... -->

Possible variation that might work better but be less readable:

Passing the whole attribute as the key here might be problematic, what might be nice is if you could pass a bunch of key => value pairs as the attribute name and value.

@attributes([
   'class' => "mb-6",
   'disabled' => $disabled ? 'disabled' : null, // if null, won't be printed at all
   'role' => fn() => $isButton ? 'button' : null, // Lazily evaluated 
   'required',
]);

In this case, any key value pair that has a value of null would be discarded, but and singular value-only entry like required above would just be printed.

If the $value is also not a string, we could echo out JUST the key, or, if it aligns with most HTML attribute use cases, we could echo out the key again, i.e. disabled="disabled".

Known Issues

Using this on custom components at the moment appears to spit out the attributes compiled, but prevents the custom component from rendering, i'm looking into this.

github-actions · 2024-09-13T16:03:35Z

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

tpetry · 2024-09-13T16:08:15Z

I like the initial idea (mine) more 😅

Does the idea work with ...$attributes as discussed on twitter? What happens in your example when two array keys specify overlapping HTML attributes?

danmatthews · 2024-09-13T16:49:25Z

Haha, well you would I suppose!

my thought is that the second way 'feels' more laravel-like to me, but that's just a personal take obviously.

and another BUT - the @class directive works in the same way you describe, so there would be consistency there.

What happens in your example when two array keys specify overlapping HTML attributes?

In my version, i would suggest that, passing two keys that are the same would either:

Act as arrays normally do, with last-key-wins.

@attributes([
  'id' => '1',
  'id' => '2',
]); // would print id="2"

OR

If you pass a second argument as true we could merge those attribute values, but so far i can only really see that this would be useful for the class attribute though

Does the idea work with ...$attributes as discussed on twitter?

Haven't tested this yet, but i will, if we can detect that it's an instance of the ComponentAttributeBag class, then we can choose to manually parse/render it there.

timacdonald · 2024-09-14T00:56:28Z

src/Illuminate/Collections/Arr.php

+    /**
+     * Conditionally compile attributes from an array into an HTML attribute string.
+     *
+     * @param  array  $array
+     * @return string
+     */
+    public static function toHtmlAttributes($array)
+    {
+       return Collection::make($array)->map(function ($value, $key) {
+        if ($key) {
+            return $value ? sprintf('%s', $key) : null;
+        } elseif ($value instanceof ComponentAttributeBag) {
+            return collect($value->getAttributes())->map(fn ($value, $key) => sprintf('%s="%s"', $key, $value))->join(' ');
+        } else {
+            return sprintf("%s", $value);
+        }
+       })
+           ->implode(" ");
+    }
+


I would want to make sure this matches the attribute parsing we do in Vite.

<div {{ Arr::toHtmlAttributes([ 'null-value'=> null, // removed 'false-value' => false, // removed 'empty-string-value' => '', 'zero-value' => 0, 'true-value' => true, 'other-key' => 'other-value', ]) }}> // <div empty-string-value="" zero-value="0" true-value="true-value" other-key="other-value" >

timacdonald · 2024-09-14T00:57:26Z

The conditionals and the attributeName="attributeValue" syntax in the array key feels a bit weird to me and I don't feel like they match up with the @class directive.

I wonder if this could feel more at home in Laravel if we encouraged composition with the new when helper and the existing @class directive.

With the when helper:

<button @attributes([
    'wire:poll.5s' => when($condition, 'calculate'),
    'disabled' => $isDisabled,
    'data-foo' => when($foo, 'foo bar'),
])>

// ..

<form wire:poll.5s="calculate" disabled="disabled">

With the @class directive.

<form 
    @attributes([
        'wire:poll.5s' => when($condition, 'calculate'),
        'disabled' => $isDisabled,
        'data-foo' => when($foo, 'foo bar'),
    ])
    @class([
        'px-y',
        'mt-10' => $loop->first,
    ])
>

// ..

<form wire:poll.5s="calculate" disabled="disabled" class=">

Then you get lazily evaluation for free as when already supports it.

<form @attributes([
    'data-foo' => when($foo, fn () => /* ... */),
])>

tpetry · 2024-09-14T07:50:09Z

using when is a good idea I hadn't thought about. More verbose, but clearer syntax.

danmatthews · 2024-09-14T08:47:44Z

@timacdonald yeah I prefer that way as well, will re-work the PR.

Did you mean to make the directive name singular? @attribute in yours vs @attributes in ours? I know that would match the @class directive more.

johanrosenson · 2024-09-14T21:17:29Z

are boolean attributes outputted with a "true" value? They should not.

https://developer.mozilla.org/en-US/docs/Glossary/Boolean/HTML

Note: The strings "true" and "false" are invalid values. To set the attribute to false, the attribute should be omitted altogether. Though modern browsers treat any string value as true, you should not rely on that behavior.

If true the attribute should just be written without any value: <button disabled>Hi</button>

danmatthews · 2024-09-14T21:19:34Z

are boolean attributes outputted with a "true" value? They should not.

https://developer.mozilla.org/en-US/docs/Glossary/Boolean/HTML

for true at this very moment, they will indeed, but for boolean false the attribute will be omitted.

Note: The strings "true" and "false" are invalid values. To set the attribute to false, the attribute should be omitted altogether. Though modern browsers treat any string value as true, you should not rely on that behavior.

If true the attribute should just be written without any value: <button disabled>Hi</button>

Will add this in, thank you!

timacdonald · 2024-09-14T23:49:59Z

Did you mean to make the directive name singular?

My mistake. I updated my snippets to use @attributes.

are boolean attributes outputted with a "true" value? They should not.

They should match what we do for the Vite helper, i.e., attribute="attribute"

Although, the spec is inconsistent with the implementation, e.g, aria-hidden="true" is correct 🫠

johanrosenson · 2024-09-15T18:18:08Z

They should #52783 (comment), i.e., attribute="attribute"

My opinion is that they should match @disabled(), @checked and similar helpers:

framework/src/Illuminate/View/Compilers/Concerns/CompilesConditionals.php

Line 336 in da28aab

protected function compileDisabled($condition)

I know technically both are correct, but the one without a value (which are not really needed) feels way nicer, and it's the first option listed on mdn.

Although, the spec is inconsistent with the implementation, e.g, aria-hidden="true" is correct 🫠

True, i have never understood why aria booleans are treated differently compared to other booleans 🫠

…sed ones.

danmatthews · 2024-09-17T09:00:34Z

src/Illuminate/View/Compilers/ComponentTagCompiler.php

+     * @param string $expression
+     * @return array
+     */
+    public function reformatAttributeExpressionStringToArray(string $expression)


@timacdonald could use some pointers here if you don't mind?

For any @attributes call on a normal HTML element i'm just doing this - which is fairly simple.

For @attributes calls on slots and custom components, it's tough, because we can't compile into an existing attribute and prefix with : in the same way we can with @class, the attributes need to be parsed before compile time, so that they're fully parsed and passed down into the component.

BUT, this doesn't work when functions and

I'd ended up with this, which is taking the @attributes expression, parsing it using a regex, and turning it back into an array so it can be parsed properly by the new toHtmlAttributes method, but this isn't going to support things like calls to when or $variables properly.

Am i on the right track, or should i look at taking another approach? I'm trying to get my head around blade's compilation order, and the expectations of the output.

Cheers!

Hey, Dan, I don't know a heap about the Blade compiler unfortunately.

I don't imagine that the regex is gonna be the answer...but then again blade is entirely regex so maybe it is?

No worries man, i'll keep plucking at the problem, it feels like this is over-engineered to me, but will keep digging. 👍

Sounds good. Don't take my feedback as anything you must do. Just sharing an opinion.

danmatthews added 2 commits September 13, 2024 14:14

New @attributes directive/helper.

1a74123

Syntax fix.

5ab08ed

danmatthews added 3 commits September 13, 2024 18:00

Enables @attributes on custom components.

c795114

New array toHtmlAttributes helper on the Arr class.

22aa33d

WIP - compiles @attributes for components.

a4411ec

timacdonald reviewed Sep 14, 2024

View reviewed changes

Tests for Arr:: helper and refactor to use key => val structure.

216fcd1

Parsing and tests for processing string based arrays back into php ba…

2e1c3a5

…sed ones.

danmatthews commented Sep 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New @attributes blade directive. #52783

New @attributes blade directive. #52783

danmatthews commented Sep 13, 2024 •

edited

Loading

github-actions bot commented Sep 13, 2024

tpetry commented Sep 13, 2024

danmatthews commented Sep 13, 2024

timacdonald Sep 14, 2024

timacdonald commented Sep 14, 2024 •

edited

Loading

tpetry commented Sep 14, 2024

danmatthews commented Sep 14, 2024

johanrosenson commented Sep 14, 2024

danmatthews commented Sep 14, 2024

timacdonald commented Sep 14, 2024

johanrosenson commented Sep 15, 2024

danmatthews Sep 17, 2024

timacdonald Sep 18, 2024

danmatthews Sep 19, 2024

timacdonald Sep 19, 2024

New @attributes blade directive. #52783

Are you sure you want to change the base?

New @attributes blade directive. #52783

Conversation

danmatthews commented Sep 13, 2024 • edited Loading

Progress

The @attributes() blade directive.

Original PR Description

Why

Possible variation that might work better but be less readable:

Known Issues

github-actions bot commented Sep 13, 2024

tpetry commented Sep 13, 2024

danmatthews commented Sep 13, 2024

timacdonald Sep 14, 2024

Choose a reason for hiding this comment

timacdonald commented Sep 14, 2024 • edited Loading

tpetry commented Sep 14, 2024

danmatthews commented Sep 14, 2024

johanrosenson commented Sep 14, 2024

danmatthews commented Sep 14, 2024

timacdonald commented Sep 14, 2024

johanrosenson commented Sep 15, 2024

danmatthews Sep 17, 2024

Choose a reason for hiding this comment

timacdonald Sep 18, 2024

Choose a reason for hiding this comment

danmatthews Sep 19, 2024

Choose a reason for hiding this comment

timacdonald Sep 19, 2024

Choose a reason for hiding this comment

danmatthews commented Sep 13, 2024 •

edited

Loading

The `@attributes()` blade directive.

timacdonald commented Sep 14, 2024 •

edited

Loading