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.

Sign up for GitHub

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

Restore attribute "refresh=morph" to flag turbo frames to reload during a page refresh #1068

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Restore attribute "refresh=morph" to flag turbo frames to reload during a page refresh #1068

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 7 additions & 15 deletions src/core/drive/morph_renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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,
Expand All @@ -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()
}
Expand All @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion src/tests/fixtures/frame_refresh_morph.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<turbo-frame id="remote-frame">
<turbo-frame id="refresh-morph">
<h2>Loaded morphed frame</h2>
</turbo-frame>
3 changes: 3 additions & 0 deletions src/tests/fixtures/frame_refresh_reload.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<turbo-frame id="refresh-reload">
<h2>Loaded reloadable frame</h2>
</turbo-frame>
6 changes: 5 additions & 1 deletion src/tests/fixtures/page_refresh.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,14 @@
<body>
<h1>Page to be refreshed</h1>

<turbo-frame id="remote-frame" src="/src/tests/fixtures/frame_refresh_morph.html">
<turbo-frame id="refresh-morph" 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!

Expand Down
6 changes: 5 additions & 1 deletion src/tests/fixtures/page_refresh_replace.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,14 @@
<body>
<h1>Page to be refreshed</h1>

<turbo-frame id="remote-frame" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
<turbo-frame id="refresh-morph" 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>
Expand Down
19 changes: 12 additions & 7 deletions src/tests/functional/page_refresh_tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,21 @@ 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")
await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
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 }) => {
Expand All @@ -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 }) => {
Expand Down