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

[8.x] Fix appendable attributes in Blade components #35131

Merged
merged 8 commits into from
Nov 6, 2020
Merged

[8.x] Fix appendable attributes in Blade components #35131

merged 8 commits into from
Nov 6, 2020

Conversation

panda-madness
Copy link
Contributor

@panda-madness panda-madness commented Nov 6, 2020

Currently, attempting to render a component which prepends a non-class attribute in it's template without actually overriding it in the calling template yields an Exception.

For example, given the following component called test-component:

<div {{ $attributes->merge(['data-controller' => $attributes->prepends('profile-controller')]) }}>
    {{ $slot }}
</div>

Rendering this component like so: <x-test-component data-controller="some-controller" /> works as expected.

Rendering this component like so: <x-test-component /> yields the following error:

Screen Shot 2020-11-06 at 13 59 09

This PR fixes this behavior. <x-test-component /> will now properly render as

<div data-controller="profile-controller"></div>

See #35128 for more details.

@driesvints driesvints changed the title Fix appendable attributes in Blade components [8.x] Fix appendable attributes in Blade components Nov 6, 2020
@driesvints
Copy link
Member

@panda-madness it's also best that you provide a thorough explanation in your PR description (see our PR templates) so Taylor knows what this is about and why it's needed. Don't just link to an issue.

@panda-madness
Copy link
Contributor Author

@driesvints I've updated the PR description.

I've also modified the test to include this use case, but I'm not quite sure why an unrelated test started failing...

@driesvints
Copy link
Member

@panda-madness your whitespace change from here seems to be the culprit: 8b2e529#diff-9e28a6d9f9f352efc3960eab4ff2d081a25459d765d3f1ac2c9735dd4356dd50L45

Think I got it now.

@panda-madness
Copy link
Contributor Author

This is a little nitpicky, but should we change the AppendableAttributeValue class name to PrependableAttributeValue so it maches the method name better?

@taylorotwell taylorotwell merged commit 267b8cd into laravel:8.x Nov 6, 2020
@driesvints
Copy link
Member

@panda-madness probably not worth the breaking change.

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.

3 participants