fix(scroll): use location.key for scroll behaviours (#12403)

## Description This is a simple fix to use `location.key` for saving scroll state, if it's available. Based on the suggestions found in this discussion: taion/scroll-behavior#135 (comment) ## Related Issues Fixes #12390.
gatsbyjs · Mar 28, 2019 · 853ceb9 · 853ceb9
1 parent 2133288
commit 853ceb9
Show file tree

Hide file tree

Showing 6 changed files with 129 additions and 44 deletions.
diff --git a/e2e-tests/production-runtime/cypress/integration/1-production.js b/e2e-tests/production-runtime/cypress/integration/1-production.js
@@ -11,47 +11,6 @@ describe(`Production build tests`, () => {
     })
   }
 
-  it(`should restore scroll position only when going back in history`, () => {
-    cy.getTestElement(`long-page`)
-      .click()
-      .waitForAPI(`onRouteUpdate`)
-
-    cy.scrollTo(`bottom`)
-
-    // allow ScrollContext to update scroll position store
-    // it uses requestAnimationFrame so wait a bit to allow
-    // it to store scroll position
-    cy.wait(500)
-
-    cy.getTestElement(`below-the-fold`)
-      .click()
-      .waitForAPI(`onRouteUpdate`)
-
-    // after going back we expect page will
-    // be restore previous scroll position
-    cy.go(`back`).waitForAPI(`onRouteUpdate`)
-
-    cy.window().then(win => {
-      expect(win.scrollY).not.to.eq(0, 0)
-    })
-
-    cy.go(`forward`).waitForAPI(`onRouteUpdate`)
-
-    // after clicking link we expect page will be scrolled to top
-    cy.getTestElement(`long-page`)
-      .click()
-      .waitForAPI(`onRouteUpdate`)
-
-    cy.window().then(win => {
-      expect(win.scrollY).to.eq(0, 0)
-    })
-
-    // reset to index page
-    cy.getTestElement(`index-link`)
-      .click()
-      .waitForAPI(`onRouteUpdate`)
-  })
-
   it(`should navigate back after a reload`, () => {
     cy.getTestElement(`page2`).click()
 

diff --git a/e2e-tests/production-runtime/cypress/integration/scroll-behavior.js b/e2e-tests/production-runtime/cypress/integration/scroll-behavior.js
@@ -0,0 +1,96 @@
+describe(`Scroll behaviour`, () => {
+  it(`should restore scroll position only when going back in history`, () => {
+    cy.visit(`/`).waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`long-page`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.scrollTo(`bottom`)
+
+    // allow ScrollContext to update scroll position store
+    // it uses requestAnimationFrame so wait a bit to allow
+    // it to store scroll position
+    cy.wait(500)
+
+    cy.getTestElement(`below-the-fold`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    // after going back we expect page will
+    // be restore previous scroll position
+    cy.go(`back`).waitForAPI(`onRouteUpdate`)
+
+    cy.window().then(win => {
+      expect(win.scrollY).not.to.eq(0, 0)
+    })
+
+    cy.go(`forward`).waitForAPI(`onRouteUpdate`)
+
+    // after clicking link we expect page will be scrolled to top
+    cy.getTestElement(`long-page`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.window().then(win => {
+      expect(win.scrollY).to.eq(0, 0)
+    })
+
+    // reset to index page
+    cy.getTestElement(`index-link`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+  })
+
+  it(`should keep track of location.key`, () => {
+    cy.visit(`/`).waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`long-page`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`below-the-fold`)
+      .scrollIntoView({
+        // this is weird hack - seems like Cypress in run mode doesn't update scroll correctly
+        duration: 100,
+      })
+      .wait(500) // allow ScrollContext to update scroll position store
+      .storeScrollPosition(`middle-of-the-page`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`long-page`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.getTestElement(`even-more-below-the-fold`)
+      .scrollIntoView({
+        // this is weird hack - seems like Cypress in run mode doesn't update scroll correctly
+        duration: 100,
+      })
+      .wait(500) // allow ScrollContext to update scroll position store
+      .storeScrollPosition(`bottom-of-the-page`)
+      .click()
+      .waitForAPI(`onRouteUpdate`)
+
+    cy.go(`back`).waitForAPI(`onRouteUpdate`)
+
+    cy.location(`pathname`)
+      .should(`equal`, `/long-page/`)
+      .wait(500)
+      // we went back in hitsory 1 time, so we should end up at the bottom of the page
+      .shouldMatchScrollPosition(`bottom-of-the-page`)
+      .shouldNotMatchScrollPosition(`middle-of-the-page`)
+
+    cy.go(`back`).waitForAPI(`onRouteUpdate`)
+    cy.go(`back`).waitForAPI(`onRouteUpdate`)
+
+    cy.location(`pathname`)
+      .should(`equal`, `/long-page/`)
+      .wait(500)
+      // we went back in hitsory 2 more times, so we should end up in the middle of the page
+      // instead of at the bottom
+      .shouldMatchScrollPosition(`middle-of-the-page`)
+      .shouldNotMatchScrollPosition(`bottom-of-the-page`)
+  })
+})
diff --git a/e2e-tests/production-runtime/cypress/support/commands.js b/e2e-tests/production-runtime/cypress/support/commands.js
@@ -8,3 +8,28 @@ Cypress.Commands.add(`lifecycleCallCount`, action =>
         ).length
     )
 )
+
+const storedScrollPositions = {}
+
+Cypress.Commands.add(`getScrollPosition`, () =>
+  cy.window().then(win => win.scrollY)
+)
+
+Cypress.Commands.add(`storeScrollPosition`, { prevSubject: true }, (prev, id) =>
+  cy.getScrollPosition().then(scrollPosition => {
+    storedScrollPositions[id] = scrollPosition
+    return prev
+  })
+)
+
+Cypress.Commands.add(`shouldMatchScrollPosition`, id =>
+  cy.getScrollPosition().should(scrollPosition => {
+    expect(scrollPosition).to.be.closeTo(storedScrollPositions[id], 100)
+  })
+)
+
+Cypress.Commands.add(`shouldNotMatchScrollPosition`, id =>
+  cy.getScrollPosition().should(scrollPosition => {
+    expect(scrollPosition).not.to.be.closeTo(storedScrollPositions[id], 100)
+  })
+)
diff --git a/e2e-tests/production-runtime/package.json b/e2e-tests/production-runtime/package.json
@@ -18,7 +18,7 @@
   ],
   "license": "MIT",
   "scripts": {
-    "build": "gatsby build",
+    "build": "CYPRESS_SUPPORT=y gatsby build",
     "build:offline": "TEST_PLUGIN_OFFLINE=y gatsby build",
     "develop": "gatsby develop",
     "format": "prettier --write '**/*.js'",

diff --git a/e2e-tests/production-runtime/src/pages/long-page.js b/e2e-tests/production-runtime/src/pages/long-page.js
@@ -9,7 +9,11 @@ const LongPage = () => (
     <p>Welcome to long page</p>
     <div style={{ height: `200vh` }} />
     <Link to="/" data-testid="below-the-fold">
-      Go back to the homepage
+      Go back to the homepage - middle of the page
+    </Link>
+    <div style={{ height: `200vh` }} />
+    <Link to="/" data-testid="even-more-below-the-fold">
+      Go back to the homepage - bottom of the page
     </Link>
   </Layout>
 )

diff --git a/packages/gatsby-react-router-scroll/src/StateStorage.js b/packages/gatsby-react-router-scroll/src/StateStorage.js
@@ -50,7 +50,8 @@ export default class SessionStorage {
   }
 
   getStateKey(location, key) {
-    const stateKeyBase = `${STATE_KEY_PREFIX}${location.pathname}`
+    const locationKey = location.key || location.pathname
+    const stateKeyBase = `${STATE_KEY_PREFIX}${locationKey}`
     return key === null || typeof key === `undefined`
       ? stateKeyBase
       : `${stateKeyBase}|${key}`