laravel / jetstream

Tailwind scaffolding for the Laravel framework.
https://jetstream.laravel.com
MIT License
3.98k stars 822 forks source link

Ensure Inertia pages are children of AppLayout #1517

Open mst101 opened 4 months ago

mst101 commented 4 months ago

I noticed that the AppLayout.vue component is currently a child of the following pages:

This means the whole layout is refreshed when clicking between these pages. Not only is this inefficient, but any custom code in the navigation bar (e.g. a search input field) will have its content reset.

This PR solves this problem by providing a wrapper around AppLayout and makes use of the defineOptions macro:

defineOptions({
    layout: AppLayout,
})

... to ensure that the above vue components are children of AppLayout.vue, not the other way around.

taylorotwell commented 4 months ago

I'm not sure I follow. How is AppLayout a child of Teams/Show when it's the root component of the page and everything else is contained within it?

mst101 commented 4 months ago

If you have a look at Vue DevTools, this shows the current hierarchy of components:

image

This PR adjusts things, like so:

image

If you check out this repo, you'll see why this could be an issue: https://github.com/mst101/jetstream-bug

e.g. Imagine you're on the Dashboard page and you write some text in an input field in the nav. You then click on the 'Profile' page... and find that the input text you entered has been overwritten (because the site has reloaded AppLayout.vue).

That's perfectly normal behaviour for a traditional server-side app of course. But it needn't be for an SPA.

taylorotwell commented 4 months ago

A point of feedback from creator of Inertia:

I find the PageContainer component name a bit weird. I would have just called this PageHeader and not included all the page content within it.

taylorotwell commented 4 months ago

Please mark as ready for review when the requested changes have been made.