From 33c7b702fd27a2b4f7fcef72c84f529f959987bf Mon Sep 17 00:00:00 2001
From: Sean Doyle <sean.p.doyle24@gmail.com>
Date: Mon, 22 Jan 2024 10:18:16 -0500
Subject: [PATCH] Drive `turbo-frame` with `Turbo.visit(url, { frame:, action:
 })`

Closes [#489][]
Closes [#468][]

First, add test coverage to exercise the `<turbo-frame id="frame">` and
`<turbo-frame id="hello" target="frame">` as outlined in a
[comment on #489][].

Next, add coverage to support driving a `<turbo-frame
data-turbo-action="advance">` through the `Turbo.visit` method. To pass
that test coverage, invoke
`frameElement.delegate.proposeVisitIfNavigatedWithAction(frameElement)`
prior to modifying the element's `[src]` attribute.

To support the implementation changes necessary to pass the tests, this
commit changes the `proposeVisitIfNavigatedWithAction(frame, action)`
interface by flattening the variable arguments into a single `action`
argument, then moving the `getVisitAction` call to the call sites that
require it. The call sites that know their `action` pass it in directly
when its available.

[#489]: https://github.com/hotwired/turbo/issues/489
[#468]: https://github.com/hotwired/turbo/issues/468
[comment on #489]: https://github.com/hotwired/turbo/issues/489#issuecomment-1503078653
---
 src/core/frames/frame_controller.js | 10 ++++-----
 src/core/session.js                 |  4 +++-
 src/tests/fixtures/frames.html      |  4 ++--
 src/tests/functional/frame_tests.js | 33 +++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/core/frames/frame_controller.js b/src/core/frames/frame_controller.js
index 02358d123..e82e097f4 100644
--- a/src/core/frames/frame_controller.js
+++ b/src/core/frames/frame_controller.js
@@ -147,7 +147,7 @@ export class FrameController {
   // Appearance observer delegate
 
   elementAppearedInViewport(element) {
-    this.proposeVisitIfNavigatedWithAction(element, element)
+    this.proposeVisitIfNavigatedWithAction(element, getVisitAction(element))
     this.#loadSourceURL()
   }
 
@@ -235,7 +235,7 @@ export class FrameController {
   formSubmissionSucceededWithResponse(formSubmission, response) {
     const frame = this.#findFrameElement(formSubmission.formElement, formSubmission.submitter)
 
-    frame.delegate.proposeVisitIfNavigatedWithAction(frame, formSubmission.formElement, formSubmission.submitter)
+    frame.delegate.proposeVisitIfNavigatedWithAction(frame, getVisitAction(formSubmission.submitter, formSubmission.formElement, frame))
     frame.delegate.loadResponse(response)
 
     if (!formSubmission.isSafe) {
@@ -340,15 +340,15 @@ export class FrameController {
   #navigateFrame(element, url, submitter) {
     const frame = this.#findFrameElement(element, submitter)
 
-    frame.delegate.proposeVisitIfNavigatedWithAction(frame, element, submitter)
+    frame.delegate.proposeVisitIfNavigatedWithAction(frame, getVisitAction(submitter, element, frame))
 
     this.#withCurrentNavigationElement(element, () => {
       frame.src = url
     })
   }
 
-  proposeVisitIfNavigatedWithAction(frame, element, submitter) {
-    this.action = getVisitAction(submitter, element, frame)
+  proposeVisitIfNavigatedWithAction(frame, action = null) {
+    this.action = action
 
     if (this.action) {
       const pageSnapshot = PageSnapshot.fromElement(frame).clone()
diff --git a/src/core/session.js b/src/core/session.js
index 7bfb20ca2..eb0c9880c 100644
--- a/src/core/session.js
+++ b/src/core/session.js
@@ -98,8 +98,10 @@ export class Session {
     const frameElement = options.frame ? document.getElementById(options.frame) : null
 
     if (frameElement instanceof FrameElement) {
+      const action = options.action || getVisitAction(frameElement)
+
+      frameElement.delegate.proposeVisitIfNavigatedWithAction(frameElement, action)
       frameElement.src = location.toString()
-      frameElement.loaded
     } else {
       this.navigator.proposeVisit(expandURL(location), options)
     }
diff --git a/src/tests/fixtures/frames.html b/src/tests/fixtures/frames.html
index d6a82d831..24c3272b9 100644
--- a/src/tests/fixtures/frames.html
+++ b/src/tests/fixtures/frames.html
@@ -27,7 +27,7 @@ <h2>Frames: #frame</h2>
       <form action="/src/tests/fixtures/frames/frame.html">
         <button id="frame-form-get-no-redirect">Navigate #frame without a redirect</button>
       </form>
-      <button id="add-turbo-action-to-frame" type="button">Add [data-turbo-action="advance"] to frame</button>
+      <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-frame-with-search-params" href="/src/tests/fixtures/frames/frame.html?key=value">Navigate #frame with ?key=value</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>
@@ -57,7 +57,7 @@ <h2>Frames: #frame</h2>
     <turbo-frame id="hello" target="frame">
       <h2>Frames: #hello</h2>
 
-      <a href="/src/tests/fixtures/frames/frame.html">Load #frame</a>
+      <a id="hello-link-frame" href="/src/tests/fixtures/frames/frame.html">Load #frame</a>
       <button type="button" id="remove-target-from-hello">Remove #hello[target]</button>
 
     </turbo-frame>
diff --git a/src/tests/functional/frame_tests.js b/src/tests/functional/frame_tests.js
index b090aff8a..f1925908b 100644
--- a/src/tests/functional/frame_tests.js
+++ b/src/tests/functional/frame_tests.js
@@ -652,6 +652,39 @@ test("navigating turbo-frame[data-turbo-action=advance] from within pushes URL s
   assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame.html")
 })
 
+test("navigating turbo-frame[data-turbo-action=advance] from outside with target pushes URL state", async ({ page }) => {
+  await page.click("#add-turbo-action-to-frame")
+  await page.click("#hello-link-frame")
+  await nextEventNamed(page, "turbo:load")
+
+  await expect(page.locator("h1")).toHaveText("Frames")
+  await expect(page.locator("#frame h2")).toHaveText("Frame: Loaded")
+  expect(pathname(page.url())).toEqual("/src/tests/fixtures/frames/frame.html")
+})
+
+test("navigating turbo-frame[data-turbo-action=advance] with Turbo.visit pushes URL state", async ({ page }) => {
+  const path = "/src/tests/fixtures/frames/frame.html"
+
+  await page.click("#add-turbo-action-to-frame")
+  await page.evaluate((path) => window.Turbo.visit(path, { frame: "frame" }), path)
+  await nextEventNamed(page, "turbo:load")
+
+  await expect(page.locator("h1")).toHaveText("Frames")
+  await expect(page.locator("#frame h2")).toHaveText("Frame: Loaded")
+  expect(pathname(page.url())).toEqual(path)
+})
+
+test("navigating turbo-frame without advance with Turbo.visit specifying advance pushes URL state", async ({ page }) => {
+  const path = "/src/tests/fixtures/frames/frame.html"
+
+  await page.evaluate((path) => window.Turbo.visit(path, { frame: "frame", action: "advance" }), path)
+  await nextEventNamed(page, "turbo:load")
+
+  await expect(page.locator("h1")).toHaveText("Frames")
+  await expect(page.locator("#frame h2")).toHaveText("Frame: Loaded")
+  expect(pathname(page.url())).toEqual(path)
+})
+
 test("navigating turbo-frame[data-turbo-action=advance] to the same URL clears the [aria-busy] and [data-turbo-preview] state", async ({
   page
 }) => {