Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reloading a morphing frame should trigger reloads on its child morphing frames recursively #1311

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions src/core/drive/morphing_page_renderer.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { FrameElement } from "../../elements/frame_element"
import { PageRenderer } from "./page_renderer"
import { dispatch } from "../../util"
import { morphElements } from "../morphing"
Expand All @@ -7,14 +6,16 @@ export class MorphingPageRenderer extends PageRenderer {
static renderElement(currentElement, newElement) {
morphElements(currentElement, newElement, {
callbacks: {
beforeNodeMorphed: element => !canRefreshFrame(element)
beforeNodeMorphed: (node, newNode) => {
if (super.shouldRefreshChildFrameWithMorphing(null, node, newNode)) {
node.reload()
return false
}
return true
}
}
})

for (const frame of currentElement.querySelectorAll("turbo-frame")) {
if (canRefreshFrame(frame)) frame.reload()
}

dispatch("turbo:morph", { detail: { currentElement, newElement } })
}

Expand All @@ -31,9 +32,3 @@ export class MorphingPageRenderer extends PageRenderer {
}
}

function canRefreshFrame(frame) {
return frame instanceof FrameElement &&
frame.src &&
frame.refresh === "morph" &&
!frame.closest("[data-turbo-permanent]")
}
13 changes: 12 additions & 1 deletion src/core/frames/morphing_frame_renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,21 @@ export class MorphingFrameRenderer extends FrameRenderer {
detail: { currentElement, newElement }
})

morphChildren(currentElement, newElement)
morphChildren(currentElement, newElement, {
callbacks: {
beforeNodeMorphed: (node, newNode) => {
if (super.shouldRefreshChildFrameWithMorphing(currentElement, node, newNode)) {
node.reload()
return false
}
return true
}
}
})
}

async preservingPermanentElements(callback) {
return await callback()
}
}

3 changes: 2 additions & 1 deletion src/core/morphing.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ export function morphElements(currentElement, newElement, { callbacks, ...option
})
}

export function morphChildren(currentElement, newElement) {
export function morphChildren(currentElement, newElement, options = {}) {
morphElements(currentElement, newElement.children, {
...options,
morphStyle: "innerHTML"
})
}
Expand Down
13 changes: 13 additions & 0 deletions src/core/renderer.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { FrameElement } from "../elements/frame_element"
import { Bardo } from "./bardo"

export class Renderer {
Expand All @@ -7,6 +8,18 @@ export class Renderer {
// Abstract method
}

static shouldRefreshChildFrameWithMorphing(parentFrame, currentFrame, newFrame) {
return currentFrame instanceof FrameElement &&
// newFrame cannot yet be an instance of FrameElement because custom
// elements don't get initialized until they're attached to the DOM, so
// test its Element#nodeName instead
newFrame instanceof Element && newFrame.nodeName === "TURBO-FRAME" &&
currentFrame.shouldReloadWithMorph &&
currentFrame.id === newFrame.id &&
!currentFrame.closest("[data-turbo-permanent]") &&
currentFrame.parentElement.closest("turbo-frame[src][refresh=morph]") === parentFrame
}

constructor(currentSnapshot, newSnapshot, isPreview, willRender = true) {
this.currentSnapshot = currentSnapshot
this.newSnapshot = newSnapshot
Expand Down
8 changes: 8 additions & 0 deletions src/tests/fixtures/frames.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
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")
} else if (target.id == "change-frame-id-to-different-nested-child") {
target.closest("turbo-frame")?.setAttribute("id", "different-nested-child")
}
})
</script>
Expand Down Expand Up @@ -79,13 +81,18 @@ <h2>Frames: #hello</h2>

<turbo-frame id="nested-root" target="frame">
<h2>Frames: #nested-root</h2>
<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="inner-outer-frame-link" href="/src/tests/fixtures/frames/frame.html" data-turbo-frame="nested-child">Inner/Outer frame link</a>
<form data-turbo-frame="nested-child" method="get" action="/src/tests/fixtures/frames/frame.html">
<input id="inner-outer-frame-submit" type="submit" value="Inner/Outer form submit">
</form>

<turbo-frame id="nested-child">
<h2>Frames: #nested-child</h2>
<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>
<button id="change-frame-id-to-different-nested-child" type="button">Change id to different-nested-child</button>
<a href="/src/tests/fixtures/frames/frame.html">Load #nested-child</a>
<a href="/src/tests/fixtures/one.html" data-turbo-frame="_top">Visit one.html</a>

Expand All @@ -95,6 +102,7 @@ <h2>Frames: #nested-child</h2>
</turbo-frame>

<turbo-frame id="nested-child-navigate-top" target="_top">
<button id="add-src-to-frame" type="button">Add [src="/src/tests/fixtures/frames.html"] to #frame so it can be reloaded</button>
<form method="get" action="/src/tests/fixtures/one.html">
<input id="nested-child-navigate-top-submit" type="submit" value="Nested Child form submit">
</form>
Expand Down
30 changes: 30 additions & 0 deletions src/tests/functional/frame_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,36 @@ test("calling reload on a frame[refresh=morph] morphs the contents", async ({ pa
expect(await nextEventOnTarget(page, "frame", "turbo:before-frame-morph")).toBeTruthy()
})

test("calling reload on a frame[refresh=morph] reloads descendant frame[refresh=morph]s via reload() as well", async ({ page }) => {
await page.click("#nested-root #add-refresh-morph-to-frame")
await page.click("#nested-root #add-src-to-frame")
await page.click("#nested-child #add-refresh-morph-to-frame")
await page.click("#nested-child #add-src-to-frame")
await page.click("#nested-child-navigate-top #add-src-to-frame")

await page.evaluate(() => document.getElementById("nested-root").reload())

// Only the frames marked with refresh="morph" uses morphing
expect(await nextEventOnTarget(page, "nested-root", "turbo:before-frame-morph")).toBeTruthy()
expect(await nextEventOnTarget(page, "nested-child", "turbo:before-frame-morph")).toBeTruthy()
expect(await noNextEventOnTarget(page, "nested-child-navigate-top", "turbo:before-frame-morph")).toBeTruthy()
})

test("calling reload on a frame[refresh=morph] morphs descendant frame[refresh=morph] normally if the id no longer matches", async ({ page }) => {
await page.click("#nested-root #add-refresh-morph-to-frame")
await page.click("#nested-root #add-src-to-frame")
await page.click("#nested-child #add-refresh-morph-to-frame")
await page.click("#nested-child #add-src-to-frame")
await page.click("#nested-child #change-frame-id-to-different-nested-child")

await page.evaluate(() => document.getElementById("nested-root").reload())

expect(await nextEventOnTarget(page, "nested-root", "turbo:before-frame-morph")).toBeTruthy()
expect(await noNextEventOnTarget(page, "different-nested-child", "turbo:before-frame-morph")).toBeTruthy()
assert.ok(await hasSelector(page, "#nested-child"))
assert.notOk(await hasSelector(page, "#different-nested-child"))
})

test("calling reload on a frame[refresh=morph] preserves [data-turbo-permanent] elements", async ({ page }) => {
await page.click("#add-src-to-frame")
await page.click("#add-refresh-morph-to-frame")
Expand Down
Loading