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

Introduce FormLinkInterceptor #631

Merged
merged 2 commits into from
Jul 17, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
25 changes: 19 additions & 6 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ import {
import { FetchMethod, FetchRequest, FetchRequestDelegate, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { AppearanceObserver, AppearanceObserverDelegate } from "../../observers/appearance_observer"
import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy, attributeTrue } from "../../util"
import { clearBusyState, getAttribute, parseHTMLDocument, markAsBusy } from "../../util"
import { FormSubmission, FormSubmissionDelegate } from "../drive/form_submission"
import { Snapshot } from "../snapshot"
import { ViewDelegate } from "../view"
import { getAction, expandURL, urlsAreEqual, locationIsVisitable } from "../url"
import { FormInterceptor, FormInterceptorDelegate } from "./form_interceptor"
import { FrameView } from "./frame_view"
import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../../observers/form_link_interceptor"
import { FrameRenderer } from "./frame_renderer"
import { session } from "../index"
import { isAction } from "../types"
Expand All @@ -26,12 +27,14 @@ export class FrameController
FormInterceptorDelegate,
FormSubmissionDelegate,
FrameElementDelegate,
FormLinkInterceptorDelegate,
LinkInterceptorDelegate,
ViewDelegate<Snapshot<FrameElement>>
{
readonly element: FrameElement
readonly view: FrameView
readonly appearanceObserver: AppearanceObserver
readonly formLinkInterceptor: FormLinkInterceptor
readonly linkInterceptor: LinkInterceptor
readonly formInterceptor: FormInterceptor
formSubmission?: FormSubmission
Expand All @@ -46,6 +49,7 @@ export class FrameController
this.element = element
this.view = new FrameView(this, this.element)
this.appearanceObserver = new AppearanceObserver(this, this.element)
this.formLinkInterceptor = new FormLinkInterceptor(this, this.element)
this.linkInterceptor = new LinkInterceptor(this, this.element)
this.formInterceptor = new FormInterceptor(this, this.element)
}
Expand All @@ -58,6 +62,7 @@ export class FrameController
} else {
this.loadSourceURL()
}
this.formLinkInterceptor.start()
this.linkInterceptor.start()
this.formInterceptor.start()
}
Expand All @@ -67,6 +72,7 @@ export class FrameController
if (this.connected) {
this.connected = false
this.appearanceObserver.stop()
this.formLinkInterceptor.stop()
this.linkInterceptor.stop()
this.formInterceptor.stop()
}
Expand Down Expand Up @@ -146,14 +152,21 @@ export class FrameController
this.loadSourceURL()
}

// Form link interceptor delegate

shouldInterceptFormLinkClick(link: Element): boolean {
return this.shouldInterceptNavigation(link)
}

formLinkClickIntercepted(link: Element, form: HTMLFormElement): void {
const frame = this.findFrameElement(link)
if (frame) form.setAttribute("data-turbo-frame", frame.id)
}

// Link interceptor delegate

shouldInterceptLinkClick(element: Element, _url: string) {
if (element.hasAttribute("data-turbo-method") || attributeTrue(element, "data-turbo-stream")) {
return false
} else {
return this.shouldInterceptNavigation(element)
}
return this.shouldInterceptNavigation(element)
}

linkClickIntercepted(element: Element, url: string) {
Expand Down
63 changes: 15 additions & 48 deletions src/core/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import { FormSubmitObserver, FormSubmitObserverDelegate } from "../observers/for
import { FrameRedirector } from "./frames/frame_redirector"
import { History, HistoryDelegate } from "./drive/history"
import { LinkClickObserver, LinkClickObserverDelegate } from "../observers/link_click_observer"
import { FormLinkInterceptor, FormLinkInterceptorDelegate } from "../observers/form_link_interceptor"
import { getAction, expandURL, locationIsVisitable, Locatable } from "./url"
import { Navigator, NavigatorDelegate } from "./drive/navigator"
import { PageObserver, PageObserverDelegate } from "../observers/page_observer"
import { ScrollObserver } from "../observers/scroll_observer"
import { StreamMessage } from "./streams/stream_message"
import { StreamObserver } from "../observers/stream_observer"
import { Action, Position, StreamSource, isAction } from "./types"
import { attributeTrue, clearBusyState, dispatch, markAsBusy } from "../util"
import { clearBusyState, dispatch, markAsBusy } from "../util"
import { PageView, PageViewDelegate } from "./drive/page_view"
import { Visit, VisitOptions } from "./drive/visit"
import { PageSnapshot } from "./drive/page_snapshot"
Expand All @@ -35,6 +36,7 @@ export class Session
implements
FormSubmitObserverDelegate,
HistoryDelegate,
FormLinkInterceptorDelegate,
LinkClickObserverDelegate,
NavigatorDelegate,
PageObserverDelegate,
Expand All @@ -53,7 +55,7 @@ export class Session
readonly formSubmitObserver = new FormSubmitObserver(this)
readonly scrollObserver = new ScrollObserver(this)
readonly streamObserver = new StreamObserver(this)

readonly formLinkInterceptor = new FormLinkInterceptor(this, document.documentElement)
readonly frameRedirector = new FrameRedirector(document.documentElement)

drive = true
Expand All @@ -66,6 +68,7 @@ export class Session
if (!this.started) {
this.pageObserver.start()
this.cacheObserver.start()
this.formLinkInterceptor.start()
this.linkClickObserver.start()
this.formSubmitObserver.start()
this.scrollObserver.start()
Expand All @@ -86,6 +89,7 @@ export class Session
if (this.started) {
this.pageObserver.stop()
this.cacheObserver.stop()
this.formLinkInterceptor.stop()
this.linkClickObserver.stop()
this.formSubmitObserver.stop()
this.scrollObserver.stop()
Expand Down Expand Up @@ -157,6 +161,14 @@ export class Session
this.history.updateRestorationData({ scrollPosition: position })
}

// Form link interceptor delegate

shouldInterceptFormLinkClick(_link: Element): boolean {
return true
}

formLinkClickIntercepted(_link: Element, _form: HTMLFormElement) {}

// Link click observer delegate

willFollowLinkToLocation(link: Element, location: URL, event: MouseEvent) {
Expand All @@ -169,39 +181,7 @@ export class Session

followedLinkToLocation(link: Element, location: URL) {
const action = this.getActionForLink(link)
this.convertLinkWithMethodClickToFormSubmission(link) || this.visit(location.href, { action })
}

convertLinkWithMethodClickToFormSubmission(link: Element) {
const linkMethod = link.getAttribute("data-turbo-method")
const useTurboStream = attributeTrue(link, "data-turbo-stream")

if (linkMethod || useTurboStream) {
const form = document.createElement("form")
form.setAttribute("method", linkMethod || "get")
form.action = link.getAttribute("href") || "undefined"
form.hidden = true

const attributes = ["data-turbo-confirm", "data-turbo-stream"]
attributes.forEach((attribute) => {
if (link.hasAttribute(attribute)) {
form.setAttribute(attribute, link.getAttribute(attribute)!)
}
})

const frame = this.getTargetFrameForLink(link)
if (frame) {
form.setAttribute("data-turbo-frame", frame)
form.addEventListener("turbo:submit-start", () => form.remove())
} else {
form.addEventListener("submit", () => form.remove())
}

document.body.appendChild(form)
return dispatch("submit", { cancelable: true, target: form })
} else {
return false
}
this.visit(location.href, { action })
}

// Navigator delegate
Expand Down Expand Up @@ -423,19 +403,6 @@ export class Session
return isAction(action) ? action : "advance"
}

getTargetFrameForLink(link: Element) {
const frame = link.getAttribute("data-turbo-frame")

if (frame) {
return frame
} else {
const container = link.closest("turbo-frame")
if (container) {
return container.id
}
}
}

get snapshot() {
return this.view.snapshot
}
Expand Down
52 changes: 52 additions & 0 deletions src/observers/form_link_interceptor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { LinkInterceptor, LinkInterceptorDelegate } from "../core/frames/link_interceptor"

export type FormLinkInterceptorDelegate = {
shouldInterceptFormLinkClick(link: Element): boolean
formLinkClickIntercepted(link: Element, form: HTMLFormElement): void
}

export class FormLinkInterceptor implements LinkInterceptorDelegate {
readonly linkInterceptor: LinkInterceptor
readonly delegate: FormLinkInterceptorDelegate

constructor(delegate: FormLinkInterceptorDelegate, element: HTMLElement) {
this.delegate = delegate
this.linkInterceptor = new LinkInterceptor(this, element)
}

start() {
this.linkInterceptor.start()
}

stop() {
this.linkInterceptor.stop()
}

shouldInterceptLinkClick(link: Element): boolean {
return (
this.delegate.shouldInterceptFormLinkClick(link) &&
(link.hasAttribute("data-turbo-method") || link.hasAttribute("data-turbo-stream"))
)
}

linkClickIntercepted(link: Element, action: string): void {
const form = document.createElement("form")
form.setAttribute("action", action)
form.hidden = true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies if this edge case is already handled, I don't have a full understanding of all of the internals of Turbo.

But in the case that Turbo.session.drive = false, but on this link data-turbo="true" is specified, then wouldn't the form created by this also need to set data-turbo="true" on the form element for it to be handled by Turbo?

I have a PR open to address this (#500) but the addition of this FormLinkInterceptor would change where that behavior should be implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for raising these concerns! I've pushed up d317319 to add test coverage for the cases you've outlined.


const method = link.getAttribute("data-turbo-method")
if (method) form.setAttribute("method", method)

const turboConfirm = link.getAttribute("data-turbo-confirm")
if (turboConfirm) form.setAttribute("data-turbo-confirm", turboConfirm)

const turboStream = link.getAttribute("data-turbo-stream")
if (turboStream) form.setAttribute("data-turbo-stream", turboStream)

this.delegate.formLinkClickIntercepted(link, form)

document.body.appendChild(form)
form.requestSubmit()
form.remove()
}
}
1 change: 1 addition & 0 deletions src/tests/fixtures/form.html
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ <h2>Frame: Form</h2>
<form method="post" action="https://httpbin.org/post">
<button id="submit-external">POST to https://httpbin.org/post</button>
</form>
<a href="/__turbo/redirect?path=/src/tests/fixtures/frames/hello.html" data-turbo-method="post" data-turbo-frame="hello" id="turbo-method-post-to-targeted-frame">Turbo method post to targeted frame</a>
<turbo-frame id="hello"></turbo-frame>
<hr>

Expand Down
10 changes: 10 additions & 0 deletions src/tests/functional/form_submission_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,16 @@ test("test POST to external action targetting frame ignored", async ({ page }) =
assert.equal(page.url(), "https://httpbin.org/post")
})

test("test following a link with data-turbo-method set and a target set navigates the target frame", async ({
page,
}) => {
await page.click("#turbo-method-post-to-targeted-frame")
await nextBeat()

const frameText = await page.locator("#hello h2")
assert.equal(await frameText.textContent(), "Hello from a frame")
})

function formSubmitStarted(page: Page) {
return getFromLocalStorage(page, "formSubmitStarted")
}
Expand Down