From 9ff6e16da631f5d58d467bceea7e42e40df02a3a Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Fri, 17 Nov 2023 12:34:45 +0100 Subject: [PATCH] Restore attribute "refresh=morph" to flag turbo frames to reload during a page refresh. This adds a new turbo-frame attribute: `refresh`. When its value is `morph`: * It will reload the turbo frame with morphing during a page refresh. * It won't update the turbo frame with the server response. * It won't remove it if it's missing in the server response. This attribute was part of the original proposal we presented in Rails World, then we removed it [1], because we thought it wasn't needed. But after testing the library in different scenarios, we've found assuming certain behavior for all the remote frames was problematic since it implied being too clever about what you wanted to do with the frame. The new attribute makes for a simple behavior: * The default behavior will be the expected one: turbo frames will be morphed as any other element. If they get a new URL they will be reloadded, if the get deleted in the response, they will disappear, etc. * You can use the new attribute to flag the frames for which you want the special behavior. [1]: https://github.com/hotwired/turbo/pull/1019/commits/0c6a95d07fd7661e734ed1fbfe19daa9a00b3fd4 --- src/core/drive/morph_renderer.js | 22 +++++++------------- src/tests/fixtures/frame_refresh_morph.html | 2 +- src/tests/fixtures/frame_refresh_reload.html | 3 +++ src/tests/fixtures/page_refresh.html | 6 +++++- src/tests/fixtures/page_refresh_replace.html | 6 +++++- src/tests/functional/page_refresh_tests.js | 19 ++++++++++------- 6 files changed, 33 insertions(+), 25 deletions(-) create mode 100644 src/tests/fixtures/frame_refresh_reload.html diff --git a/src/core/drive/morph_renderer.js b/src/core/drive/morph_renderer.js index c3379be2b..2269e9ed4 100644 --- a/src/core/drive/morph_renderer.js +++ b/src/core/drive/morph_renderer.js @@ -1,6 +1,5 @@ import Idiomorph from "idiomorph" import { dispatch } from "../../util" -import { urlsAreEqual } from "../url" import { Renderer } from "../renderer" export class MorphRenderer extends Renderer { @@ -27,7 +26,7 @@ export class MorphRenderer extends Renderer { } #morphElements(currentElement, newElement, morphStyle = "outerHTML") { - this.isMorphingTurboFrame = this.#remoteFrameReplacement(currentElement, newElement) + this.isMorphingTurboFrame = this.#isFrameReloadedWithMorph(currentElement) Idiomorph.morph(currentElement, newElement, { morphStyle: morphStyle, @@ -44,27 +43,20 @@ export class MorphRenderer extends Renderer { } #shouldMorphElement = (oldNode, newNode) => { - if (!(oldNode instanceof HTMLElement) || this.isMorphingTurboFrame) { - return true - } - else if (oldNode.hasAttribute("data-turbo-permanent")) { - return false + if (oldNode instanceof HTMLElement) { + return !oldNode.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isFrameReloadedWithMorph(oldNode)) } else { - return !this.#remoteFrameReplacement(oldNode, newNode) + return true } } - #remoteFrameReplacement = (oldNode, newNode) => { - return this.#isRemoteFrame(oldNode) && this.#isRemoteFrame(newNode) && urlsAreEqual(oldNode.getAttribute("src"), newNode.getAttribute("src")) - } - #shouldRemoveElement = (node) => { return this.#shouldMorphElement(node) } #reloadRemoteFrames() { this.#remoteFrames().forEach((frame) => { - if (this.#isRemoteFrame(frame)) { + if (this.#isFrameReloadedWithMorph(frame)) { this.#renderFrameWithMorph(frame) frame.reload() } @@ -85,8 +77,8 @@ export class MorphRenderer extends Renderer { this.#morphElements(currentElement, newElement.children, "innerHTML") } - #isRemoteFrame(node) { - return node instanceof HTMLElement && node.nodeName.toLowerCase() === "turbo-frame" && node.getAttribute("src") + #isFrameReloadedWithMorph(element) { + return element.src && element.refresh === "morph" } #remoteFrames() { diff --git a/src/tests/fixtures/frame_refresh_morph.html b/src/tests/fixtures/frame_refresh_morph.html index 70bf85601..bdfeed9fc 100644 --- a/src/tests/fixtures/frame_refresh_morph.html +++ b/src/tests/fixtures/frame_refresh_morph.html @@ -1,3 +1,3 @@ - +

