From 99d07550a6bbea2573bb3607cf98e177c34aa676 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Wed, 28 Feb 2024 17:37:12 -0500 Subject: [PATCH] Add `[method]` and `[scroll]` attributes for Refresh Stream > I have a page where I am using Turbo morph. when I submit a form and > redirect I would like to reset the scroll, but if a refresh is triggered > by a broadcast I would like to preserve teh scroll. Is that possible ? > > [#turbo Discord][discord] This commit expands the set of attributes for `` to include `[method]` and `[scroll]` (in addition to `[request-id]`). These attributes correspond directly to the [`turbo-refresh`-prefixed `` element][meta] elements that control morphing and scroll preservation. When present on the ``, their values are forward along to the `Session.refresh` method call, which in turn encodes them into Visit options under the `refresh` key. Those options are then used during `Visit` instantiation, and transformed into `.refresh` properties. At render time, the `PageRenderer` attempts to read the refresh method and scroll preservation settings with the following precedence: 1. read from the corresponding `Visit.refresh` property (possibly null) 2. read from the corresponding `` element (possibly null) If no value is provided, fallback to the default (`{ method: "replace", scroll: "reset" }`). [discord]: https://discord.com/channels/988103760050012160/1044659721229054033/1212443786270212167 [meta]: https://turbo.hotwired.dev/handbook/page_refreshes#morphing --- src/core/drive/navigator.js | 2 +- src/core/drive/page_snapshot.js | 8 ++++---- src/core/drive/page_view.js | 6 ++++-- src/core/drive/visit.js | 7 +++++-- src/core/session.js | 6 ++++-- src/core/streams/stream_actions.js | 6 +++++- src/tests/functional/page_refresh_tests.js | 19 +++++++++++++++++++ 7 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/core/drive/navigator.js b/src/core/drive/navigator.js index 8d8d3e0be..d6f659ff8 100644 --- a/src/core/drive/navigator.js +++ b/src/core/drive/navigator.js @@ -99,7 +99,7 @@ export class Navigator { } else { await this.view.renderPage(snapshot, false, true, this.currentVisit) } - if(!snapshot.shouldPreserveScrollPosition) { + if (snapshot.refreshScroll !== "preserve") { this.view.scrollToTop() } this.view.clearSnapshotCache() diff --git a/src/core/drive/page_snapshot.js b/src/core/drive/page_snapshot.js index 58a5a75a9..56e0c63f1 100644 --- a/src/core/drive/page_snapshot.js +++ b/src/core/drive/page_snapshot.js @@ -74,12 +74,12 @@ export class PageSnapshot extends Snapshot { return this.headSnapshot.getMetaValue("view-transition") === "same-origin" } - get shouldMorphPage() { - return this.getSetting("refresh-method") === "morph" + get refreshMethod() { + return this.getSetting("refresh-method") } - get shouldPreserveScrollPosition() { - return this.getSetting("refresh-scroll") === "preserve" + get refreshScroll() { + return this.getSetting("refresh-scroll") } // Private diff --git a/src/core/drive/page_view.js b/src/core/drive/page_view.js index 1583f25a0..8789ca96d 100644 --- a/src/core/drive/page_view.js +++ b/src/core/drive/page_view.js @@ -16,7 +16,8 @@ export class PageView extends View { } renderPage(snapshot, isPreview = false, willRender = true, visit) { - const shouldMorphPage = this.isPageRefresh(visit) && this.snapshot.shouldMorphPage + const refreshMethod = visit?.refresh?.method || this.snapshot.refreshMethod + const shouldMorphPage = this.isPageRefresh(visit) && refreshMethod === "morph" const rendererClass = shouldMorphPage ? MorphRenderer : PageRenderer const renderer = new rendererClass(this.snapshot, snapshot, PageRenderer.renderElement, isPreview, willRender) @@ -60,7 +61,8 @@ export class PageView extends View { } shouldPreserveScrollPosition(visit) { - return this.isPageRefresh(visit) && this.snapshot.shouldPreserveScrollPosition + const refreshScroll = visit?.refresh?.scroll || this.snapshot.refreshScroll + return this.isPageRefresh(visit) && refreshScroll === "preserve" } get snapshot() { diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js index ec7565979..7c3e73b46 100644 --- a/src/core/drive/visit.js +++ b/src/core/drive/visit.js @@ -12,7 +12,8 @@ const defaultOptions = { willRender: true, updateHistory: true, shouldCacheSnapshot: true, - acceptsStreamResponse: false + acceptsStreamResponse: false, + refresh: {} } export const TimingMetric = { @@ -72,7 +73,8 @@ export class Visit { updateHistory, shouldCacheSnapshot, acceptsStreamResponse, - direction + direction, + refresh } = { ...defaultOptions, ...options @@ -92,6 +94,7 @@ export class Visit { this.shouldCacheSnapshot = shouldCacheSnapshot this.acceptsStreamResponse = acceptsStreamResponse this.direction = direction || Direction[action] + this.refresh = refresh } get adapter() { diff --git a/src/core/session.js b/src/core/session.js index cdb978348..76311c604 100644 --- a/src/core/session.js +++ b/src/core/session.js @@ -107,10 +107,12 @@ export class Session { } } - refresh(url, requestId) { + refresh(url, options = {}) { + const { method, requestId, scroll } = options const isRecentRequest = requestId && this.recentRequests.has(requestId) + if (!isRecentRequest) { - this.visit(url, { action: "replace", shouldCacheSnapshot: false }) + this.visit(url, { action: "replace", shouldCacheSnapshot: false, refresh: { method, scroll } }) } } diff --git a/src/core/streams/stream_actions.js b/src/core/streams/stream_actions.js index 486dc8566..92f33fbd9 100644 --- a/src/core/streams/stream_actions.js +++ b/src/core/streams/stream_actions.js @@ -36,7 +36,11 @@ export const StreamActions = { }, refresh() { - session.refresh(this.baseURI, this.requestId) + const method = this.getAttribute("method") + const requestId = this.requestId + const scroll = this.getAttribute("scroll") + + session.refresh(this.baseURI, { method, requestId, scroll }) }, morph() { diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js index c5c116c08..d9cd60b7f 100644 --- a/src/tests/functional/page_refresh_tests.js +++ b/src/tests/functional/page_refresh_tests.js @@ -160,6 +160,13 @@ test("uses morphing to update remote frames marked with refresh='morph'", async await expect(page.locator("#refresh-reload")).toHaveText("Loaded reloadable frame") }) +test("overrides the meta value to render with replace when the Turbo Stream has [method=replace] attribute", async ({ page }) => { + await page.goto("/src/tests/fixtures/page_refresh.html") + + await page.evaluate(() => document.body.insertAdjacentHTML("beforeend", ``)) + await nextEventNamed(page, "turbo:render", { renderMethod: "replace" }) +}) + test("don't refresh frames contained in [data-turbo-permanent] elements", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") @@ -225,6 +232,18 @@ test("it preserves the scroll position when the turbo-refresh-scroll meta tag is await assertPageScroll(page, 10, 10) }) +test("overrides the meta value to reset the scroll position when the Turbo Stream has [scroll=reset] attribute", async ({ page }) => { + await page.goto("/src/tests/fixtures/page_refresh.html") + + await page.evaluate(() => window.scrollTo(10, 10)) + await assertPageScroll(page, 10, 10) + + await page.evaluate(() => document.body.insertAdjacentHTML("beforeend", ``)) + await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) + + await assertPageScroll(page, 0, 0) +}) + test("it does not preserve the scroll position on regular 'advance' navigations, despite of using a 'preserve' option", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html")