Skip to content

Commit

Permalink
Extract FrameVisit to drive FrameController
Browse files Browse the repository at this point in the history
The problem
---

Programmatically driving a `<turbo-frame>` element when its `[src]`
attribute changes is a suitable end-user experience in consumer
applications. It's a fitting black-box interface for the outside world:
change the value of the attribute and let Turbo handle the rest.

However, internally, it's a lossy abstraction.

For example, the `FrameRedirector` class listens for page-wide events
`click` and `submit` events, determines if their targets are meant to
drive a `<turbo-frame>` element by:

1. finding an element that matches a clicked `<a>` element's `[data-turbo-frame]` attribute
2. finding an element that matches a submitted `<form>` element's `[data-turbo-frame]` attribute
3. finding an element that matches a submitted `<form>` element's
   _submitter's_ `[data-turbo-frame]` attribute
4. finding the closest `<turbo-frame>` ancestor to the `<a>` or `<form>`

Once it finds the matching frame element, it disposes of all that
additional context and navigates the `<turbo-frame>` by updating its
`[src]` attribute. This makes it impossible to control various aspects
of the frame navigation (like its "rendering" explored in
[#146][]) outside of its destination URL.

Similarly, since a `<form>` and submitter pairing have an impact on
which `<turbo-frame>` is navigated, the `FrameController` implementation
passes around a `HTMLFormElement` and `HTMLSubmitter?` data clump and
constantly re-fetches a matching `<turbo-frame>` instance.

Outside of frames, page-wide navigation is driven by a `Visit` instance
that manages the HTTP lifecycle and delegates along the way to a
`VisitDelegate`. It also pairs calls to visit with a `VisitOption`
object to capture additional context.

The proposal
---

This commit introduces the `FrameVisit` class. It serves as an
encapsulation of the `FetchRequest` and `FormSubmission` lifecycle
events involved in navigating a frame.

It's implementation draws inspiration from the `Visit`, `VisitDelegate`,
and `VisitOptions` pairing. Since the `FrameVisit` knows how to unify
both `FetchRequest` and `FormSubmission` hooks, the resulting callbacks
fired from within the `FrameController` are flat and consistent.

Extra benefits
---

The biggest benefit is the introduction of a DRY abstraction to
manage the behind the scenes HTTP calls necessary to drive a
`<turbo-frame>`.

With the introduction of the `FrameVisit` concept, we can also declare a
`visit()` and `submit()` method to the `FrameElementDelegate` in the
place of other implementation-specific methods like `loadResponse()` and
`formSubmissionIntercepted()`.

In addition, these changes have the potential to close
[#326][], since we can consistently invoke
`loadResponse()` across `<a>`-click-initiated and
`<form>`-submission-initiated visits. To ensure that's the case, this
commit adds test coverage for navigating a `<turbo-frame>` by making a
`GET` request to an endpoint that responds with a `500` status.

[#146]: #146
[#326]: #326
  • Loading branch information
seanpdoyle committed Jul 29, 2022
1 parent 9b62ab2 commit b0d767e
Show file tree
Hide file tree
Showing 11 changed files with 288 additions and 160 deletions.
4 changes: 3 additions & 1 deletion src/core/drive/form_submission.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FetchRequest, FetchMethod, fetchMethodFromString, FetchRequestHeaders } from "../../http/fetch_request"
import { FetchResponse } from "../../http/fetch_response"
import { expandURL } from "../url"
import { dispatch, getMetaContent } from "../../util"
import { clearBusyState, dispatch, getMetaContent, markAsBusy } from "../../util"
import { StreamMessage } from "../streams/stream_message"

export interface FormSubmissionDelegate {
Expand Down Expand Up @@ -167,6 +167,7 @@ export class FormSubmission {

requestStarted(_request: FetchRequest) {
this.state = FormSubmissionState.waiting
markAsBusy(this.formElement)
this.submitter?.setAttribute("disabled", "")
dispatch<TurboSubmitStartEvent>("turbo:submit-start", {
target: this.formElement,
Expand Down Expand Up @@ -205,6 +206,7 @@ export class FormSubmission {
requestFinished(_request: FetchRequest) {
this.state = FormSubmissionState.stopped
this.submitter?.removeAttribute("disabled")
clearBusyState(this.formElement)
dispatch<TurboSubmitEndEvent>("turbo:submit-end", {
target: this.formElement,
detail: { formSubmission: this, ...this.result },
Expand Down
11 changes: 4 additions & 7 deletions src/core/drive/navigator.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { Action, isAction } from "../types"
import { Action } from "../types"
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 { Visit, VisitDelegate, VisitOptions, getVisitAction } from "./visit"
import { PageSnapshot } from "./page_snapshot"

export type NavigatorDelegate = VisitDelegate & {
Expand Down Expand Up @@ -173,9 +172,7 @@ export class Navigator {
return this.history.restorationIdentifier
}

getActionForFormSubmission(formSubmission: FormSubmission): Action {
const { formElement, submitter } = formSubmission
const action = getAttribute("data-turbo-action", submitter, formElement)
return isAction(action) ? action : "advance"
getActionForFormSubmission({ formElement, submitter }: FormSubmission): Action {
return getVisitAction(submitter, formElement) || "advance"
}
}
10 changes: 8 additions & 2 deletions src/core/drive/visit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { History } from "./history"
import { getAnchor } from "../url"
import { Snapshot } from "../snapshot"
import { PageSnapshot } from "./page_snapshot"
import { Action, ResolvingFunctions } from "../types"
import { getHistoryMethodForAction, uuid } from "../../util"
import { Action, ResolvingFunctions, isAction } from "../types"
import { getAttribute, getHistoryMethodForAction, uuid } from "../../util"
import { PageView } from "./page_view"

export interface VisitDelegate {
Expand Down Expand Up @@ -472,6 +472,12 @@ export class Visit implements FetchRequestDelegate {
}
}

export function getVisitAction(...elements: (Element | undefined)[]): Action | null {
const action = getAttribute("data-turbo-action", ...elements)

return isAction(action) ? action : null
}

function isSuccessful(statusCode: number) {
return statusCode >= 200 && statusCode < 300
}
Loading

0 comments on commit b0d767e

Please sign in to comment.