isBackRoute results in unintuitive scroll behaviour

bloodf commented 3 years ago

From vue-storefront created by christian-reichart: vuestorefront/vue-storefront#5215

Current behavior

isBackRoute is true if the user visited the target page before, not only if browser back is used. Because the scroll behaviour is attached to this flag, it results in scrolling to the position you left target page, even if you navigate to it normally (not through browser back or forward). This feels very unintuitive, since scrolling back to a certain position should only happen if you use the native browser navigation.

Expected behavior

The jump to the saved position should only happen if native browser navigation is used, not if navigation happens through the site.

I tried using the vue router scrollbehaviour like this:

    scrollBehavior: (to, from, savedPosition) => {
      console.log('$ saved position: ', savedPosition);
      if (to.hash) {
        return {
          selector: to.hash
        }
      }
      if (rootStore.getters['url/isBackRoute']) {
        const { scrollPosition = { x: 0, y: 0 } } = rootStore.getters['url/getCurrentRoute']
        return scrollPosition
      } else if (to.path !== from.path) { // do not change scroll position when navigating on the same page (ex. change filters)
        return { x: 0, y: 0 }
      }
    }

however, savedPosition seems to always be null and I can't figure out why.

Steps to reproduce the issue

Behaviour in steps: -> Visit PLP -> Visit PDP of product A -> Navigating through the site (different PLPs, PDPs, etc.) -> Navigating back to PDP product A (through the actual site, not browser history) -> You are back at the scroll position you left the page some time ago

Version of Vue Storefront

[x] Vue Storefront
[ ] Vue Storefront Next

Can you handle fixing this bug by yourself?

[ ] YES
[x] NO

Which Release Cycle state this refers to? Info for developer. (doesn't apply to Next)

Pick one option.

[ ] This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
[ ] This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
[x] This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.

Environment details

Can be reproduced on the demo store https://demo.vuestorefront.io/ in Firefox and Chrome on Mac

Additional information

Maybe consider renaming 'isBackRoute' to 'isVisitedRoute' or something like that. :)

bloodf commented 3 years ago

Interesting.... I've also noticed this behavior. @christian-reichart Did you come up with a working solution? I found this suggestion that looks promising, but have not tested it or looked into how it could be implemented in VSF. https://stackoverflow.com/questions/51980296/detect-back-button-in-navigation-guards-of-vue-router/65242660#65242660 Seems like a pretty straight forward approach though.

bloodf commented 3 years ago

Any update regarding this issue?

bloodf commented 3 years ago

Also tried to solve it like this:

    window.addEventListener('popstate', () => {
      console.log('$ pop state detected!');
    });

however, this listener seems to always trigger after the router guards.

bloodf commented 3 years ago

@didkan I currently have not found a solution. Thanks for the link, I may look into this again if I find the time. Also if you find out anything let me know. :)

vuestorefront / vue-storefront-1