Always morph remote turbo-frames when a page refresh happens

Instead of flagging the frames you want to morph with an special attribute, we are always going to reload and refresh with morphing all the turbo-frames in the page. This simplifies the API as it removes the concern of categorizing remote turbo-frames on the programmer side when using page-refreshes. This is an idea by @afcapel, who raised the concern of the confusion the attribute replace=morph caused, and questioned its necessity. We can bring the old approach back if we find real cases that justify it.
hotwired · Oct 27, 2023 · 00b3015 · 00b3015
1 parent aade8c8
commit 00b3015
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 31 deletions.
diff --git a/src/core/drive/morph_renderer.js b/src/core/drive/morph_renderer.js
@@ -26,7 +26,7 @@ export class MorphRenderer extends Renderer {
   }
 
   #morphElements(currentElement, newElement, morphStyle = "outerHTML") {
-    this.isMorphingTurboFrame = this.#isFrameReloadedWithMorph(currentElement)
+    this.isMorphingTurboFrame = this.#isRemoteFrame(currentElement)
 
     Idiomorph.morph(currentElement, newElement, {
       morphStyle: morphStyle,
@@ -40,7 +40,7 @@ export class MorphRenderer extends Renderer {
 
   #reloadRemoteFrames() {
     this.#remoteFrames().forEach((frame) => {
-      if (this.#isFrameReloadedWithMorph(frame)) {
+      if (this.#isRemoteFrame(frame)) {
         this.#renderFrameWithMorph(frame)
         frame.reload()
       }
@@ -67,7 +67,7 @@ export class MorphRenderer extends Renderer {
 
   #shouldMorphElement = (node) => {
     if (node instanceof HTMLElement) {
-      return !node.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isFrameReloadedWithMorph(node))
+      return !node.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isRemoteFrame(node))
     } else {
       return true
     }
@@ -82,8 +82,8 @@ export class MorphRenderer extends Renderer {
     }
   }
 
-  #isFrameReloadedWithMorph(element) {
-    return element.src && element.refresh === "morph"
+  #isRemoteFrame(element) {
+    return element.nodeName.toLowerCase() === "turbo-frame" && element.src
   }
 
   #remoteFrames() {

diff --git a/src/tests/fixtures/frame_refresh_morph.html b/src/tests/fixtures/frame_refresh_morph.html
@@ -1,3 +1,3 @@
-<turbo-frame id="refresh-morph">
+<turbo-frame id="remote-frame">
   <h2>Loaded morphed frame</h2>
 </turbo-frame>
diff --git a/src/tests/fixtures/frame_refresh_reload.html b/src/tests/fixtures/frame_refresh_reload.html
diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html
@@ -24,14 +24,10 @@
   <body>
     <h1>Page to be refreshed</h1>
 
-    <turbo-frame id="refresh-morph" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
+    <turbo-frame id="remote-frame" src="/src/tests/fixtures/frame_refresh_morph.html">
       <h2>Frame to be morphed</h2>
     </turbo-frame>
 
-    <turbo-frame id="refresh-reload" src="/src/tests/fixtures/frame_refresh_reload.html" refresh="reload">
-      <h2>Frame to be reloaded</h2>
-    </turbo-frame>
-
     <div id="preserve-me" data-turbo-permanent>
       Preserve me!
     </div>

diff --git a/src/tests/fixtures/page_refresh_replace.html b/src/tests/fixtures/page_refresh_replace.html
@@ -23,14 +23,10 @@
   <body>
     <h1>Page to be refreshed</h1>
 
-    <turbo-frame id="refresh-morph" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
+    <turbo-frame id="remote-frame" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
       <h2>Frame to be morphed</h2>
     </turbo-frame>
 
-    <turbo-frame id="refresh-reload" src="/src/tests/fixtures/frame_refresh_reload.html" refresh="reload">
-      <h2>Frame to be reloaded</h2>
-    </turbo-frame>
-
     <div id="preserve-me" data-turbo-permanent>
       Preserve me!
     </div>

diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js
@@ -4,8 +4,7 @@ import {
   nextBeat,
   nextEventNamed,
   nextEventOnTarget,
-  noNextEventNamed,
-  noNextEventOnTarget
+  noNextEventNamed
 } from "../helpers/page"
 
 test("renders a page refresh with morphing", async ({ page }) => {
@@ -31,33 +30,29 @@ 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 marked with refresh='morph'", async ({ page }) => {
+test("uses morphing to update remote frames marked", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")
 
   await page.click("#form-submit")
   await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
   await nextBeat()
 
   // Only the frame marked with refresh="morph" uses morphing
-  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")
+  expect(await nextEventOnTarget(page, "remote-frame", "turbo:before-frame-morph")).toBeTruthy()
+  await expect(page.locator("#remote-frame")).toHaveText("Loaded morphed frame")
 })
 
 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("refresh-morph").setAttribute("data-modified", "true"))
+  await page.evaluate(() => document.getElementById("remote-frame").setAttribute("data-modified", "true"))
 
   await page.click("#form-submit")
   await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
   await nextBeat()
 
-  await expect(page.locator("#refresh-morph")).toHaveAttribute("data-modified", "true")
-  await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
+  await expect(page.locator("#remote-frame")).toHaveAttribute("data-modified", "true")
+  await expect(page.locator("#remote-frame")).toHaveText("Loaded morphed frame")
 })
 
 test("it preserves the scroll position when the turbo-refresh-scroll meta tag is 'preserve'", async ({ page }) => {