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

Ensure Inertia pages are children of AppLayout #1517

Draft
wants to merge 1 commit into
base: 5.x
Choose a base branch
from

Conversation

mst101
Copy link

@mst101 mst101 commented Jul 15, 2024

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

  • resources/js/Pages/API/Index.vue
  • resources/js/Pages/Dashboard.vue
  • resources/js/Pages/Profile/Show.vue
  • resources/js/Pages/Teams/Create.vue
  • resources/js/Pages/Teams/Show.vue

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
Copy link
Member

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?

@taylorotwell taylorotwell marked this pull request as draft July 15, 2024 22:02
@mst101
Copy link
Author

mst101 commented Jul 15, 2024

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.

@driesvints driesvints marked this pull request as ready for review July 22, 2024 08:11
@taylorotwell
Copy link
Member

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 taylorotwell marked this pull request as draft July 26, 2024 06:17
@taylorotwell
Copy link
Member

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

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.

2 participants