From ad194f95598cc9176996ed6ed9d98f004a20ba1c Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 30 Sep 2021 15:02:13 -0700 Subject: [PATCH] Replace LinkInterceptor with LinkClickObserver Follow-up to https://github.com/hotwired/turbo/pull/382 --- In the same style as [hotwired/turbo#382][], replace instances of `LinkInterceptor` with `LinkClickObserver`, making the necessary interface changes in classes that used to extend `LinkInterceptorDelegate`. Conditional logic that was once covered in the `LinkInterceptor` is moved into the predicate methods of the delegates (for example, the `FrameController.willFollowLinkToLocation` and `FrameRedirector.willFollowLinkToLocation`). Since the recently introduced [FormLinkInterceptor][] was named after the `LinkInterceptor`, this commit also renames that class (and its call sites) to match the `LinkClickObserver`-suffix, along with the `LinkInterceptorDelegate` method structure. [hotwired/turbo#382]: https://github.com/hotwired/turbo/pull/382 [FormLinkInterceptor]: https://github.com/hotwired/turbo/commit/f8a94c5f761c4f080d68a87459d3916eb4fb6993 --- src/core/frames/frame_controller.ts | 38 +++++++-------- src/core/frames/frame_redirector.ts | 18 +++---- src/core/frames/link_interceptor.ts | 57 ----------------------- src/core/session.ts | 22 ++++----- src/elements/frame_element.ts | 2 +- src/observers/form_link_click_observer.ts | 56 ++++++++++++++++++++++ src/observers/form_link_interceptor.ts | 56 ---------------------- src/observers/link_click_observer.ts | 16 ++++--- 8 files changed, 105 insertions(+), 160 deletions(-) delete mode 100644 src/core/frames/link_interceptor.ts create mode 100644 src/observers/form_link_click_observer.ts delete mode 100644 src/observers/form_link_interceptor.ts diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts index 3e5404957..7d8781eff 100644 --- a/src/core/frames/frame_controller.ts +++ b/src/core/frames/frame_controller.ts @@ -14,8 +14,8 @@ import { ViewDelegate, ViewRenderOptions } 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 { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer" +import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../observers/form_link_click_observer" import { FrameRenderer } from "./frame_renderer" import { session } from "../index" import { isAction } from "../types" @@ -28,15 +28,15 @@ export class FrameController FormInterceptorDelegate, FormSubmissionDelegate, FrameElementDelegate, - FormLinkInterceptorDelegate, - LinkInterceptorDelegate, + FormLinkClickObserverDelegate, + LinkClickObserverDelegate, ViewDelegate> { readonly element: FrameElement readonly view: FrameView readonly appearanceObserver: AppearanceObserver - readonly formLinkInterceptor: FormLinkInterceptor - readonly linkInterceptor: LinkInterceptor + readonly formLinkClickObserver: FormLinkClickObserver + readonly linkClickObserver: LinkClickObserver readonly formInterceptor: FormInterceptor formSubmission?: FormSubmission fetchResponseLoaded = (_fetchResponse: FetchResponse) => {} @@ -51,8 +51,8 @@ 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.formLinkClickObserver = new FormLinkClickObserver(this, this.element) + this.linkClickObserver = new LinkClickObserver(this, this.element) this.formInterceptor = new FormInterceptor(this, this.element) } @@ -64,8 +64,8 @@ export class FrameController } else { this.loadSourceURL() } - this.formLinkInterceptor.start() - this.linkInterceptor.start() + this.formLinkClickObserver.start() + this.linkClickObserver.start() this.formInterceptor.start() } } @@ -74,8 +74,8 @@ export class FrameController if (this.connected) { this.connected = false this.appearanceObserver.stop() - this.formLinkInterceptor.stop() - this.linkInterceptor.stop() + this.formLinkClickObserver.stop() + this.linkClickObserver.stop() this.formInterceptor.stop() } } @@ -161,25 +161,25 @@ export class FrameController this.loadSourceURL() } - // Form link interceptor delegate + // Form link click observer delegate - shouldInterceptFormLinkClick(link: Element): boolean { + willSubmitFormLinkClickToLocation(link: Element): boolean { return this.shouldInterceptNavigation(link) } - formLinkClickIntercepted(link: Element, form: HTMLFormElement): void { + submittedFormLinkToLocation(link: Element, _location: URL, form: HTMLFormElement): void { const frame = this.findFrameElement(link) if (frame) form.setAttribute("data-turbo-frame", frame.id) } - // Link interceptor delegate + // Link click observer delegate - shouldInterceptLinkClick(element: Element, _url: string) { + willFollowLinkToLocation(element: Element) { return this.shouldInterceptNavigation(element) } - linkClickIntercepted(element: Element, url: string) { - this.navigateFrame(element, url) + followedLinkToLocation(element: Element, location: URL) { + this.navigateFrame(element, location.href) } // Form interceptor delegate diff --git a/src/core/frames/frame_redirector.ts b/src/core/frames/frame_redirector.ts index 1ddb5a0d5..e283df7b6 100644 --- a/src/core/frames/frame_redirector.ts +++ b/src/core/frames/frame_redirector.ts @@ -1,37 +1,37 @@ import { FormInterceptor, FormInterceptorDelegate } from "./form_interceptor" import { FrameElement } from "../../elements/frame_element" -import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor" import { expandURL, getAction, locationIsVisitable } from "../url" +import { LinkClickObserver, LinkClickObserverDelegate } from "../../observers/link_click_observer" -export class FrameRedirector implements LinkInterceptorDelegate, FormInterceptorDelegate { +export class FrameRedirector implements LinkClickObserverDelegate, FormInterceptorDelegate { readonly element: Element - readonly linkInterceptor: LinkInterceptor + readonly linkClickObserver: LinkClickObserver readonly formInterceptor: FormInterceptor constructor(element: Element) { this.element = element - this.linkInterceptor = new LinkInterceptor(this, element) + this.linkClickObserver = new LinkClickObserver(this, element) this.formInterceptor = new FormInterceptor(this, element) } start() { - this.linkInterceptor.start() + this.linkClickObserver.start() this.formInterceptor.start() } stop() { - this.linkInterceptor.stop() + this.linkClickObserver.stop() this.formInterceptor.stop() } - shouldInterceptLinkClick(element: Element, _url: string) { + willFollowLinkToLocation(element: Element) { return this.shouldRedirect(element) } - linkClickIntercepted(element: Element, url: string) { + followedLinkToLocation(element: Element, url: URL) { const frame = this.findFrameElement(element) if (frame) { - frame.delegate.linkClickIntercepted(element, url) + frame.delegate.followedLinkToLocation(element, url) } } diff --git a/src/core/frames/link_interceptor.ts b/src/core/frames/link_interceptor.ts deleted file mode 100644 index 65ff1066f..000000000 --- a/src/core/frames/link_interceptor.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { TurboClickEvent, TurboBeforeVisitEvent } from "../session" - -export interface LinkInterceptorDelegate { - shouldInterceptLinkClick(element: Element, url: string): boolean - linkClickIntercepted(element: Element, url: string): void -} - -export class LinkInterceptor { - readonly delegate: LinkInterceptorDelegate - readonly element: Element - private clickEvent?: Event - - constructor(delegate: LinkInterceptorDelegate, element: Element) { - this.delegate = delegate - this.element = element - } - - start() { - this.element.addEventListener("click", this.clickBubbled) - document.addEventListener("turbo:click", this.linkClicked) - document.addEventListener("turbo:before-visit", this.willVisit) - } - - stop() { - this.element.removeEventListener("click", this.clickBubbled) - document.removeEventListener("turbo:click", this.linkClicked) - document.removeEventListener("turbo:before-visit", this.willVisit) - } - - clickBubbled = (event: Event) => { - if (this.respondsToEventTarget(event.target)) { - this.clickEvent = event - } else { - delete this.clickEvent - } - } - - linkClicked = ((event: TurboClickEvent) => { - if (this.clickEvent && this.respondsToEventTarget(event.target) && event.target instanceof Element) { - if (this.delegate.shouldInterceptLinkClick(event.target, event.detail.url)) { - this.clickEvent.preventDefault() - event.preventDefault() - this.delegate.linkClickIntercepted(event.target, event.detail.url) - } - } - delete this.clickEvent - }) - - willVisit = ((_event: TurboBeforeVisitEvent) => { - delete this.clickEvent - }) - - respondsToEventTarget(target: EventTarget | null) { - const element = target instanceof Element ? target : target instanceof Node ? target.parentElement : null - return element && element.closest("turbo-frame, html") == this.element - } -} diff --git a/src/core/session.ts b/src/core/session.ts index 7b2bf24e7..44fbcd22f 100644 --- a/src/core/session.ts +++ b/src/core/session.ts @@ -5,7 +5,7 @@ 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 { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../observers/form_link_click_observer" import { getAction, expandURL, locationIsVisitable, Locatable } from "./url" import { Navigator, NavigatorDelegate } from "./drive/navigator" import { PageObserver, PageObserverDelegate } from "../observers/page_observer" @@ -38,7 +38,7 @@ export class Session implements FormSubmitObserverDelegate, HistoryDelegate, - FormLinkInterceptorDelegate, + FormLinkClickObserverDelegate, LinkClickObserverDelegate, NavigatorDelegate, PageObserverDelegate, @@ -53,11 +53,11 @@ export class Session readonly pageObserver = new PageObserver(this) readonly cacheObserver = new CacheObserver() - readonly linkClickObserver = new LinkClickObserver(this) + readonly linkClickObserver = new LinkClickObserver(this, document.documentElement) readonly formSubmitObserver = new FormSubmitObserver(this) readonly scrollObserver = new ScrollObserver(this) readonly streamObserver = new StreamObserver(this) - readonly formLinkInterceptor = new FormLinkInterceptor(this, document.documentElement) + readonly formLinkClickObserver = new FormLinkClickObserver(this, document.documentElement) readonly frameRedirector = new FrameRedirector(document.documentElement) drive = true @@ -70,12 +70,12 @@ export class Session if (!this.started) { this.pageObserver.start() this.cacheObserver.start() - this.formLinkInterceptor.start() + this.frameRedirector.start() + this.formLinkClickObserver.start() this.linkClickObserver.start() this.formSubmitObserver.start() this.scrollObserver.start() this.streamObserver.start() - this.frameRedirector.start() this.history.start() this.preloader.start() this.started = true @@ -91,12 +91,12 @@ export class Session if (this.started) { this.pageObserver.stop() this.cacheObserver.stop() - this.formLinkInterceptor.stop() + this.frameRedirector.stop() + this.formLinkClickObserver.stop() this.linkClickObserver.stop() this.formSubmitObserver.stop() this.scrollObserver.stop() this.streamObserver.stop() - this.frameRedirector.stop() this.history.stop() this.started = false } @@ -163,13 +163,13 @@ export class Session this.history.updateRestorationData({ scrollPosition: position }) } - // Form link interceptor delegate + // Form link click observer delegate - shouldInterceptFormLinkClick(_link: Element): boolean { + willSubmitFormLinkClickToLocation(): boolean { return true } - formLinkClickIntercepted(_link: Element, _form: HTMLFormElement) {} + submittedFormLinkToLocation() {} // Link click observer delegate diff --git a/src/elements/frame_element.ts b/src/elements/frame_element.ts index 7e8dfab5c..8adc1a535 100644 --- a/src/elements/frame_element.ts +++ b/src/elements/frame_element.ts @@ -16,7 +16,7 @@ export interface FrameElementDelegate { sourceURLChanged(): void disabledChanged(): void formSubmissionIntercepted(element: HTMLFormElement, submitter?: HTMLElement): void - linkClickIntercepted(element: Element, url: string): void + followedLinkToLocation(element: Element, url: URL): void loadResponse(response: FetchResponse): void fetchResponseLoaded: (fetchResponse: FetchResponse) => void visitCachedSnapshot: (snapshot: Snapshot) => void diff --git a/src/observers/form_link_click_observer.ts b/src/observers/form_link_click_observer.ts new file mode 100644 index 000000000..a5a43db42 --- /dev/null +++ b/src/observers/form_link_click_observer.ts @@ -0,0 +1,56 @@ +import { LinkClickObserver, LinkClickObserverDelegate } from "./link_click_observer" + +export type FormLinkClickObserverDelegate = { + willSubmitFormLinkClickToLocation(link: Element, location: URL, event: MouseEvent): boolean + submittedFormLinkToLocation(link: Element, location: URL, form: HTMLFormElement): void +} + +export class FormLinkClickObserver implements LinkClickObserverDelegate { + readonly linkClickObserver: LinkClickObserver + readonly delegate: FormLinkClickObserverDelegate + + constructor(delegate: FormLinkClickObserverDelegate, element: HTMLElement) { + this.delegate = delegate + this.linkClickObserver = new LinkClickObserver(this, element) + } + + start() { + this.linkClickObserver.start() + } + + stop() { + this.linkClickObserver.stop() + } + + willFollowLinkToLocation(link: Element, action: URL, originalEvent: MouseEvent): boolean { + return ( + this.delegate.willSubmitFormLinkClickToLocation(link, action, originalEvent) && + (link.hasAttribute("data-turbo-method") || link.hasAttribute("data-turbo-stream")) + ) + } + + followedLinkToLocation(link: Element, action: URL): void { + const form = document.createElement("form") + form.setAttribute("data-turbo", "true") + form.setAttribute("action", action.href) + form.setAttribute("hidden", "") + + const method = link.getAttribute("data-turbo-method") + if (method) form.setAttribute("method", method) + + const turboFrame = link.getAttribute("data-turbo-frame") + if (turboFrame) form.setAttribute("data-turbo-frame", turboFrame) + + const turboConfirm = link.getAttribute("data-turbo-confirm") + if (turboConfirm) form.setAttribute("data-turbo-confirm", turboConfirm) + + const turboStream = link.hasAttribute("data-turbo-stream") + if (turboStream) form.setAttribute("data-turbo-stream", "") + + this.delegate.submittedFormLinkToLocation(link, action, form) + + document.body.appendChild(form) + form.requestSubmit() + form.remove() + } +} diff --git a/src/observers/form_link_interceptor.ts b/src/observers/form_link_interceptor.ts deleted file mode 100644 index 6ac62a9bb..000000000 --- a/src/observers/form_link_interceptor.ts +++ /dev/null @@ -1,56 +0,0 @@ -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("data-turbo", "true") - form.setAttribute("action", action) - form.setAttribute("hidden", "") - - const method = link.getAttribute("data-turbo-method") - if (method) form.setAttribute("method", method) - - const turboFrame = link.getAttribute("data-turbo-frame") - if (turboFrame) form.setAttribute("data-turbo-frame", turboFrame) - - const turboConfirm = link.getAttribute("data-turbo-confirm") - if (turboConfirm) form.setAttribute("data-turbo-confirm", turboConfirm) - - const turboStream = link.hasAttribute("data-turbo-stream") - if (turboStream) form.setAttribute("data-turbo-stream", "") - - this.delegate.formLinkClickIntercepted(link, form) - - document.body.appendChild(form) - form.requestSubmit() - form.remove() - } -} diff --git a/src/observers/link_click_observer.ts b/src/observers/link_click_observer.ts index 8e359a2e0..218eb2ba1 100644 --- a/src/observers/link_click_observer.ts +++ b/src/observers/link_click_observer.ts @@ -7,33 +7,35 @@ export interface LinkClickObserverDelegate { export class LinkClickObserver { readonly delegate: LinkClickObserverDelegate + readonly eventTarget: EventTarget started = false - constructor(delegate: LinkClickObserverDelegate) { + constructor(delegate: LinkClickObserverDelegate, eventTarget: EventTarget) { this.delegate = delegate + this.eventTarget = eventTarget } start() { if (!this.started) { - addEventListener("click", this.clickCaptured, true) + this.eventTarget.addEventListener("click", this.clickCaptured, true) this.started = true } } stop() { if (this.started) { - removeEventListener("click", this.clickCaptured, true) + this.eventTarget.removeEventListener("click", this.clickCaptured, true) this.started = false } } clickCaptured = () => { - removeEventListener("click", this.clickBubbled, false) - addEventListener("click", this.clickBubbled, false) + this.eventTarget.removeEventListener("click", this.clickBubbled, false) + this.eventTarget.addEventListener("click", this.clickBubbled, false) } - clickBubbled = (event: MouseEvent) => { - if (this.clickEventIsSignificant(event)) { + clickBubbled = (event: Event) => { + if (event instanceof MouseEvent && this.clickEventIsSignificant(event)) { const target = (event.composedPath && event.composedPath()[0]) || event.target const link = this.findLinkFromClickTarget(target) if (link && doesNotTargetIFrame(link)) {