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

[10.x] Introduce short-hand "false" syntax for Blade component props #48084

Merged
merged 1 commit into from Aug 27, 2023
Merged

[10.x] Introduce short-hand "false" syntax for Blade component props #48084

merged 1 commit into from Aug 27, 2023

Conversation

ryangjchandler
Copy link
Contributor

Currently, setting the value of a Blade component prop to false can only be done with the binding syntax.

<x-layout :margin="false">

This syntax is quite verbose for something so simple, so this pull request introduces a new short-hand syntax.

<x-layout !margin />

The syntax is similar to the "not" operation in regular PHP code and feels quite familiar.

Not sure whether "short false" syntax is the best name for this, open to suggestions.

@laserhybiz
Copy link

this seems to me more similar to css !important (or tailwinds important modifier)

@driesvints
Copy link
Member

Smart!

@Rizky92
Copy link

Rizky92 commented Aug 17, 2023

I support this implementation. "Falsy" props were one of our concerns when making blade components. We have to work around this by making every boolean props to be false, and add them manually when the component is needed.

This PR allowed us to refactor some of those falsies to truthy by default.

@julien-fontaine
Copy link

@ryangjchandler Why don't you use the short-hand:

<x-layout />

It's better to let the user add the behavior they need, rather than forcing them to remove a default one.

@taylorotwell taylorotwell merged commit 901ca01 into laravel:10.x Aug 27, 2023
21 checks passed
@aguingand
Copy link
Contributor

@taylorotwel @driesvints This will fail to parse in some cases, e.g:

<x-test :value="$value && !old('value')" />

@driesvints
Copy link
Member

@ryangjchandler

@ryangjchandler
Copy link
Contributor Author

ryangjchandler commented Aug 28, 2023

I'm on vacation until Wednesday, maybe worth reverting the merge before tomorrows release so I can take a look at it & add in some test cases.

@aguingand have you physically confirmed that the RegEx currently parses incorrectly for that example case?

@driesvints
Copy link
Member

Done: #48220

taylorotwell pushed a commit that referenced this pull request Aug 28, 2023
@aguingand
Copy link
Contributor

aguingand commented Aug 28, 2023

I understand the reasoning behind short syntaxes but it'll need a rework of how attributes are parsed. The :$aaa syntax can also have false-positive in the value but it's less likely

<x-test :value="$value ? $yes :$no" />

@ryangjchandler
Copy link
Contributor Author

FYI, I am going to revisit this in the week. Been slammed with work since getting back from vacation!

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.

None yet

7 participants