Merge pull request #1192 from krschacht/frame-reload-uses-morphing

Lift the frame morphing logic up to FrameController.reload

src/core/drive/morphing_page_renderer.js

@@ -1,5 +1,4 @@
 import { FrameElement } from "../../elements/frame_element"
-import { MorphingFrameRenderer } from "../frames/morphing_frame_renderer"
 import { PageRenderer } from "./page_renderer"
 import { dispatch } from "../../util"
 import { morphElements } from "../morphing"
@@ -13,7 +12,7 @@ export class MorphingPageRenderer extends PageRenderer {
     })
 
     for (const frame of currentElement.querySelectorAll("turbo-frame")) {
-      if (canRefreshFrame(frame)) refreshFrame(frame)
+      if (canRefreshFrame(frame)) frame.reload()
     }
 
     dispatch("turbo:morph", { detail: { currentElement, newElement } })
@@ -38,11 +37,3 @@ function canRefreshFrame(frame) {
     frame.refresh === "morph" &&
     !frame.closest("[data-turbo-permanent]")
 }
-
-function refreshFrame(frame) {
-  frame.addEventListener("turbo:before-frame-render", ({ detail }) => {
-    detail.render = MorphingFrameRenderer.renderElement
-  }, { once: true })
-
-  frame.reload()
-}

src/core/frames/frame_controller.js

@@ -20,6 +20,7 @@ import { FrameView } from "./frame_view"
 import { LinkInterceptor } from "./link_interceptor"
 import { FormLinkClickObserver } from "../../observers/form_link_click_observer"
 import { FrameRenderer } from "./frame_renderer"
+import { MorphingFrameRenderer } from "./morphing_frame_renderer"
 import { session } from "../index"
 import { StreamMessage } from "../streams/stream_message"
 import { PageSnapshot } from "../drive/page_snapshot"
@@ -89,6 +90,12 @@ export class FrameController {
   }
 
   sourceURLReloaded() {
+    if (this.element.shouldReloadWithMorph) {
+      this.element.addEventListener("turbo:before-frame-render", ({ detail }) => {
+        detail.render = MorphingFrameRenderer.renderElement
+      }, { once: true })
+    }
+
     const { src } = this.element
     this.element.removeAttribute("complete")
     this.element.src = null
@@ -256,6 +263,7 @@ export class FrameController {
       detail: { newFrame, ...options },
       cancelable: true
     })
+
     const {
       defaultPrevented,
       detail: { render }
@@ -300,6 +308,7 @@ export class FrameController {
     if (newFrameElement) {
       const snapshot = new Snapshot(newFrameElement)
       const renderer = new FrameRenderer(this, this.view.snapshot, snapshot, FrameRenderer.renderElement, false, false)
+
       if (this.view.renderPromise) await this.view.renderPromise
       this.changeHistory()
 

src/elements/frame_element.js

@@ -91,6 +91,10 @@ export class FrameElement extends HTMLElement {
     }
   }
 
+  get shouldReloadWithMorph() {
+    return this.src && this.refresh === "morph"
+  }
+
   /**
    * Determines if the element is loading
    */

src/tests/fixtures/frames.html

@@ -11,6 +11,12 @@
           target.closest("turbo-frame")?.setAttribute("data-turbo-action", "advance")
         } else if (target.id == "remove-target-from-hello") {
           document.getElementById("hello").removeAttribute("target")
+        } else if (target.id == "add-refresh-reload-to-frame") {
+          target.closest("turbo-frame")?.setAttribute("refresh", "reload")
+        } else if (target.id == "add-refresh-morph-to-frame") {
+          target.closest("turbo-frame")?.setAttribute("refresh", "morph")
+        } else if (target.id == "add-src-to-frame") {
+          target.closest("turbo-frame")?.setAttribute("src", "/src/tests/fixtures/frames.html")
         }
       })
     </script>
@@ -28,6 +34,9 @@ <h2>Frames: #frame</h2>
         <button id="frame-form-get-no-redirect">Navigate #frame without a redirect</button>
       </form>
       <button id="add-turbo-action-to-frame" type="button">Add [data-turbo-action="advance"] to #frame</button>
+      <button id="add-refresh-reload-to-frame" type="button">Add [refresh="reload"] to #frame</button>
+      <button id="add-refresh-morph-to-frame" type="button">Add [refresh="morph"] to #frame</button>
+      <button id="add-src-to-frame" type="button">Add [src="/src/tests/fixtures/frames.html"] to #frame so it can be reloaded</button>
       <a id="link-frame" href="/src/tests/fixtures/frames/frame.html">Navigate #frame from within</a>
       <a id="link-frame-with-search-params" href="/src/tests/fixtures/frames/frame.html?key=value">Navigate #frame with ?key=value</a>
       <a id="link-nested-frame-action-advance" href="/src/tests/fixtures/frames/frame.html" data-turbo-action="advance">Navigate #frame from within with a[data-turbo-action="advance"]</a>
