Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

feat(nuxt): default router scroll behavior #3851

Merged
merged 31 commits into from Oct 19, 2022
Merged

feat(nuxt): default router scroll behavior #3851

merged 31 commits into from Oct 19, 2022

Conversation

joel-wenzel
Copy link
Contributor

@joel-wenzel joel-wenzel commented Mar 23, 2022

πŸ”— Linked issue

Discussion #3338

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

Added new hook for page:transition:finish emitted from NuxtPage. Hook fires after the vue transition afterLeave event.

Also added default scroll behavior, which can still be overridden by using app/router.options

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Mar 23, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit 023936f
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/634ff01efa0257000931f48e

@pi0 pi0 added enhancement New feature or request nuxt3 🍰 p2-nice-to-have Priority 2: nothing is broken but it's worth addressing labels Mar 23, 2022
@pi0 pi0 self-requested a review March 23, 2022 08:52
@pi0
Copy link
Member

pi0 commented Apr 22, 2022

@joel-wenzel sorry, your PR got stalled. Can you please consider rebasing it?

@posva Can you please confirm if this changes are looking good to you or any change needed?

@fanckush
Copy link

Merging this PR would fix the issue nuxt/nuxt#13284 btw

@joel-wenzel
Copy link
Contributor Author

@joel-wenzel sorry, your PR got stalled. Can you please consider rebasing it?

@pi0 No problem. Rebased. Should be all good now

@joel-wenzel
Copy link
Contributor Author

@pi0 I merged instead of rebase. If you would rather I make a new branch/PR let me know

@bart
Copy link

bart commented Jun 1, 2022

When will it be merged? Current Nuxt 3 page link behaviour is counter-intuitive because page sticks on current position of previous page.

Update 2022-06-04:
When playing around with Content Wind I noticed that scroll behaviour was as expected. So I checked for a custom router configuration et voila:

import type { RouterConfig } from '@nuxt/schema'

// https://router.vuejs.org/api/#routeroptions
export default <RouterConfig>{
  scrollBehavior (to, _, savedPosition) {
    const nuxtApp = useNuxtApp()

    // If history back
    if (savedPosition) {
      // Handle Suspense resolution
      return new Promise((resolve) => {
        nuxtApp.hooks.hookOnce('page:finish', () => {
          setTimeout(() => resolve(savedPosition), 50)
        })
      })
    }
    // Scroll to heading on click
    if (to.hash) {
      setTimeout(() => {
        const heading = document.querySelector(to.hash) as any

        return window.scrollTo({
          top: heading.offsetTop,
          behavior: 'smooth'
        })
      })
      return
    }

    // Scroll to top of window
    return { top: 0 }
  }
}

This might also solve your problems with latest plain Nuxt 3 apps.

@fanckush

This comment was marked as duplicate.

@Rigo-m
Copy link
Contributor

Rigo-m commented Jun 16, 2022

When will it be merged? Current Nuxt 3 page link behaviour is counter-intuitive because page sticks on current position of previous page.

Update 2022-06-04: When playing around with Content Wind I noticed that scroll behaviour was as expected. So I checked for a custom router configuration et voila:

import type { RouterConfig } from '@nuxt/schema'

// https://router.vuejs.org/api/#routeroptions
export default <RouterConfig>{
  scrollBehavior (to, _, savedPosition) {
    const nuxtApp = useNuxtApp()

    // If history back
    if (savedPosition) {
      // Handle Suspense resolution
      return new Promise((resolve) => {
        nuxtApp.hooks.hookOnce('page:finish', () => {
          setTimeout(() => resolve(savedPosition), 50)
        })
      })
    }
    // Scroll to heading on click
    if (to.hash) {
      setTimeout(() => {
        const heading = document.querySelector(to.hash) as any

        return window.scrollTo({
          top: heading.offsetTop,
          behavior: 'smooth'
        })
      })
      return
    }

    // Scroll to top of window
    return { top: 0 }
  }
}

This might also solve your problems with latest plain Nuxt 3 apps.

The problem of this, at least in content-wind, is that the scroll starts when the next page has yet to be mounted

@axelthat
Copy link

Any updates?

@joel-wenzel
Copy link
Contributor Author

Any updates?

I am waiting until I hear from the core team before fixing merge conflicts. I have fixed them 3 or 4 times now only to become outdated again

@pi0
Copy link
Member

pi0 commented Jun 22, 2022

Hi @joel-wenzel. Sorry, it took long on your pull request. I was mainly waiting for @posva's take about the possibility of moving this to the vue-router core. If you can rebase your PR I can locally test and merge to iterate.

@pi0 pi0 removed their assignment Oct 19, 2022
onResolve: () => nuxtApp.callHook('page:finish', routeProps.Component).finally(done)
}, { default: () => h(Component, { key, routeProps, pageKey: key, hasTransition: !!transitionProps } as {}) })
onResolve: () => {
nextTick(() => nuxtApp.callHook('page:finish', routeProps.Component).finally(done))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up @danielroe we are calling page:finish in nextTick by this PR

@pi0
Copy link
Member

pi0 commented Oct 19, 2022

Thank you so much for working on this @joel-wenzel and sorry it took long to land!

I've made a few refactors and fixes on top of your work.

@lamdinh95
Copy link

Hello @pi0, I've updated to rc.13 today but the scroll behaviour do not work correctly. When I navigate back, the scroll doesn't go to the save position but it's reset instead.
The problem cause by this line in file: packages/nuxt/src/pages/runtime/router.options.ts
const hasTransition = to.meta.pageTransition !== false && from.meta.pageTransition !== false;
to.meta.pageTransition was undefined so the strict equal not work in here.

@danielroe
Copy link
Member

This should be resolved in the edge channel, or in the next RC.

Let me know if not.

@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x enhancement New feature or request nuxt3 🍰 p2-nice-to-have Priority 2: nothing is broken but it's worth addressing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet