From 274d3698aa3c7d006c8ae6197265cd989a3ace8f Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 11 Nov 2021 07:54:49 -0500 Subject: [PATCH] Push history state from frame navigations (#398) * Add `linkClickIntercepted` to FrameElementDelegate Expand the `FrameElementDelegate` interface to include a `linkClickIntercepted` to match its existing `formSubmissionIntercepted`, then replace a manual `setAttribute` and `src` assignment with a delegation to the `FrameElementDelegate` instance. * 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 * Extract `getAttribute` utility function For cases where we need to find an attribute value from a collection of elements, use `getAttribute` instead of a long chain of `||` and `?` operators. --- src/core/drive/navigator.ts | 3 +- src/core/drive/visit.ts | 10 ++-- src/core/frames/frame_controller.ts | 28 +++++++++-- src/core/frames/frame_redirector.ts | 3 +- src/elements/frame_element.ts | 1 + src/tests/fixtures/frames.html | 27 ++++++++++ src/tests/fixtures/frames/frame.html | 2 + src/tests/functional/frame_tests.ts | 73 ++++++++++++++++++++++++++++ src/util.ts | 8 +++ 9 files changed, 146 insertions(+), 9 deletions(-) diff --git a/src/core/drive/navigator.ts b/src/core/drive/navigator.ts index 6b87a3126..59b2ad0e1 100644 --- a/src/core/drive/navigator.ts +++ b/src/core/drive/navigator.ts @@ -3,6 +3,7 @@ import { FetchMethod } from "../../http/fetch_request" import { FetchResponse } from "../../http/fetch_response" import { FormSubmission } from "./form_submission" import { expandURL, getAnchor, getRequestURL, Locatable, locationIsVisitable } from "../url" +import { getAttribute } from "../../util" import { Visit, VisitDelegate, VisitOptions } from "./visit" import { PageSnapshot } from "./page_snapshot" @@ -158,7 +159,7 @@ export class Navigator { getActionForFormSubmission(formSubmission: FormSubmission): Action { const { formElement, submitter } = formSubmission - const action = submitter?.getAttribute("data-turbo-action") || formElement.getAttribute("data-turbo-action") + const action = getAttribute("data-turbo-action", submitter, formElement) return isAction(action) ? action : "advance" } } diff --git a/src/core/drive/visit.ts b/src/core/drive/visit.ts index 624992dec..7105d923b 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 = { @@ -69,6 +71,7 @@ export class Visit implements FetchRequestDelegate { readonly referrer?: URL readonly timingMetrics: TimingMetrics = {} + willRender: boolean followedRedirect = false frame?: number historyChanged = false @@ -87,7 +90,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 @@ -201,7 +205,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 d85e5f49d..705e46640 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -2,7 +2,7 @@ import { FrameElement, FrameElementDelegate, FrameLoadingStyle } from "../../ele import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request" import { FetchResponse } from "../../http/fetch_response" import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer" -import { parseHTMLDocument } from "../../util" +import { getAttribute, parseHTMLDocument } from "../../util" import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission" import { Snapshot } from "../snapshot" import { ViewDelegate } from "../view" @@ -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 @@ -199,6 +200,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) } @@ -247,12 +251,30 @@ 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 = 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 + + session.visit(location, { willRender: false, action, response: { redirected, responseHTML, statusCode } }) + } + + 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") + const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target") return getFrameElementById(id) ?? this.element } @@ -285,7 +307,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest } private shouldInterceptNavigation(element: Element, submitter?: HTMLElement) { - const id = submitter?.getAttribute("data-turbo-frame") || element.getAttribute("data-turbo-frame") || this.element.getAttribute("target") + const id = getAttribute("data-turbo-frame", submitter, element) || this.element.getAttribute("target") if (element instanceof HTMLFormElement && !this.formActionIsVisitable(element, submitter)) { return false diff --git a/src/core/frames/frame_redirector.ts b/src/core/frames/frame_redirector.ts index f7dd36555..c300c43cb 100644 --- a/src/core/frames/frame_redirector.ts +++ b/src/core/frames/frame_redirector.ts @@ -31,8 +31,7 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptor linkClickIntercepted(element: Element, url: string) { const frame = this.findFrameElement(element) if (frame) { - frame.setAttribute("reloadable", "") - frame.src = url + frame.delegate.linkClickIntercepted(element, url) } } diff --git a/src/elements/frame_element.ts b/src/elements/frame_element.ts index d13abce7f..bfde5d66c 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -9,6 +9,7 @@ export interface FrameElementDelegate { sourceURLChanged(): void disabledChanged(): void formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement): void + linkClickIntercepted(element: Element, url: string): void loadResponse(response: FetchResponse): void isLoading: boolean } 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 2a7821d65..a91ea3673 100644 --- a/src/tests/functional/frame_tests.ts +++ b/src/tests/functional/frame_tests.ts @@ -280,6 +280,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")) diff --git a/src/util.ts b/src/util.ts index 5b873e404..5cef3785b 100644 --- a/src/util.ts +++ b/src/util.ts @@ -55,3 +55,11 @@ export function uuid() { } }).join("") } + +export function getAttribute(attributeName: string, ...elements: (Element | undefined)[]): string | null { + for (const value of elements.map(element => element?.getAttribute(attributeName))) { + if (typeof value == "string") return value + } + + return null +}