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

feat(nuxt): default router scroll behavior#3851

Merged
pi0 merged 31 commits intonuxt:mainfrom
joel-wenzel:feature/scroll-behavior
Oct 19, 2022
Merged

feat(nuxt): default router scroll behavior#3851
pi0 merged 31 commits intonuxt:mainfrom
joel-wenzel:feature/scroll-behavior

Conversation

@joel-wenzel
Copy link
Copy Markdown
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
Copy Markdown

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
Comment thread packages/nuxt3/src/pages/routing/default-router.options.ts Outdated
Comment thread packages/nuxt3/src/pages/routing/default-router.options.ts Outdated
Comment thread packages/nuxt3/src/pages/routing/default-router.options.ts Outdated
Comment thread packages/nuxt3/src/pages/runtime/page.ts Outdated
@pi0
Copy link
Copy Markdown
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
Copy Markdown

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

Comment thread packages/nuxt3/src/pages/routing/default-router.options.ts Outdated
@joel-wenzel
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

@bart
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown

Any updates?

@joel-wenzel
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread packages/nuxt/src/pages/routing/default-router.options.ts Outdated
@pi0 pi0 removed their assignment Oct 19, 2022
Comment thread packages/nuxt/src/pages/runtime/page.ts Outdated
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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.