Don't scroll to top of page on reload (hotwired#571)

* Don't scroll on reload Potential solution for hotwired#387 * Be more specific with the check * Put the check back * Guard the performScroll call directly * WIP - spec * Properly assert in scroll spec Co-authored-by: manuelpuyol@github.com * Update rendering_tests.ts
srt32 · May 19, 2022 · daabebb · daabebb
1 parent e2d5305
commit daabebb
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 1 deletion.
diff --git a/src/core/drive/page_view.ts b/src/core/drive/page_view.ts
@@ -14,9 +14,11 @@ type PageViewRenderer = PageRenderer | ErrorRenderer
 export class PageView extends View<Element, PageSnapshot, PageViewRenderer, PageViewDelegate> {
   readonly snapshotCache = new SnapshotCache(10)
   lastRenderedLocation = new URL(location.href)
+  forceReloaded = false
 
   renderPage(snapshot: PageSnapshot, isPreview = false, willRender = true) {
     const renderer = new PageRenderer(this.snapshot, snapshot, isPreview, willRender)
+    if (!renderer.shouldRender) { this.forceReloaded = true }
     return this.render(renderer)
   }
 

diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts
@@ -425,7 +425,9 @@ export class Visit implements FetchRequestDelegate {
     })
     await callback()
     delete this.frame
-    this.performScroll()
+    if (!this.view.forceReloaded) {
+      this.performScroll()
+    }
   }
 
   cancelRender() {

diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html
@@ -24,6 +24,9 @@
     <script type="importmap" nonce="123">
       { "imports": { "turbo": "/dist/turbo.es2017-umd.js?123"} }
     </script>
+    <style>
+      .push-off-screen { margin-top: 1000px; }
+    </style>
   </head>
   <body>
     <section>
@@ -53,5 +56,8 @@ <h1>Rendering</h1>
       <source src="/src/tests/fixtures/video.mp4" type="video/mp4">
     </video>
     <button id="permanent-video-button" type="button">Play</button>
+
+    <hr class="push-off-screen">
+    <p><a id="below-the-fold-visit-control-reload-link" href="/src/tests/fixtures/visit_control_reload.html">Visit control: reload</a></p>
   </body>
 </html>
diff --git a/src/tests/functional/rendering_tests.ts b/src/tests/functional/rendering_tests.ts
@@ -72,6 +72,27 @@ export class RenderingTests extends TurboDriveTestCase {
     this.assert.equal(reason, "turbo_visit_control_is_reload")
   }
 
+  async "test maintains scroll position before visit when turbo-visit-control setting is reload"() {
+    await this.scrollToSelector("#below-the-fold-visit-control-reload-link")
+    this.assert.notOk(await this.isScrolledToTop(), "scrolled down")
+
+    await this.remote.execute(() => localStorage.setItem("scrolls", "false"))
+
+    this.remote.execute(() => addEventListener("scroll", () => {
+      localStorage.setItem("scrolls", "true")
+    }))
+
+    this.clickSelector("#below-the-fold-visit-control-reload-link")
+
+    await this.nextBody
+
+    const scrolls = await this.remote.execute(() => localStorage.getItem("scrolls"))
+    this.assert.ok(scrolls === "false", "scroll position is preserved")
+
+    this.assert.equal(await this.pathname, "/src/tests/fixtures/visit_control_reload.html")
+    this.assert.equal(await this.visitAction, "load")
+  }
+
   async "test accumulates asset elements in head"() {
     const originalElements = await this.assetElements