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] Allow component slots to be empty, even if they have an html comment or line break #49935

Closed
wants to merge 7 commits into from

Conversation

comes
Copy link
Contributor

@comes comes commented Jan 31, 2024

This PR addresses and solves an issue with Livewire V3 morph markers.

Since Livewire is injecting morph markers, we cannot safely ask if a slot is empty or not. The same happens if we add a linebreak or nonprintable characters like whitespaces or tabs. The slot is not empty and should be rendered.

From a technical point of view, the slot is not empty, the blade compiler must render this.
But from a 'what should happen in the template' point of view, the slot might be empty, and the HTML should contain something that occurs in the other condition.

here is an example. Given is a component table.blade.php

01    <table>
02    @if($slot->isNotEmpty())
03        {{ $slot }}
04    @else
05        <tr><td>{{ __('No Results') }}</td></tr>
06    @endif
07    </table>

In the view, we use this <x-table /> and do a loop. The @foreach() is an empty result set; I expect that we see No Results.

another blade file

01    <x-table>
02    <!-- start loop -->
03    @foreach([] as $value)
04        <tr><td>{{ $value }}</td></tr>
05    @endforeach
06    <!-- end loop -->
07    </x-table>

The slot content starts with <!-- in line 2 and ends with --> in line 6. If the loop is empty, our slot content is <!-- start loop --><!-- end loop -->. Should we render it, or is the slot empty?

The Filament package has a is_slot_empty helper, which addresses this issue.

Shouldn't we bring this into the Framework to make slots a bit more intelligent?

@comes comes changed the title Allow component slots to be empty, even if they have an html comment or line break. [10.x] Allow component slots to be empty, even if they have an html comment or line break. Jan 31, 2024
@driesvints driesvints changed the title [10.x] Allow component slots to be empty, even if they have an html comment or line break. [10.x] Allow component slots to be empty, even if they have an html comment or line break Feb 1, 2024
@taylorotwell
Copy link
Member

taylorotwell commented Feb 1, 2024

Can we just make an extra method that is like willRenderContent or something so there is no breaking changes?

@devajmeireles
Copy link
Contributor

When I looked at this pull request I wondered if this was really useful, because there may be cases where we want to pass the comment to the slot.

Now that Taylor commented on the possible breaking change, perhaps an auxiliary method would be interesting, as he said, or perhaps even a method that activates this cleaning behavior.

If Taylor's idea goes forward, I suggest that the helper method be isReallyEmpty, or if there is a helper method to activate cleaning within the current isEmpty then we could use something like sanitizeContent offering by default the regex entered here

@taylorotwell taylorotwell marked this pull request as draft February 2, 2024 16:45
@taylorotwell
Copy link
Member

Drafting until updated. Please mark as ready for review if you want me to take another look.

@comes
Copy link
Contributor Author

comes commented Feb 2, 2024

Hej @taylorotwell,

thanks for your review and input. Are you ok with something like $slot->sanitze()->isEmpty(), where sanitize provides a callback for the isEmpty() method?

I modified this PR and provided a sanitize() function.
This function has an optional callable or string parameter.

public function sanitize(null|string|callable $callable = null)
{
    if (is_string($callable) && ! function_exists($callable)) {
        throw new \InvalidArgumentException("Callable does not exist.");
    }

    $this->sanitizerResolver =
        $callable ??
        fn ($input) => trim(preg_replace("/<!--([\s\S]*?)-->/", '', $input)); // replace everything between <!-- and --> with empty string

    return $this;
}

Picking up my example above, we now have different options:

The well-known behavior:

@if($slot->isEmpty())
...
@endif

The HTML sanitizer removes all <!-- ... --> from the slot contents before checking if it's empty.

@if($slot->sanitize()->isEmpty())
...
@endif

If, for whatever reason, someone loves to have it a bit more flexible, the sanitize function allows a callable or function name.

@if($slot->sanitize('trim')->isEmpty())
...
@endif

This approach is a bit more flexible than a willRenderContent function.

If you think this is still a problem, I'm open to creating a willRenderContent with the same behavior of having a flexible callable as input.

No matter which solution wins, I think it will make things easier for laravel developers.

@devajmeireles
Copy link
Contributor

willRenderContent

Most likely he will prefer a helper method such as willRenderContent, which he has suggested. My suggestion is isReallyEmpty

@comes
Copy link
Contributor Author

comes commented Feb 3, 2024

I created another PR #49966, an alternate implementation of the same problem. It is a single-function solution sanitizedEmpty() and sanitizedNotEmpty().

Please decide which one fits better in the framework.

@comes comes marked this pull request as ready for review February 3, 2024 15:16
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