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

Customize blade directive to allow changing the div id #186

Closed

Conversation

RobertBoes
Copy link
Contributor

While reading through #141 and #178 I noticed the $expression that's passed by the blade compiler wasn't treated as a PHP expression, but rather as a string. I think this should be treated as an expression to follow blade standards and this will allow people to conditionally change the id like so @inertia(true ? 'foo' : 'bar').

This means @inertia(foo) is not allowed, since that's not a valid expression. However, all directives mentioned in the blade docs have valid PHP as parameter for the blade directives ('title', count($records) === 1) and for example yield(content) wouldn't work either since that doesn't contain a valid PHP expression.

The easiest solution is to output the expression in the directive directly to pass that to a method call, so the directive would return this '<?php echo app(\MyClass::class)->render('.$expression.'); ?>'. This works as expected, unfortunately Inertia also needs to output the $page variable which is passed as view data. I've tried to come up with a solution for this, but this is far from perfect, since I don't really like the usage of an anonymous class for this. Maybe someone can help me figure out a better solution, or this PR can be used to improve one of the other PR's

Oh and I used @claudiodekker's tests from #141, so I added you as co-author :)

@claudiodekker claudiodekker requested a review from reinink December 3, 2020 12:17
@claudiodekker claudiodekker added the enhancement New feature or request label Dec 3, 2020
@claudiodekker
Copy link
Member

I'll be closing this PR, as we've actually already re-implemented this as part of #339 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Closed 🚪
Development

Successfully merging this pull request may close these issues.

2 participants