ideas icon indicating copy to clipboard operation
ideas copied to clipboard

Can't set default component attributes from class

Open ryangjchandler opened this issue 5 years ago • 5 comments

  • Laravel Version: 7.x
  • PHP Version: Irrelevant
  • Database Driver & Version: Irrelevant

Description:

Opening this issue after I broke the initial 7.x release 😢 . The fix that I had put in place would successfully pass the attributes that were being added from the component class constructor, but it would not successfully merge the attributes coming from the actual component tag itself.

Steps To Reproduce:

Component class

class Test extends Component
{
    public function __construct()
    {
        $this->withAttributes(['class' => 'bg-red-500 px-4']);
    }

    public function render()
    {
        return view('components.test');
    }
}

Component view

<div {{ $attributes }}>
    Testing
</div>

Parent view

<x-test class="border border-black py-4" />

The expected HTML would be:

<div class="bg-red-500 px-4 border border-black py-4">
    Testing
</div>

but the actual output is:

<div class="bg-red-500 px-4">
    Testing
</div>

Investigation

Whilst investigating this, I followed the problem down to the compiled Blade file. I believe the changes from my PR laravel/framework#31676 were not working correctly as the $component->withAttributes() call was taking place after the $component->data() call, so the compiler was not performing the merge at all.

Would like to know what others think. I have a few ideas for fixes that work now that I've written some decent tests, but I'm not sure if we should just leave this and instead tell users to invoke the $attributes variable with their default values instead.

ryangjchandler avatar Mar 03 '20 18:03 ryangjchandler

Moving this to the ideas repo as it's more a feature request. Feel free to attempt a new pr with tests once you've figured thinks out.

driesvints avatar Mar 03 '20 19:03 driesvints

@driesvints, if you wouldn't mind opening my eyes to the following :)

is this the intended behaviour

$img->withAttributes(['foo' => 'boo']);
$img->withAttributes(['bar' => 'car']);
dd($img->attributes);
// EXPECTED: foo&bar
// GOT: only bar

// ----------------------------------

$img = app()->make(Img::class, ['src' => 'some-image.jpg']);
$img->withAttributes(['foo' => 'boo']);

// presumably, this functionality is for component view, because it
// returns new ComponentAttributeBag
$img->attributes->merge(['bar' => 'car']);

dd($img->attributes);
// EXPECTED: foo&bar
// GOT: only foo

What I've got from this is that withAttributes basically sets/resets the whole ComponentAttributeBag. Tested on Laravel version 7.1.0

The solution would be to replace

public function setAttributes(array $attributes) 
{
    $this->attributes += $attributes;
}

in ComponentAttributeBag:line 138

But I don't know if that would break the logic/vision of current implementation.

matijakovacevic avatar Mar 12 '20 15:03 matijakovacevic

The withAttributes() function completely overwrites the attributes on the component. I think it should be changed to merge them instead but I imagine this might be considered a breaking change now.

ryangjchandler avatar Mar 12 '20 15:03 ryangjchandler

I assumed that withAttributes would merge attributes. I was able to access the merge method like this:

For me, $this->attributes is always empty on this line:

https://github.com/laravel/framework/pull/31676/files#diff-1005052c893d685dfa5b9c5c7431e2a880260f9a16cba75e16689ec33549755cR215

So there is always a new ComponentAttributeBag

And even when I call withAttributes() they are still not passed to the view, eg.

testing.blade.php

attributes: <br>{{ $attributes }}

Testing.php

<?php
class Testing extends Component
{
    public function render()
    {
        $this->withAttributes(['new' => 'attr']);
        return view('components.testing');
    }
}

Actual when rendered:

attributes:

Expected when rendered:

attributes: new="attr"

Note:

If I call the component like:

<x-testing hello="world"></x-testing>

Then the output is:

attributes: hello="world"

ianjamieson avatar Jun 11 '21 13:06 ianjamieson

I thought that perhaps this, https://laravel.com/docs/8.x/blade#using-attributes-slots-within-component-class

Would give access to the $attributes object, which is does:

<?php
class Testing extends Component
{
    public function render()
    {
        return function (array $data) {
            $data['attributes'] = $data['attributes']->merge(['foo' => 'bar']);
            return 'components.testing';
        };
    }
}

The above works if you print the attributes in that render. However, the rendered view remains unaffected.

I am able to overwrite or set attributes not merge, if I do this:

<?php
class Testing extends Component
{
    public function render()
    {
        return function (array $data) {
            $data['attributes']['foo'] = 'bar';
            return 'components.testing';
        };
    }
}

ianjamieson avatar Jun 11 '21 14:06 ianjamieson