diff --git a/src/core/drive/navigator.ts b/src/core/drive/navigator.ts index 935f9eea7..80678fdf6 100644 --- a/src/core/drive/navigator.ts +++ b/src/core/drive/navigator.ts @@ -110,7 +110,7 @@ export class Navigator { if (fetchResponse.serverError) { await this.view.renderError(snapshot, this.currentVisit) } else { - await this.view.renderPage(snapshot, false, true, this.currentVisit) + await this.view.renderPage(snapshot, this.currentVisit || null, false, true) } this.view.scrollToTop() this.view.clearSnapshotCache() diff --git a/src/core/drive/page_view.ts b/src/core/drive/page_view.ts index 042ea748e..99bd50973 100644 --- a/src/core/drive/page_view.ts +++ b/src/core/drive/page_view.ts @@ -15,13 +15,10 @@ type PageViewRenderer = PageRenderer | ErrorRenderer export class PageView extends View { readonly snapshotCache = new SnapshotCache(10) lastRenderedLocation = new URL(location.href) - forceReloaded = false - renderPage(snapshot: PageSnapshot, isPreview = false, willRender = true, visit?: Visit) { + renderPage(snapshot: PageSnapshot, visit: Visit | null, isPreview = false, willRender = true) { const renderer = new PageRenderer(this.snapshot, snapshot, isPreview, willRender) - if (!renderer.shouldRender) { - this.forceReloaded = true - } else { + if (renderer.shouldRender) { visit?.changeHistory() } return this.render(renderer) diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index abc9a0c15..14a13117d 100644 --- a/src/core/drive/visit.ts +++ b/src/core/drive/visit.ts @@ -228,7 +228,8 @@ export class Visit implements FetchRequestDelegate { this.cacheSnapshot() if (this.view.renderPromise) await this.view.renderPromise if (isSuccessful(statusCode) && responseHTML != null) { - await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML), false, this.willRender, this) + await this.view.renderPage(PageSnapshot.fromHTMLString(responseHTML), this, false, this.willRender) + this.performScroll() this.adapter.visitRendered(this) this.complete() } else { @@ -270,7 +271,8 @@ export class Visit implements FetchRequestDelegate { this.adapter.visitRendered(this) } else { if (this.view.renderPromise) await this.view.renderPromise - await this.view.renderPage(snapshot, isPreview, this.willRender, this) + await this.view.renderPage(snapshot, this, isPreview, this.willRender) + this.performScroll() this.adapter.visitRendered(this) if (!isPreview) { this.complete() @@ -295,6 +297,7 @@ export class Visit implements FetchRequestDelegate { if (this.isSamePage) { this.render(async () => { this.cacheSnapshot() + this.performScroll() this.adapter.visitRendered(this) }) } @@ -429,9 +432,6 @@ export class Visit implements FetchRequestDelegate { }) await callback() delete this.frame - if (!this.view.forceReloaded) { - 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..3bf34f0f4 --- /dev/null +++ b/src/tests/fixtures/scroll/one.html @@ -0,0 +1,19 @@ + + + + + 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..00dac0752 --- /dev/null +++ b/src/tests/fixtures/scroll/two.html @@ -0,0 +1,19 @@ + + + + + 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 b5603121d..be4898a7e 100644 --- a/src/tests/functional/visit_tests.ts +++ b/src/tests/functional/visit_tests.ts @@ -1,7 +1,16 @@ import { Page, test } from "@playwright/test" import { assert } from "chai" import { get } from "http" -import { nextBeat, nextEventNamed, readEventLogs, visitAction, willChangeBody } from "../helpers/page" +import { + isScrolledToSelector, + isScrolledToTop, + nextBeat, + nextEventNamed, + readEventLogs, + scrollToSelector, + visitAction, + willChangeBody, +} from "../helpers/page" test.beforeEach(async ({ page }) => { await page.goto("/src/tests/fixtures/visit.html") @@ -145,6 +154,37 @@ function contentTypeOfURL(url: string): Promise { }) } +test("test can scroll to element after click-initiated turbo:visit", async ({ page }) => { + const id = "below-the-fold-link" + await page.evaluate((id: string) => { + addEventListener("turbo:load", () => document.getElementById(id)?.scrollIntoView()) + }, id) + + assert(await isScrolledToTop(page), "starts unscrolled") + + await page.click("#same-page-link") + await nextEventNamed(page, "turbo:load") + await nextBeat() + + assert(await isScrolledToSelector(page, "#" + id), "scrolls after click-initiated turbo:load") +}) + +test("test can scroll to element after history-initiated turbo:visit", async ({ page }) => { + const id = "below-the-fold-link" + await page.evaluate((id: string) => { + addEventListener("turbo:load", () => document.getElementById(id)?.scrollIntoView()) + }, id) + + await scrollToSelector(page, "#" + id) + await page.click("#" + id) + await nextEventNamed(page, "turbo:load") + await page.goBack() + await nextEventNamed(page, "turbo:load") + await nextBeat() + + assert(await isScrolledToSelector(page, "#" + id), "scrolls after history-initiated turbo:load") +}) + async function visitLocation(page: Page, location: string) { return page.evaluate((location) => window.Turbo.visit(location), location) }