Skip to content

Commit

Permalink
Morph remote turbo frames where the src attribute has changed
Browse files Browse the repository at this point in the history
There are some cases when we don't want to reload a remote turbo frame on
a page refresh.

This may be because Turbo has added a src attribute to the turbo frame
element, but we don't want to reload the frame from that URL.

Form example, when a form inside a turbo frame is submitted, Turbo adds
a src attribute to the form element. In those cases we don't want to
reload the Turbo frame from the src URL. The src attribute points to the
form submission URL, which may not be loadable with a GET request.

Same thing can happen when a link inside a turbo frame is clicked. Turbo
adds a src attribute to the frame element, but we don't want to reload the
frame from that URL.

If the src attribute of a turbo frame changes, this signals that the server
wants to render something different that what's currently on the DOM, and
Turbo should respect that.

This also matches the progressive enhancement behavior philosophy of Turbo.
The behaviour results in the Turbo frame that the server sends, which
is what would happen anyway if there was no morphing involved, or on a first
page load.
  • Loading branch information
afcapel committed Nov 13, 2023
1 parent ddfe38b commit cf56cfb
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
22 changes: 15 additions & 7 deletions src/core/drive/morph_renderer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Idiomorph from "idiomorph"
import { dispatch } from "../../util"
import { urlsAreEqual } from "../url"
import { Renderer } from "../renderer"

export class MorphRenderer extends Renderer {
Expand All @@ -26,7 +27,7 @@ export class MorphRenderer extends Renderer {
}

#morphElements(currentElement, newElement, morphStyle = "outerHTML") {
this.isMorphingTurboFrame = this.#isRemoteFrame(currentElement)
this.isMorphingTurboFrame = this.#remoteFrameReplacement(currentElement, newElement)

Idiomorph.morph(currentElement, newElement, {
morphStyle: morphStyle,
Expand All @@ -42,12 +43,19 @@ export class MorphRenderer extends Renderer {
return !(node.id && node.hasAttribute("data-turbo-permanent") && document.getElementById(node.id))
}

#shouldMorphElement = (node) => {
if (node instanceof HTMLElement) {
return !node.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isRemoteFrame(node))
} else {
#shouldMorphElement = (oldNode, newNode) => {
if (!(oldNode instanceof HTMLElement) || this.isMorphingTurboFrame) {
return true
}
else if (oldNode.hasAttribute("data-turbo-permanent")) {
return false
} else {
return !this.#remoteFrameReplacement(oldNode, newNode)
}
}

#remoteFrameReplacement = (oldNode, newNode) => {
return this.#isRemoteFrame(oldNode) && this.#isRemoteFrame(newNode) && urlsAreEqual(oldNode.getAttribute("src"), newNode.getAttribute("src"))
}

#shouldRemoveElement = (node) => {
Expand Down Expand Up @@ -77,8 +85,8 @@ export class MorphRenderer extends Renderer {
this.#morphElements(currentElement, newElement.children, "innerHTML")
}

#isRemoteFrame(element) {
return element.nodeName.toLowerCase() === "turbo-frame" && element.src
#isRemoteFrame(node) {
return node instanceof HTMLElement && node.nodeName.toLowerCase() === "turbo-frame" && node.getAttribute("src")
}

#remoteFrames() {
Expand Down
2 changes: 1 addition & 1 deletion src/tests/functional/page_refresh_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ 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 excluded from full page morphing", async ({ page }) => {
test("remote frames 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"))
Expand Down

0 comments on commit cf56cfb

Please sign in to comment.