Skip to content

Commit

Permalink
Integrate Frame to Page Visits with Snapshot Cache
Browse files Browse the repository at this point in the history
[Follow-up to hotwired#398][].

The original implementation achieved the desired outcome: navigate the
page to reflect the URL of a `<turbo-frame>`.

Unfortunately, the `session.visit()` call happens late-enough that the
`<turbo-frame>` element's contents have already been updated. This means
that when navigating back or forward through the browser's History API,
the snapshots _already_ reflect the "new" frame's HTML. This means that
navigating back _won't change the page's HTML_.

To resolve that issue, this commit integrates with the `turbo:visit` and
`turbo:before-cache` events. Depending on **3** events is a touch
awkward, but the event sequence occurs too "far" away from the
`FrameController` instance for it to be able to integrate more tightly.

This commit aims to fix the broken behavior before the `7.1.0-rc`
release, but if a concept like a `FrameVisit` introduced in [hotwired#430][]
were to ship, it might be more straightforward to manage.

[Follow-up to hotwired#398]: hotwired#398
[hotwired#430]: hotwired#430
  • Loading branch information
seanpdoyle committed Nov 11, 2021
1 parent 4a13b6d commit fd16a5a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 6 deletions.
28 changes: 22 additions & 6 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,17 +257,33 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}

private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) {
const { ownerDocument, id, src } = frame
const action = getAttribute("data-turbo-action", submitter, element, frame)

if (isAction(action)) {
const proposeVisit = async (event: Event) => {
const { detail: { fetchResponse: { location, redirected, statusCode } } } = event as CustomEvent
const responseHTML = document.documentElement.outerHTML
const clone = frame.cloneNode(true)

session.visit(location, { willRender: false, action, response: { redirected, responseHTML, statusCode } })
}
frame.addEventListener("turbo:frame-render", () => {
const snapshotHTML = ownerDocument.documentElement.outerHTML

if (src) {
session.visit(src, { willRender: false, action, snapshotHTML })
}
}, { once: true })

frame.addEventListener("turbo:frame-render", proposeVisit , { once: true })
ownerDocument.addEventListener("turbo:visit", () => {
const { snapshot } = session.navigator.currentVisit?.view || { snapshot: null }

if (snapshot) {
ownerDocument.addEventListener("turbo:before-cache", () => {
const frameInSnapshot = snapshot.element.querySelector<FrameElement>(`turbo-frame#${id}`)

if (frameInSnapshot) {
frameInSnapshot.replaceWith(clone)
}
}, { once: true })
}
}, { once: true })
}
}

Expand Down
29 changes: 29 additions & 0 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,35 @@ export class FrameTests extends TurboDriveTestCase {
this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
}

async "test navigating back after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames previous contents"() {
await this.clickSelector("#add-turbo-action-to-frame")
await this.clickSelector("#link-frame")
await this.nextBeat
await this.goBack()

const title = await this.querySelector("h1")
const frameTitle = await this.querySelector("#frame h2")

this.assert.equal(await title.getVisibleText(), "Frames")
this.assert.equal(await frameTitle.getVisibleText(), "Frames: #frame")
this.assert.equal(await this.pathname, "/src/tests/fixtures/frames.html")
}

async "test navigating back then forward after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames next contents"() {
await this.clickSelector("#add-turbo-action-to-frame")
await this.clickSelector("#link-frame")
await this.nextBeat
await this.goBack()
await this.goForward()

const title = await this.querySelector("h1")
const frameTitle = await this.querySelector("#frame h2")

this.assert.equal(await title.getVisibleText(), "Frames")
this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
}

async "test turbo:before-fetch-request fires on the frame element"() {
await this.clickSelector("#hello a")
this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-request"))
Expand Down

0 comments on commit fd16a5a

Please sign in to comment.