From 8af1d0baedaf23df384d8f960e4d624eefd8b459 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 25 Nov 2021 15:50:08 -0500 Subject: [PATCH] Perform scrolling prior to Visit completion Closes https://github.com/hotwired/turbo/issues/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. --- src/core/drive/visit.ts | 4 +++- src/tests/fixtures/scroll/one.html | 20 +++++++++++++++++++ src/tests/fixtures/scroll/two.html | 21 +++++++++++++++++++ src/tests/fixtures/visit.html | 7 +++++++ src/tests/functional/visit_tests.ts | 31 +++++++++++++++++++++++++++++ 5 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 src/tests/fixtures/scroll/one.html create mode 100644 src/tests/fixtures/scroll/two.html diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index 9e900b89b..fc106010e 100644 --- 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 new file mode 100644 index 000000000..d1db3880d --- /dev/null +++ b/src/tests/fixtures/scroll/one.html @@ -0,0 +1,20 @@ + + + + + Scroll: One + + + + + +

Scroll: One

+
+ two.html#two-below-fold +
+
+ + diff --git a/src/tests/fixtures/scroll/two.html b/src/tests/fixtures/scroll/two.html new file mode 100644 index 000000000..e1dfdf7e5 --- /dev/null +++ b/src/tests/fixtures/scroll/two.html @@ -0,0 +1,21 @@ + + + + + Scroll: Two + + + + + +

Scroll: Two

+
+ one.html#one-below-fold +
+
+
+ + diff --git a/src/tests/fixtures/visit.html b/src/tests/fixtures/visit.html index 08e0831ba..138a72c36 100644 --- a/src/tests/fixtures/visit.html +++ b/src/tests/fixtures/visit.html @@ -5,6 +5,9 @@ Turbo +
@@ -12,6 +15,10 @@

Visit

Same-origin link

Same-origin link with ?key=value

Sample response

+

Same page link

+
+

one.html

+
diff --git a/src/tests/functional/visit_tests.ts b/src/tests/functional/visit_tests.ts index c80cccb75..62a52a4b7 100644 --- 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]) }