Skip to content

Commit

Permalink
Merge pull request #1175 from Intrepidd/fix-frame-morph-unexpected-re…
Browse files Browse the repository at this point in the history
…loading

Morphing + complete frame issue : Stop reloading turbo frames when complete attribute changes
  • Loading branch information
jorgemanrubia committed Mar 4, 2024
2 parents 00527e5 + a235658 commit f0beef3
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 31 deletions.
22 changes: 6 additions & 16 deletions src/core/frames/frame_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,12 @@ export class FrameController {

sourceURLReloaded() {
const { src } = this.element
this.#ignoringChangesToAttribute("complete", () => {
this.element.removeAttribute("complete")
})
this.element.removeAttribute("complete")
this.element.src = null
this.element.src = src
return this.element.loaded
}

completeChanged() {
if (this.#isIgnoringChangesTo("complete")) return

this.#loadSourceURL()
}

loadingStyleChanged() {
if (this.loadingStyle == FrameLoadingStyle.lazy) {
this.appearanceObserver.start()
Expand Down Expand Up @@ -528,13 +520,11 @@ export class FrameController {
}

set complete(value) {
this.#ignoringChangesToAttribute("complete", () => {
if (value) {
this.element.setAttribute("complete", "")
} else {
this.element.removeAttribute("complete")
}
})
if (value) {
this.element.setAttribute("complete", "")
} else {
this.element.removeAttribute("complete")
}
}

get isActive() {
Expand Down
6 changes: 2 additions & 4 deletions src/elements/frame_element.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class FrameElement extends HTMLElement {
loaded = Promise.resolve()

static get observedAttributes() {
return ["disabled", "complete", "loading", "src"]
return ["disabled", "loading", "src"]
}

constructor() {
Expand All @@ -48,11 +48,9 @@ export class FrameElement extends HTMLElement {
attributeChangedCallback(name) {
if (name == "loading") {
this.delegate.loadingStyleChanged()
} else if (name == "complete") {
this.delegate.completeChanged()
} else if (name == "src") {
this.delegate.sourceURLChanged()
} else {
} else if (name == "disabled") {
this.delegate.disabledChanged()
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/tests/fixtures/frame_refresh_after_navigation.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<turbo-frame id="refresh-after-navigation">
<h2 id="refresh-after-navigation-content">Frame has been navigated</h2>
</turbo-frame>
5 changes: 5 additions & 0 deletions src/tests/fixtures/page_refresh.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ <h2>Frame to be morphed</h2>
<h2>Frame to be reloaded</h2>
</turbo-frame>

<turbo-frame id="refresh-after-navigation">
<h2>Frame to be navigated then reset to its initial state after reload</h2>
<a id="refresh-after-navigation-link" href="/src/tests/fixtures/frame_refresh_after_navigation.html">Navigate</a>
</turbo-frame>

<div id="preserve-me" data-turbo-permanent>
Preserve me!

Expand Down
11 changes: 0 additions & 11 deletions src/tests/functional/loading_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,6 @@ test("navigating away from a page does not reload its frames", async ({ page })
assert.equal(requestLogs.length, 1)
})

test("removing the [complete] attribute of an eager frame reloads the content", async ({ page }) => {
await nextEventOnTarget(page, "frame", "turbo:frame-load")
await page.evaluate(() => document.querySelector("#loading-eager turbo-frame")?.removeAttribute("complete"))
await nextEventOnTarget(page, "frame", "turbo:frame-load")

assert.ok(
await hasSelector(page, "#loading-eager turbo-frame[complete]"),
"sets the [complete] attribute after re-loading"
)
})

test("changing [src] attribute on a [complete] frame with loading=lazy defers navigation", async ({ page }) => {
await page.click("#loading-lazy summary")
await nextEventOnTarget(page, "hello", "turbo:frame-load")
Expand Down
28 changes: 28 additions & 0 deletions src/tests/functional/page_refresh_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,34 @@ test("frames marked with refresh='morph' are excluded from full page morphing",
await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
})

test("navigated frames without refresh attribute are reset after morphing", async ({ page }) => {
await page.goto("/src/tests/fixtures/page_refresh.html")

await page.click("#refresh-after-navigation-link")

await nextBeat()

assert.ok(
await hasSelector(page, "#refresh-after-navigation-content"),
"navigates theframe"
)

await page.click("#form-submit")

await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
await nextBeat()

assert.ok(
await hasSelector(page, "#refresh-after-navigation-link"),
"resets the frame"
)

assert.notOk(
await hasSelector(page, "#refresh-after-navigation-content"),
"does not reload the frame"
)
})

test("it preserves the scroll position when the turbo-refresh-scroll meta tag is 'preserve'", async ({ page }) => {
await page.goto("/src/tests/fixtures/page_refresh.html")

Expand Down

0 comments on commit f0beef3

Please sign in to comment.