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

feat: API for handling locale change during page transitions #963

Merged

Conversation

pmrotule
Copy link

@pmrotule pmrotule commented Nov 15, 2020

Closes #150

The issue is happening since nuxt-i18n is setting the locale from onNavigate being called in a middleware which won't wait for the page transition to fade out. Unfortunately, this cannot be fully solved from this module since it is not possible to set a global pageTransition.beforeEnter while making sure it won't be overwritten from a transition.beforeEnter defined in a page component.

That being said, the new option skipSettingLocaleOnNavigate would allow us to workaround the issue. If it is true, the locale will not be set on navigate which would let us setting it ourselves from a beforeEnter transition hook:

// nuxt.config.js

export default {
  // ...
  plugins: ['~/plugins/router'],

  i18n: {
    // ...
    skipSettingLocaleOnNavigate: true,
  },
}
// plugins/router.js

export default ({ app }) => {
  app.nuxt.defaultTransition.beforeEnter = () => {
    app.i18n.setPendingLocale()
  }
  // Optional: wait for locale before scrolling for a smoother transition
  app.router.options.scrollBehavior = async (to, from, savedPosition) => {
    // Make sure the route has changed otherwise `beforeEnter` will never be called
    if (to.name !== from.name) await app.i18n.waitForPendingLocale()
    return savedPosition || { x: 0, y: 0 }
  }
}
// in any page component

export default {
  transition: {
    beforeEnter() {
      this.$i18n.setPendingLocale()
    },
  }
}

I'll wait for your feedback before adding test and documentation. I hope it makes sense.

Copy link
Collaborator

@rchl rchl left a comment

Choose a reason for hiding this comment

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

It's not the prettiest solution but I don't know better (better would probably require some changes in Nuxt).

So I'm fine with it for now.

src/templates/plugin.main.js Outdated Show resolved Hide resolved
src/templates/plugin.main.js Outdated Show resolved Hide resolved
@MartinMa
Copy link

MartinMa commented Dec 1, 2020

Very interesting solution.

@pmrotule Is there a way to let i18n-module call the setPendingLocale() function on its own? You mentioned beforeEnter but there can only be one beforeEnter listener I suppose? Is beforeEnter the best possible time to change the locale? When exactly is it called (it isn't actually specified in the docs)? Right before the new page begins to transition in? There would still be an edge case when the old page transitions out (say slide to the left) and the new page transitions in at the same time (say slide in from the right). In this edge case both translations should be visible at the same time. I mean, maybe we just have to live with the fact that this doesn't work.

@pmrotule
Copy link
Author

pmrotule commented Dec 3, 2020

Very interesting solution.

@pmrotule Is there a way to let i18n-module call the setPendingLocale() function on its own? You mentioned beforeEnter but there can only be one beforeEnter listener I suppose? Is beforeEnter the best possible time to change the locale? When exactly is it called (it isn't actually specified in the docs)? Right before the new page begins to transition in? There would still be an edge case when the old page transitions out (say slide to the left) and the new page transitions in at the same time (say slide in from the right). In this edge case both translations should be visible at the same time. I mean, maybe we just have to live with the fact that this doesn't work.

@MartinMa beforeEnter is indeed the exact time where you need to change it since it is called by the <transition> wrapping up your <router-view /> which differs from the router.beforeEach hook. Basically, when you call switchLocalePath, it triggers a navigation which will call up to 3 different hooks at the same time:

  • router.beforeEach will be called
  • Any global or matching middleware as well
  • The router-view <transition> will start

The problem is, nuxt-i18n is switching the locale from a middleware so it's switched right away without waiting for the transition to fade out the current page. Unfortunately, setPendingLocale() cannot be called from nuxt-i18n because it has to be called when the new page transitions in which is happening on beforeEnter, but beforeEnter can be set globally as well as from page components and in this case, there is really no way of accessing it. The only way I could see it working (like @rchl quickly mentioned) would be by adding a new config in Nuxt allowing to define a pageTransition.beforeEnter that couldn't be overwritten from a page component (something like beforeAnyRouteEnter).

By the way, sorry about the delay, I should find time in the next few days to wrap up this PR.

@pmrotule
Copy link
Author

pmrotule commented Jan 11, 2021

@rchl I finally addressed your feedback and updated the documentation. I tried to add e2e tests, but it wasn't working for me locally. Let me know how you think we should move on with this fix.

@pmrotule pmrotule requested a review from rchl January 11, 2021 15:43
@AgustinBanchio
Copy link

Looking forward to this PR

@MartinMa
Copy link

@pmrotule Thank you very much. Looks well thought out and well documented.

@pmrotule pmrotule force-pushed the fix/page-transition-on-lang-switch branch from 81b768b to cc45e66 Compare January 18, 2021 07:46
@pmrotule pmrotule changed the title Fix broken page transition with lang switcher fix: broken page transition with lang switcher Feb 2, 2021
@pmrotule pmrotule force-pushed the fix/page-transition-on-lang-switch branch from cc45e66 to 3f12610 Compare February 2, 2021 13:28
@pmrotule
Copy link
Author

pmrotule commented Feb 2, 2021

@rchl Any feedback? I just rebased again. Should be good to go.

@rchl
Copy link
Collaborator

rchl commented Feb 2, 2021

I will have a look as soon as possible (hopefully this evening).

Copy link
Collaborator

@rchl rchl left a comment

Choose a reason for hiding this comment

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

Just some small changes and comments.

docs/content/en/api.md Outdated Show resolved Hide resolved
docs/content/en/lang-switcher.md Outdated Show resolved Hide resolved
@pmrotule pmrotule requested a review from rchl February 3, 2021 07:38
@rchl
Copy link
Collaborator

rchl commented Feb 3, 2021

Great work! Thanks.

@Venegrad
Copy link

7.3 not working

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.

Page transition broken with nuxt-i18n
5 participants