@@ -60,7 +69,7 @@ <h2>Frames: #frame</h2>
     <turbo-frame id="hello" target="frame">
       <h2>Frames: #hello</h2>
 
-      <a id="hello-link-frame" href="/src/tests/fixtures/frames/frame.html">Load #frame</a>
+      <a href="/src/tests/fixtures/frames/frame.html">Load #frame</a>
       <button type="button" id="remove-target-from-hello">Remove #hello[target]</button>
 
     </turbo-frame>

src/tests/functional/frame_tests.js

@@ -267,12 +267,43 @@ test("following a link to a page with a matching frame does not dispatch a turbo
   )
 })
 
-test("following a link within a frame with a target set navigates the target frame", async ({ page }) => {
+test("following a link within a frame which has a target set navigates the target frame without morphing even when frame[refresh=morph]", async ({ page }) => {
+  await page.click("#add-refresh-morph-to-frame")
   await page.click("#hello a")
   await nextBeat()
 
-  const frameText = await page.textContent("#frame h2")
-  assert.equal(frameText, "Frame: Loaded")
+  expect(await nextEventOnTarget(page, "frame", "turbo:before-frame-render")).toBeTruthy()
+  expect(await noNextEventOnTarget(page, "frame", "turbo:before-frame-morph")).toBeTruthy()
+  await expect(page.locator("#frame h2")).toHaveText("Frame: Loaded")
+})
+
+test("navigating from within replaces the contents even with turbo-frame[refresh=morph]", async ({ page }) => {
+  await page.click("#add-refresh-morph-to-frame")
+  await page.click("#link-frame")
+  await nextBeat()
+
+  expect(await nextEventOnTarget(page, "frame", "turbo:before-frame-render")).toBeTruthy()
+  expect(await noNextEventOnTarget(page, "frame", "turbo:before-frame-morph")).toBeTruthy()
+  await expect(page.locator("#frame h2")).toHaveText("Frame: Loaded")
+})
+
+test("calling reload on a frame replaces the contents", async ({ page }) => {
+  await page.click("#add-src-to-frame")
+
+  await page.evaluate(() => document.getElementById("frame").reload())
+
+  expect(await nextEventOnTarget(page, "frame", "turbo:before-frame-render")).toBeTruthy()
+  expect(await noNextEventOnTarget(page, "frame", "turbo:before-frame-morph")).toBeTruthy()
+})
+
+test("calling reload on a frame[refresh=morph] morphs the contents", async ({ page }) => {
+  await page.click("#add-src-to-frame")
+  await page.click("#add-refresh-morph-to-frame")
+
+  await page.evaluate(() => document.getElementById("frame").reload())
+
+  expect(await nextEventOnTarget(page, "frame", "turbo:before-frame-render")).toBeTruthy()
+  expect(await nextEventOnTarget(page, "frame", "turbo:before-frame-morph")).toBeTruthy()
 })
 
 test("following a link in rapid succession cancels the previous request", async ({ page }) => {
@@ -667,7 +698,7 @@ test("navigating turbo-frame[data-turbo-action=advance] from within pushes URL s
 
 test("navigating turbo-frame[data-turbo-action=advance] from outside with target pushes URL state", async ({ page }) => {
   await page.click("#add-turbo-action-to-frame")
-  await page.click("#hello-link-frame")
+  await page.click("#hello a")
   await nextEventNamed(page, "turbo:load")
 
   await expect(page.locator("h1")).toHaveText("Frames")

src/tests/functional/page_refresh_tests.js

@@ -160,7 +160,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 marked with refresh='morph'", async ({ page }) => {
+test("uses morphing to only update remote frames marked with refresh='morph'", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")
 
   await page.click("#form-submit")
@@ -180,12 +180,15 @@ test("uses morphing to update remote frames marked with refresh='morph'", async
 test("don't refresh frames contained in [data-turbo-permanent] elements", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")
 
+  // Set the frame's text since the final assertion cannot be noNextEventOnTarget as that is passing even when the frame reloads.
+  const frame = page.locator("#remote-permanent-frame")
+  await frame.evaluate((frame) => frame.textContent = "Frame to be preserved")
+
   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 noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy()
+  await expect(page.locator("#remote-permanent-frame")).toHaveText("Frame to be preserved")
 })
 
 test("frames marked with refresh='morph' are excluded from full page morphing", async ({ page }) => {