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

[11.x] Always inherit parent attributes #53011

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Conversation

royduin
Copy link
Contributor

@royduin royduin commented Oct 2, 2024

This makes sure the functionality added with #32576 is always applied. This way it also works with slots:

<x-something>
    <x-slot:heading :attributes="$heading->attributes->class('bg-red-500')">
        {{ $heading }}
    </x-slot:heading>
    Content
</x-something>

That's handled here: https://github.com/laravel/framework/blob/11.x/src/Illuminate/View/Concerns/ManagesComponents.php#L185:L198, but the attributes are passed through the constructor so they currently don't go through setAttributes(). When you do this now you'll end up with:

<div attributes="class=\"bg-red-500\"">Heading</div>

@taylorotwell
Copy link
Member

Could you add tests?

@taylorotwell taylorotwell marked this pull request as draft October 2, 2024 14:41
@royduin
Copy link
Contributor Author

royduin commented Oct 2, 2024

I'll do tomorrow! Thanks for reviewing.

@royduin royduin marked this pull request as ready for review October 3, 2024 09:51
@taylorotwell
Copy link
Member

taylorotwell commented Oct 3, 2024

I'm worried this might break some existing applications. Can you explain the issue more?

Please mark as ready for review after commenting.

@taylorotwell taylorotwell marked this pull request as draft October 3, 2024 17:47
@royduin
Copy link
Contributor Author

royduin commented Oct 4, 2024

With #32576 it's made possible to passthrough attributes, let's stick with the example from there. With this "base button" component:

<!-- button.blade.php -->
<button {{ $attributes->merge(['type' => 'button', 'class' => 'p-3 rounded']) }}>
    {{ $slot }}
</button>

This is now possible:

<!-- button-danger.blade.php -->
<x-button class="bg-red-200" :attributes="$attributes">
    {{ $slot }}
</x-button>

Or with {{ $attributes }} or :$attributes. This can be used like this where everything is passed through:

<x-button-danger type="submit" class="mt-5">Delete</x-button-danger>

And the output:

<button type="submit" class="p-3 rounded bg-red-200 mt-5">
    Delete
</button>

With this PR this is also going to work with slots, let's create a card component with a heading:

<!-- card.blade.php -->
<div {{ $attributes->class('p-5 rounded bg-gray-200') }}>
    <div {{ $heading->attributes->class('border-b font-bold') }}>
         {{ $heading }}
    </div>
    {{ $slot }}
</div>

And a variant on that where we just like to add some extra attributes:

<!-- card-variant.blade.php -->
<x-card class="border-4 border-gray-500" :$attributes>
    <x-slot:heading :attributes="$heading->attributes->class('bg-green-200')">
        Heading!
    </x-slot>
    {{ $slot }}
</x-card>

And when we use that we still like to have the ability to add even more attributes:

<x-card-variant class="mt-5">
    <x-slot:heading class="font-green-500">
        Heading!
    </x-slot>
    Content
</x-card-variant>

Currently you'll end up with:

<div class="p-5 rounded bg-gray-200 border-4 border-gray-500 mt-5">
    <div class="border-b font-bold" attributes="class=\"bg-green-200 font-green-500\"">
         Heading!
    </div>
    Content
</div>

With this PR you'll get the expected result:

<div class="p-5 rounded bg-gray-200 border-4 border-gray-500 mt-5">
    <div class="border-b font-bold bg-green-200 font-green-500">
         Heading!
    </div>
    Content
</div>

I don't see where this could break; the only change is that slot attributes will go through the same method: https://github.com/laravel/framework/blob/d9ecee50e451c948f13ccc91a24e1a8345334f03/src/Illuminate/View/ComponentAttributeBag.php#L380:L392 that "extracts the attributes attribute".

@royduin royduin marked this pull request as ready for review October 4, 2024 08:07
@taylorotwell taylorotwell merged commit d9bb513 into laravel:11.x Oct 4, 2024
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants