From d0ccb16cf68ae862bd94dc2cd4e9423e4305e542 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Fri, 17 Sep 2021 10:42:17 -0400 Subject: [PATCH] Push history state from frame navigations Closes https://github.com/hotwired/turbo/issues/50 Closes https://github.com/hotwired/turbo/issues/361 Closes https://github.com/hotwired/turbo/pull/167 --- Extend of built-in support for `` elements with [data-turbo-action][] (with `"replace"` or `"advance"`) to also encompass `` navigations. Account for the combination of of `[data-turbo-frame]` and `[data-turbo-action]` to navigate the target `` _and_ navigate the page's history push state, supporting: * `turbo-frame[data-turbo-action="..."]` * `turbo-frame a[data-turbo-action="..."]` * `a[data-turbo-frame="..."][data-turbo-action="..."]` * `form[data-turbo-frame="..."][data-turbo-action="..."]` * `form[data-turbo-frame="..."] button[data-turbo-action="..."]` * `form button[data-turbo-frame="..."][data-turbo-action="..."]` Whenever a Turbo Frame response is loaded that was initiated from one of those submitters, forms, anchors, or turbo-frames annotated with a `[data-turbo-action]`, the subsequent firing `turbo:frame-render` event will create a `Visit` instance that will skip rendering, won't result in a network request, and will instead only update the snapshot cache and history. [data-turbo-action]: https://turbo.hotwired.dev/handbook/drive#application-visits --- src/core/drive/visit.ts | 10 ++-- src/core/frames/frame_controller.ts | 22 +++++++++ src/tests/fixtures/frames.html | 27 ++++++++++ src/tests/fixtures/frames/frame.html | 2 + src/tests/functional/frame_tests.ts | 73 ++++++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 3 deletions(-) diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index 7b050c1cb..0fb1bac00 100644 --- a/src/core/drive/visit.ts +++ b/src/core/drive/visit.ts @@ -39,6 +39,7 @@ export enum VisitState { export type VisitOptions = { action: Action, historyChanged: boolean, + willRender: boolean referrer?: URL, snapshotHTML?: string, response?: VisitResponse @@ -46,7 +47,8 @@ export type VisitOptions = { const defaultOptions: VisitOptions = { action: "advance", - historyChanged: false + historyChanged: false, + willRender: true } export type VisitResponse = { @@ -68,6 +70,7 @@ export class Visit implements FetchRequestDelegate { readonly referrer?: URL readonly timingMetrics: TimingMetrics = {} + willRender: boolean followedRedirect = false frame?: number historyChanged = false @@ -86,7 +89,8 @@ export class Visit implements FetchRequestDelegate { this.location = location this.restorationIdentifier = restorationIdentifier || uuid() - const { action, historyChanged, referrer, snapshotHTML, response } = { ...defaultOptions, ...options } + const { action, historyChanged, referrer, snapshotHTML, response, willRender } = { ...defaultOptions, ...options } + this.willRender = willRender this.action = action this.historyChanged = historyChanged this.referrer = referrer @@ -200,7 +204,7 @@ export class Visit implements FetchRequestDelegate { } loadResponse() { - if (this.response) { + if (this.response && this.willRender) { const { statusCode, responseHTML } = this.response this.render(async () => { this.cacheSnapshot() diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 7727f6491..cb3219f9a 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -12,6 +12,7 @@ import { FrameView } from "./frame_view" import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor" import { FrameRenderer } from "./frame_renderer" import { session } from "../index" +import { isAction } from "../types" export class FrameController implements AppearanceObserverDelegate, FetchRequestDelegate, FormInterceptorDelegate, FormSubmissionDelegate, FrameElementDelegate, LinkInterceptorDelegate, ViewDelegate> { readonly element: FrameElement @@ -203,6 +204,9 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest formSubmissionSucceededWithResponse(formSubmission: FormSubmission, response: FetchResponse) { const frame = this.findFrameElement(formSubmission.formElement, formSubmission.submitter) + + this.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter) + frame.delegate.loadResponse(response) } @@ -251,10 +255,28 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest private navigateFrame(element: Element, url: string, submitter?: HTMLElement) { const frame = this.findFrameElement(element, submitter) + + this.proposeVisitIfNavigatedWithAction(frame, element, submitter) + frame.setAttribute("reloadable", "") frame.src = url } + private proposeVisitIfNavigatedWithAction(frame: FrameElement, element: Element, submitter?: HTMLElement) { + const action = submitter?.getAttribute("data-turbo-action") || element.getAttribute("data-turbo-action") || frame.getAttribute("data-turbo-action") + + if (isAction(action)) { + const proposeVisit = async (event: Event) => { + const { detail: { fetchResponse: { location, statusCode } } } = event as CustomEvent + const responseHTML = document.documentElement.outerHTML + + session.visit(location, { willRender: false, action, response: { statusCode, responseHTML } }) + } + + frame.addEventListener("turbo:frame-render", proposeVisit , { once: true }) + } + } + private findFrameElement(element: Element, submitter?: HTMLElement) { const id = submitter?.getAttribute("data-turbo-frame") || element.getAttribute("data-turbo-frame") || this.element.getAttribute("target") return getFrameElementById(id) ?? this.element diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html index ed786a2a9..60d54f57e 100644 --- a/src/tests/fixtures/frames.html +++ b/src/tests/fixtures/frames.html @@ -5,15 +5,42 @@ Frame +

Frames

Frames: #frame

+ + +
Navigate #frame from within + Navigate #frame from within with a[data-turbo-action="advance"] Navigate #frame to /frames/form.html + Navigate #frame from outside with a[data-turbo-action="advance"] +
+ + +
+ +
+ + +
+ +
+ + +
+

Frames: #hello

diff --git a/src/tests/fixtures/frames/frame.html b/src/tests/fixtures/frames/frame.html index a15134676..914cec830 100644 --- a/src/tests/fixtures/frames/frame.html +++ b/src/tests/fixtures/frames/frame.html @@ -6,6 +6,8 @@ +

Frames: #frame

+

Frame: Loaded

diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts index 61f008622..ddb95310b 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -267,6 +267,79 @@ export class FrameTests extends TurboDriveTestCase { this.assert.equal(requestLogs.length, 0) } + async "test navigating turbo-frame[data-turbo-action=advance] from within pushes URL state"() { + await this.clickSelector("#add-turbo-action-to-frame") + await this.clickSelector("#link-frame") + await this.nextBeat + + 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 navigating turbo-frame from within with a[data-turbo-action=advance] pushes URL state"() { + await this.clickSelector("#link-nested-frame-action-advance") + await this.nextBeat + + 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 navigating frame with a[data-turbo-action=advance] pushes URL state"() { + await this.clickSelector("#link-outside-frame-action-advance") + await this.nextBeat + + 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 navigating frame with form[method=get][data-turbo-action=advance] pushes URL state"() { + await this.clickSelector("#form-get-frame-action-advance button") + await this.nextBeat + + 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 navigating frame with form[method=post][data-turbo-action=advance] pushes URL state"() { + await this.clickSelector("#form-post-frame-action-advance button") + await this.nextBeat + + 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 navigating frame with button[data-turbo-action=advance] pushes URL state"() { + await this.clickSelector("#button-frame-action-advance") + await this.nextBeat + + 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"))