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

Allow the use of layout as a Component #1807

Closed
wants to merge 3 commits into from
Closed

Allow the use of layout as a Component #1807

wants to merge 3 commits into from

Conversation

RomainMazB
Copy link
Contributor

1️⃣ Is this something that is wanted/needed? Did you create an issue / discussion about it first?
I did not create an issue, but it's probably related with this one: #1748

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
No.

3️⃣ Does it include tests, if possible? (Not a deal-breaker, just a nice-to-have)
No.

4️⃣ Please include a thorough description of the improvement and reasons why it's useful.
When using layout as a Component, like it's done in Jetstream (as an example) for the AppLayout and GuestLayout, we are able to define some "complex data" we don't want to put in our blade file:

class AppLayout extends Component
{
    public bool $canCreateCharacter;

    public function __construct()
    {
        $user = Auth::user();

        $userCharacters = $user->characters->map(
            static fn ($character) => $character->id
        );

        // Retrieve the world not full where the user doesn't have a character yet
        $this->canCreateCharacter = World::whereDoesntHave('characters',
                static fn (Builder $query) =>
                $query->whereIn('id', $userCharacters)
            )
                ->subscriptionAvailable()
                ->count() > 0;
    }

    public function render()
    {
        return view('layouts.app');
    }
}

But since our blade file is strongly attached to our Component class, some errors appear when we are trying to render our Livewire components using its class directly:

Route::get('/', 'Livewire\\Messenger\\Messenger')->name('messenger.index');

ErrorException
Undefined variable: canCreateCharacter (View: /home/maz/websites/terrajdr/resources/views/layouts/app.blade.php) (View: /home/maz/websites/terrajdr/resources/views/layouts/app.blade.php)

As mentioned in a thread on the Livewire forum, I've succeed into resolving it using base Livewire methods by manually instantiate the AppLayout class and passing its data as parameters:

// Somewhere in my Livewire Component class
    public function render()
    {
        $view = view('livewire.messenger.index')->layout('layouts.app', (new AppLayout())->data());
    }

But manually add this code, referencing the default layout and passing its data manually is totally boring and we don't want that.

So I've dive into the Livewire code and searched why I needed to do that.
I finally understood that Livewire do not use x-blade component syntax, which is (I suppose) the source of the problem.

First I tried to search for a method to programmatically know if x-component was used by the layout file, but didn't find one (open to suggestion).
Then I figured out that I just needed to create a view file to render the layouts that are extending Component class
I've made some tests, using a x-blade specific layout to render my components and it worked perfectly well.

Since x-blade syntax is not natively supported by Laravel 7 which is supported by Livewire and assuming the fact that most of us don't use Blade Component to render layouts, I propose to just set a config boolean to change the default behavior to use this feature.

I've created the viewMacro method to change the default layout and pass data to it the same way we use ->layout() or ->extend(). You can just use ->xLayout('custom-layout', ['parameter' => 'custom data'])

Doing so, we can use basic feature for some layout, and xLayout to render specific Component extending layout.

I'm totally open to suggestion to upgrade my code, I think this feature in my implementation-style or another should be merged in near future.

5️⃣ Thanks @calebporzio for providing this good package! 🙌

@calebporzio
Copy link
Collaborator

Thank you for the PR, however I don't think this is the right way forward.

I could see wanting to pass a Blade component class into ->layout() and I would be open to that, but none of the extra config-related or "x-layout" nomenclature stuff.

Thanks!

@RomainMazB
Copy link
Contributor Author

RomainMazB commented Oct 21, 2020

Thank you for the PR, however I don't think this is the right way forward.

I could see wanting to pass a Blade component class into ->layout() and I would be open to that, but none of the extra config-related or "x-layout" nomenclature stuff.

Thanks!

I will review my code to allow ->layout() to accept a Blade component class in parameter and parse it with the special livewire-view-x-layout.blade.php (this part is OK I suppose, because I don't see any way to handle without this ? @calebporzio ).

This is I agree a proper way.

@RomainMazB
Copy link
Contributor Author

On the other hand, after thinking about it. Using the native ->layout() will force the programmer to pass his default layout into each component.

// First component
class ShowPosts extends Component
{
    ...
    public function render()
    {
        return view('livewire.show-posts')
            ->layout(AppLayout::class);
    }
}


// Second component
class ShowComments extends Component
{
    ...
    public function render()
    {
        return view('livewire.show-posts')
            ->layout(AppLayout::class);
    }
}


// Third component
class ShowRates extends Component
{
    ...
    public function render()
    {
        return view('livewire.show-posts')
            ->layout(AppLayout::class);
    }
}

// And so on ...

Maybe a config key to set the default layout may be useful in this case and could provide both "x-layout" or basic layout (if the file of the default layout is not layouts/app.blade.php)

@rjdjohnston
Copy link

would be great if this was merged

@RomainMazB
Copy link
Contributor Author

@rjdjohnston A new PR is in progress here #2499 , stay tuned ;)

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

3 participants