Perform scrolling prior to Visit completion

Closes hotwired#400 When processing a `Visit`, invoke `Visit.performScroll()` while loading a response, loading a Snapshot, or executing a same-page anchor navigation. After in-lining those calls to more appropriate points in the Visit lifecycle, this commit removes the `performScroll()` call from the `Visit.render()` method.
seanpdoyle · Nov 27, 2021 · 0406188 · 0406188
1 parent aa9724d
commit 0406188
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 1 deletion.
diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts
@@ -218,6 +218,7 @@ export class Visit implements FetchRequestDelegate {
         if (this.view.renderPromise) await this.view.renderPromise
         if (isSuccessful(statusCode) && responseHTML != null) {
           await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML), false, this.willRender)
+          this.performScroll()
           this.adapter.visitRendered(this)
           this.complete()
         } else {
@@ -260,6 +261,7 @@ export class Visit implements FetchRequestDelegate {
         } else {
           if (this.view.renderPromise) await this.view.renderPromise
           await this.view.renderPage(snapshot, isPreview, this.willRender)
+          this.performScroll()
           this.adapter.visitRendered(this)
           if (!isPreview) {
             this.complete()
@@ -283,6 +285,7 @@ export class Visit implements FetchRequestDelegate {
     if (this.isSamePage) {
       this.render(async () => {
         this.cacheSnapshot()
+        this.performScroll()
         this.adapter.visitRendered(this)
       })
     }
@@ -408,7 +411,6 @@ export class Visit implements FetchRequestDelegate {
     })
     await callback()
     delete this.frame
-    this.performScroll()
   }
 
   cancelRender() {

diff --git a/src/tests/fixtures/scroll/one.html b/src/tests/fixtures/scroll/one.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Scroll: One</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+    <style>
+      html { scroll-behavior: smooth; }
+      .push-below-fold { margin-top: 100vh; }
+    </style>
+  </head>
+  <body>
+    <h1>Scroll: One</h1>
+    <hr class="push-below-fold">
+    <a id="one-below-fold" href="/src/tests/fixtures/scroll/two.html#two-below-fold">two.html#two-below-fold</a>
+    <hr class="push-below-fold">
+  </body>
+</html>
diff --git a/src/tests/fixtures/scroll/two.html b/src/tests/fixtures/scroll/two.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Scroll: Two</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+    <style>
+      html { scroll-behavior: smooth; }
+      .push-below-fold { margin-top: 100vh; }
+    </style>
+  </head>
+  <body>
+    <h1>Scroll: Two</h1>
+    <hr class="push-below-fold">
+    <a id="two-below-fold" href="/src/tests/fixtures/scroll/one.html#one-below-fold">one.html#one-below-fold</a>
+    <hr class="push-below-fold">
+  </body>
+</html>
diff --git a/src/tests/fixtures/visit.html b/src/tests/fixtures/visit.html
@@ -5,13 +5,20 @@
     <title>Turbo</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
     <script src="/src/tests/fixtures/test.js"></script>
+    <style>
+      .push-below-fold { margin-top: 100vh; }
+    </style>
   </head>
   <body>
     <section>
       <h1>Visit</h1>
       <p><a id="same-origin-link" href="/src/tests/fixtures/one.html">Same-origin link</a></p>
       <p><a id="same-origin-link-search-params" href="/src/tests/fixtures/one.html?key=value">Same-origin link with ?key=value</a></p>
       <p><a id="sample-response" href="/src/tests/fixtures/one.html">Sample response</a></p>
+      <p><a id="same-page-link" href="/src/tests/fixtures/visit.html">Same page link</a></p>
+      <hr class="push-below-fold">
+      <p><a id="below-the-fold-link" href="/src/tests/fixtures/one.html">one.html</a></p>
+      <hr class="push-below-fold">
     </section>
   </body>
 </html>
diff --git a/src/tests/functional/visit_tests.ts b/src/tests/functional/visit_tests.ts
@@ -120,6 +120,37 @@ export class VisitTests extends TurboDriveTestCase {
     this.assert.notOk(await this.hasSelector("some-cached-element"))
   }
 
+  async "test can scroll to element after click-initiated turbo:visit"() {
+    const id = "below-the-fold-link"
+    await this.evaluate((id: string) => {
+      addEventListener("turbo:load", () => document.getElementById(id)?.scrollIntoView())
+    }, id)
+
+    this.assert(await this.isScrolledToTop, "starts unscrolled")
+
+    await this.clickSelector("#same-page-link")
+    await this.nextEventNamed("turbo:load")
+    await this.nextBeat
+
+    this.assert(await this.isScrolledToSelector("#" + id), "scrolls after click-initiated turbo:load")
+  }
+
+  async "test can scroll to element after history-initiated turbo:visit"() {
+    const id = "below-the-fold-link"
+    await this.evaluate((id: string) => {
+      addEventListener("turbo:load", () => document.getElementById(id)?.scrollIntoView())
+    }, id)
+
+    await this.scrollToSelector("#" + id)
+    await this.clickSelector("#" + id)
+    await this.nextEventNamed("turbo:load")
+    await this.goBack()
+    await this.nextEventNamed("turbo:load")
+    await this.nextBeat
+
+    this.assert(await this.isScrolledToSelector("#" + id), "scrolls after history-initiated turbo:load")
+  }
+
   async visitLocation(location: string) {
     this.remote.execute((location: string) => window.Turbo.visit(location), [location])
   }