Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Push history state from frame navigations #398

Merged
merged 3 commits into from
Nov 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/core/drive/navigator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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"
}
}
10 changes: 7 additions & 3 deletions src/core/drive/visit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,16 @@ export enum VisitState {
export type VisitOptions = {
action: Action,
historyChanged: boolean,
willRender: boolean
referrer?: URL,
snapshotHTML?: string,
response?: VisitResponse
}

const defaultOptions: VisitOptions = {
action: "advance",
historyChanged: false
historyChanged: false,
willRender: true
}

export type VisitResponse = {
Expand All @@ -69,6 +71,7 @@ export class Visit implements FetchRequestDelegate {
readonly referrer?: URL
readonly timingMetrics: TimingMetrics = {}

willRender: boolean
followedRedirect = false
frame?: number
historyChanged = false
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
28 changes: 25 additions & 3 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<Snapshot<FrameElement>> {
readonly element: FrameElement
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/core/frames/frame_redirector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
1 change: 1 addition & 0 deletions src/elements/frame_element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
27 changes: 27 additions & 0 deletions src/tests/fixtures/frames.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,42 @@
<title>Frame</title>
<script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
<script src="/src/tests/fixtures/test.js"></script>
<script>
addEventListener("click", ({ target }) => {
if (target.id == "add-turbo-action-to-frame") {
target.closest("turbo-frame")?.setAttribute("data-turbo-action", "advance")
}
})
</script>
</head>
<body>
<h1>Frames</h1>

<turbo-frame id="frame" data-loaded-from="/src/tests/fixtures/frames.html">
<h2>Frames: #frame</h2>

<button id="add-turbo-action-to-frame" type="button">Add [data-turbo-action="advance"] to frame</button>
<a id="link-frame" href="/src/tests/fixtures/frames/frame.html">Navigate #frame from within</a>
<a id="link-nested-frame-action-advance" href="/src/tests/fixtures/frames/frame.html" data-turbo-action="advance">Navigate #frame from within with a[data-turbo-action="advance"]</a>
</turbo-frame>
<a id="outside-frame-form" href="/src/tests/fixtures/frames/form.html" data-turbo-frame="frame">Navigate #frame to /frames/form.html</a>

<a id="link-outside-frame-action-advance" href="/src/tests/fixtures/frames/frame.html" data-turbo-frame="frame" data-turbo-action="advance">Navigate #frame from outside with a[data-turbo-action="advance"]</a>
<form id="form-get-frame-action-advance" action="/__turbo/redirect" data-turbo-frame="frame" data-turbo-action="advance">
<input type="hidden" name="path" value="/src/tests/fixtures/frames/frame.html">
<button>Navigate #frame with GET form[data-turbo-action="advance"]</button>
</form>

<form id="form-post-frame-action-advance" method="post" action="/__turbo/redirect" data-turbo-frame="frame" data-turbo-action="advance">
<input type="hidden" name="path" value="/src/tests/fixtures/frames/frame.html">
<button>Navigate #frame with POST form[data-turbo-action="advance"]</button>
</form>

<form method="post" action="/__turbo/redirect" data-turbo-frame="frame" data-turbo-action="advance">
<input type="hidden" name="path" value="/src/tests/fixtures/frames/frame.html">
<button id="button-frame-action-advance" data-turbo-action="advance">Navigate #frame with button[data-turbo-action="advance"]</button>
</form>

<turbo-frame id="hello" target="frame">
<h2>Frames: #hello</h2>

Expand Down
2 changes: 2 additions & 0 deletions src/tests/fixtures/frames/frame.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
<script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
</head>
<body>
<h1>Frames: #frame</h1>

<turbo-frame id="frame" data-loaded-from="/src/tests/fixtures/frames/frame.html">
<h2>Frame: Loaded</h2>
</turbo-frame>
Expand Down
73 changes: 73 additions & 0 deletions src/tests/functional/frame_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
8 changes: 8 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}