adjust after reverting hotwired#412

seanpdoyle · Oct 6, 2022 · 1b780ba · 1b780ba
1 parent c212ae7
commit 1b780ba
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 38 deletions.
diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts
@@ -26,7 +26,7 @@ import { FrameView } from "./frame_view"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
 import { FormLinkClickObserver, FormLinkClickObserverDelegate } from "../../observers/form_link_click_observer"
 import { FrameRenderer } from "./frame_renderer"
-import { TurboClickEvent, session } from "../index"
+import { session } from "../index"
 import { isAction, Action } from "../types"
 import { VisitOptions } from "../drive/visit"
 import { TurboBeforeFrameRenderEvent } from "../session"
@@ -214,8 +214,8 @@ export class FrameController
 
   // Link interceptor delegate
 
-  shouldInterceptLinkClick(element: Element, url: string, originalEvent: MouseEvent) {
-    return this.shouldInterceptNavigation(element) && this.frameAllowsVisitingLocation(element, url, originalEvent)
+  shouldInterceptLinkClick(element: Element, _url: string, _originalEvent: MouseEvent) {
+    return this.shouldInterceptNavigation(element)
   }
 
   linkClickIntercepted(element: Element, url: string) {
@@ -555,16 +555,6 @@ export class FrameController
     return expandURL(root)
   }
 
-  private frameAllowsVisitingLocation(target: Element, url: string, originalEvent: MouseEvent): boolean {
-    const event = dispatch<TurboClickEvent>("turbo:click", {
-      target,
-      detail: { url, originalEvent },
-      cancelable: true,
-    })
-
-    return !event.defaultPrevented
-  }
-
   private isIgnoringChangesTo(attributeName: FrameElementObservedAttribute): boolean {
     return this.ignoredAttributes.has(attributeName)
   }

diff --git a/src/core/frames/frame_redirector.ts b/src/core/frames/frame_redirector.ts
@@ -2,9 +2,6 @@ import { FormSubmitObserver, FormSubmitObserverDelegate } from "../../observers/
 import { FrameElement } from "../../elements/frame_element"
 import { LinkInterceptor, LinkInterceptorDelegate } from "./link_interceptor"
 import { expandURL, getAction, locationIsVisitable } from "../url"
-import { TurboClickEvent } from "../session"
-import { dispatch } from "../../util"
-
 export class FrameRedirector implements LinkInterceptorDelegate, FormSubmitObserverDelegate {
   readonly element: Element
   readonly linkInterceptor: LinkInterceptor
@@ -26,8 +23,8 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormSubmitObser
     this.formSubmitObserver.stop()
   }
 
-  shouldInterceptLinkClick(element: Element, url: string, originalEvent: MouseEvent) {
-    return this.shouldRedirect(element) && this.frameAllowsVisitingLocation(element, url, originalEvent)
+  shouldInterceptLinkClick(element: Element, _url: string, _originalEvent: MouseEvent) {
+    return this.shouldRedirect(element)
   }
 
   linkClickIntercepted(element: Element, url: string, originalEvent: MouseEvent) {
@@ -52,16 +49,6 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormSubmitObser
     }
   }
 
-  private frameAllowsVisitingLocation(target: Element, url: string, originalEvent: MouseEvent): boolean {
-    const event = dispatch<TurboClickEvent>("turbo:click", {
-      target,
-      detail: { url, originalEvent },
-      cancelable: true,
-    })
-
-    return !event.defaultPrevented
-  }
-
   private shouldSubmit(form: HTMLFormElement, submitter?: HTMLElement) {
     const action = getAction(form, submitter)
     const meta = this.element.ownerDocument.querySelector<HTMLMetaElement>(`meta[name="turbo-root"]`)
@@ -71,8 +58,17 @@ export class FrameRedirector implements LinkInterceptorDelegate, FormSubmitObser
   }
 
   private shouldRedirect(element: Element, submitter?: HTMLElement) {
-    const frame = this.findFrameElement(element, submitter)
-    return frame ? frame != element.closest("turbo-frame") : false
+    const isNavigatable =
+      element instanceof HTMLFormElement
+        ? this.session.submissionIsNavigatable(element, submitter)
+        : this.session.elementIsNavigatable(element)
+
+    if (isNavigatable) {
+      const frame = this.findFrameElement(element, submitter)
+      return frame ? frame != element.closest("turbo-frame") : false
+    } else {
+      return false
+    }
   }
 
   private findFrameElement(element: Element, submitter?: HTMLElement) {

diff --git a/src/tests/functional/frame_tests.ts b/src/tests/functional/frame_tests.ts
@@ -2,9 +2,9 @@ import { Page, test } from "@playwright/test"
 import { assert, Assertion } from "chai"
 import {
   attributeForSelector,
-  cancelNextEvent,
   hasSelector,
   innerHTMLForSelector,
+  listenForEventOnTarget,
   nextAttributeMutationNamed,
   noNextAttributeMutationNamed,
   nextBeat,
@@ -442,6 +442,7 @@ test("test navigating a frame from an outer form fires events", async ({ page })
 })
 
 test("test navigating a frame from an outer link fires events", async ({ page }) => {
+  await listenForEventOnTarget(page, "outside-frame-form", "turbo:click")
   await page.click("#outside-frame-form")
 
   await nextEventOnTarget(page, "outside-frame-form", "turbo:click")
@@ -456,14 +457,8 @@ test("test navigating a frame from an outer link fires events", async ({ page })
   assert.equal(otherEvents.length, 0, "no more events")
 })
 
-test("test canceling a turbo:cilck event falls back to built-in browser navigation", async ({ page }) => {
-  await cancelNextEvent(page, "turbo:click")
-  await Promise.all([page.waitForNavigation(), page.click("#link-frame")])
-
-  assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.html")
-})
-
 test("test navigating a frame from an inner link fires events", async ({ page }) => {
+  await listenForEventOnTarget(page, "link-frame", "turbo:click")
   await page.click("#link-frame")
 
   await nextEventOnTarget(page, "link-frame", "turbo:click")
@@ -479,6 +474,7 @@ test("test navigating a frame from an inner link fires events", async ({ page })
 })
 
 test("test navigating a frame targeting _top from an outer link fires events", async ({ page }) => {
+  await listenForEventOnTarget(page, "outside-navigate-top-link", "turbo:click")
   await page.click("#outside-navigate-top-link")
 
   await nextEventOnTarget(page, "outside-navigate-top-link", "turbo:click")

diff --git a/src/tests/helpers/page.ts b/src/tests/helpers/page.ts
@@ -100,6 +100,18 @@ export async function nextEventOnTarget(page: Page, elementId: string, eventName
   return record[1]
 }
 
+export async function listenForEventOnTarget(page: Page, elementId: string, eventName: string): Promise<void> {
+  return page.locator("#" + elementId).evaluate((element, eventName) => {
+    const eventLogs = (window as any).eventLogs
+
+    element.addEventListener(eventName, ({ target, type }) => {
+      if (target instanceof Element) {
+        eventLogs.push([type, {}, target.id])
+      }
+    })
+  }, eventName)
+}
+
 export async function nextAttributeMutationNamed(
   page: Page,
   elementId: string,