Loaded morphed frame

 diff --git a/src/tests/fixtures/frame_refresh_reload.html b/src/tests/fixtures/frame_refresh_reload.html new file mode 100644 index 000000000..7dd5e83ec --- /dev/null +++ b/src/tests/fixtures/frame_refresh_reload.html @@ -0,0 +1,3 @@ + +

Loaded reloadable frame

+ diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html index ec28092a4..59463ae90 100644 --- a/src/tests/fixtures/page_refresh.html +++ b/src/tests/fixtures/page_refresh.html @@ -24,10 +24,14 @@

Page to be refreshed

- +

Frame to be morphed

 + +

Frame to be reloaded

+ +
Preserve me! diff --git a/src/tests/fixtures/page_refresh_replace.html b/src/tests/fixtures/page_refresh_replace.html index b46a1c327..df4be0cfb 100644 --- a/src/tests/fixtures/page_refresh_replace.html +++ b/src/tests/fixtures/page_refresh_replace.html @@ -23,10 +23,14 @@

Page to be refreshed

- +

Frame to be morphed

 + +

Frame to be reloaded

+ +
Preserve me!
diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js index a17cad790..53c4de98b 100644 --- a/src/tests/functional/page_refresh_tests.js +++ b/src/tests/functional/page_refresh_tests.js @@ -33,7 +33,7 @@ test("doesn't morph when the navigation doesn't go to the same URL", async ({ pa expect(await noNextEventNamed(page, "turbo:render", { renderMethod: "morph" })).toBeTruthy() }) -test("uses morphing to update remote frames", async ({ page }) => { +test("uses morphing to update remote frames marked with refresh='morph'", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") await page.click("#form-submit") @@ -41,8 +41,13 @@ test("uses morphing to update remote frames", async ({ page }) => { await nextBeat() // Only the frame marked with refresh="morph" uses morphing - expect(await nextEventOnTarget(page, "remote-frame", "turbo:before-frame-morph")).toBeTruthy() - await expect(page.locator("#remote-frame")).toHaveText("Loaded morphed frame") + expect(await nextEventOnTarget(page, "refresh-morph", "turbo:before-frame-morph")).toBeTruthy() + expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy() + + await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame") + + // Regular turbo-frames also gets reloaded since their complete attribute is removed + await expect(page.locator("#refresh-reload")).toHaveText("Loaded reloadable frame") }) test("don't refresh frames contained in [data-turbo-permanent] elements", async ({ page }) => { @@ -56,17 +61,17 @@ test("don't refresh frames contained in [data-turbo-permanent] elements", async expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy() }) -test("remote frames are excluded from full page morphing", async ({ page }) => { +test("frames marked with refresh='morph' are excluded from full page morphing", async ({ page }) => { await page.goto("/src/tests/fixtures/page_refresh.html") - await page.evaluate(() => document.getElementById("remote-frame").setAttribute("data-modified", "true")) + await page.evaluate(() => document.getElementById("refresh-morph").setAttribute("data-modified", "true")) await page.click("#form-submit") await nextEventNamed(page, "turbo:render", { renderMethod: "morph" }) await nextBeat() - await expect(page.locator("#remote-frame")).toHaveAttribute("data-modified", "true") - await expect(page.locator("#remote-frame")).toHaveText("Loaded morphed frame") + await expect(page.locator("#refresh-morph")).toHaveAttribute("data-modified", "true") + await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame") }) test("it preserves the scroll position when the turbo-refresh-scroll meta tag is 'preserve'", async ({ page }) => {