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 merging boolean or null attributes in Blade components #32245

Merged
merged 3 commits into from
Apr 6, 2020

Conversation

sileence
Copy link
Contributor

@sileence sileence commented Apr 5, 2020

The fix to escape merged attributes with e creates an unexpected behaviour when merging attributes with null or boolean values, for example:

        $bag = new ComponentAttributeBag([]);
        
        echo $bag->merge([
            'required' => false,
            'readonly' => null,
            'disabled' => true,
        ]));

Will print required="" readonly="" disabled="1" instead of disabled="disabled"

This is also inconsistent with the result that we'd get if we create a new instance of ComponentAttributeBag passing the same array as the first argument of the constructor:

new ComponentAttributeBag([
    'required' => false,
    'readonly' => null,
    'disabled' => true,
])

Will print disabled="disabled" and I think this is the right result.

Therefore I'm submitting this PR to avoid escaping boolean or null values so we get the same result in both cases.

@taylorotwell taylorotwell merged commit ff1a13e into laravel:7.x Apr 6, 2020
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