Skip to content
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

[7.x] Fix output of component attributes #31994

Merged
merged 2 commits into from Mar 17, 2020
Merged

Conversation

@perifer
Copy link
Contributor

perifer commented Mar 16, 2020

Closes #31939

This PR fixes multiple issues with how component attributes are rendered. $attribute and $attributes->merge() results in different rendered attributes for values that are true or casts to false. For example min="0" is currently removed when using merge() (see full example below).

Here is an attempt to summarise the changes. In short $attribute and $attribute->merge() now have identical output and are rendered the same way as Vue.

<x-input :checked="false">, <x-input :checked="null">, <x-input :checked=""><input>
Previously <input checked=""> and <input>

<x-input :min="0">, <x-input :min="'0'"><input min="0">
Previously <input min="0"> and <input>

<x-input :checked="true"><input checked="checked">
Previously <input checked> and <input checked="checked">

This last example is handled a bit simpler than Vue. Vue differentiate boolean attributes (checked, disabled, itemscope etc) and normal attributes. This PR doesn't (but it could be added in another PR).

An example where min="0" is removed when using merge()

A blade component with attributes:

<input type="number" {{ $attributes }} />

For an input with type number it's reasonable to have min="0":

<x-number min="0" max="10" class="p-4" />

This render as:

<input type="number" min="0" max="10" class="p-4" />

But if you used merge():

<input type="number" {{ $attributes->merge(['class' => 'm-4']) }} />

The attribute with value 0 is removed:

<input type="number" max="10" class="p-4 m-4" />

Another example with an attribute with a value false that is rendered differently when using with or without merge()

With a component that simply prints it's attributes:

<input {{ $attributes }} />

If the component is used like this:

<x-input :disabled="false" />

We would expect the rendered template to be:

<input />

But the current implementation renders:

<input disabled="" />

But if the component uses merge() like this:

<input {{ $attributes->merge() }} />

The output is what one would expect:

<input />
@taylorotwell taylorotwell merged commit f0f7dfa into laravel:7.x Mar 17, 2020
7 checks passed
7 checks passed
P7.2 - Sprefer-lowest
Details
P7.2 - Sprefer-stable
Details
P7.3 - Sprefer-lowest
Details
P7.3 - Sprefer-stable
Details
P7.4 - Sprefer-lowest
Details
P7.4 - Sprefer-stable
Details
continuous-integration/styleci/pr The analysis has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.