From 00b287af773952d6e122a19356a6cdadf5fd86d2 Mon Sep 17 00:00:00 2001
From: Sean Doyle <seanpdoyle@users.noreply.github.com>
Date: Tue, 8 Nov 2022 08:44:28 -0500
Subject: [PATCH] Fix: Promoting lazy-loaded Frames (#790)

Prior to this commit, `<turbo-frame>` elements navigated via the
`AppearanceObserver` (powered by the [loading=lazy][] attribute) were
not accounting for their [data-turbo-action][] attributes.

To resolve that issue, this commit changes the `AppearanceObserver` and
`AppearanceObserverDelegate` to utilize a parameterized `<T extends
Element>` type so that the delegate callbacks can be invoked with
`FrameElement` references. With those changes in place, this commit
changes the `FrameController.elementAppearedInViewport` callback's
invocation to prepare a `PageSnapshot` and attempt to
`proposeVisitIfNavigatedWithAction`.

[loading=lazy]: https://turbo.hotwired.dev/reference/frames#lazy-loaded-frame
[data-turbo-action]: https://turbo.hotwired.dev/reference/frames#frame-that-promotes-navigations-to-visits
---
 src/core/frames/frame_controller.ts            |  8 +++++---
 src/observers/appearance_observer.ts           | 12 ++++++------
 src/tests/fixtures/frame_navigation.html       |  7 +++++++
 src/tests/functional/frame_navigation_tests.ts | 14 +++++++++++++-
 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts
index cc1e40f57..86fa98aa8 100644
--- a/src/core/frames/frame_controller.ts
+++ b/src/core/frames/frame_controller.ts
@@ -38,7 +38,7 @@ export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: Vi
 
 export class FrameController
   implements
-    AppearanceObserverDelegate,
+    AppearanceObserverDelegate<FrameElement>,
     FetchRequestDelegate,
     FormSubmitObserverDelegate,
     FormSubmissionDelegate,
@@ -49,7 +49,7 @@ export class FrameController
 {
   readonly element: FrameElement
   readonly view: FrameView
-  readonly appearanceObserver: AppearanceObserver
+  readonly appearanceObserver: AppearanceObserver<FrameElement>
   readonly formLinkClickObserver: FormLinkClickObserver
   readonly linkInterceptor: LinkInterceptor
   readonly formSubmitObserver: FormSubmitObserver
@@ -198,7 +198,9 @@ export class FrameController
 
   // Appearance observer delegate
 
-  elementAppearedInViewport(_element: Element) {
+  elementAppearedInViewport(element: FrameElement) {
+    this.pageSnapshot = PageSnapshot.fromElement(element).clone()
+    this.proposeVisitIfNavigatedWithAction(element, element)
     this.loadSourceURL()
   }
 
diff --git a/src/observers/appearance_observer.ts b/src/observers/appearance_observer.ts
index 65518429f..7a622a481 100644
--- a/src/observers/appearance_observer.ts
+++ b/src/observers/appearance_observer.ts
@@ -1,14 +1,14 @@
-export interface AppearanceObserverDelegate {
-  elementAppearedInViewport(element: Element): void
+export interface AppearanceObserverDelegate<T extends Element> {
+  elementAppearedInViewport(element: T): void
 }
 
-export class AppearanceObserver {
-  readonly delegate: AppearanceObserverDelegate
-  readonly element: Element
+export class AppearanceObserver<T extends Element> {
+  readonly delegate: AppearanceObserverDelegate<T>
+  readonly element: T
   readonly intersectionObserver: IntersectionObserver
   started = false
 
-  constructor(delegate: AppearanceObserverDelegate, element: Element) {
+  constructor(delegate: AppearanceObserverDelegate<T>, element: T) {
     this.delegate = delegate
     this.element = element
     this.intersectionObserver = new IntersectionObserver(this.intersect)
diff --git a/src/tests/fixtures/frame_navigation.html b/src/tests/fixtures/frame_navigation.html
index 11cdc37ff..50020cb79 100644
--- a/src/tests/fixtures/frame_navigation.html
+++ b/src/tests/fixtures/frame_navigation.html
@@ -17,6 +17,13 @@ <h2>Frame Navigation</h2>
         <a id="self" href="/src/tests/fixtures/frame_navigation.html" data-turbo-frame="_self">Self Frame</a>
         <a id="top" href="/src/tests/fixtures/frame_navigation.html" data-turbo-frame="_top">Top</a>
       </turbo-frame>
+
+      <div style="height: calc(100vh*2);"></div>
+
+      <turbo-frame id="eager-loaded-frame" src="/src/tests/fixtures/frames/frame_for_eager.html" loading="lazy"
+                   data-turbo-action="advance">
+        <h2>Eager-loaded frame: Not Loaded</h2>
+      </turbo-frame>
     </div>
   </body>
 </html>
diff --git a/src/tests/functional/frame_navigation_tests.ts b/src/tests/functional/frame_navigation_tests.ts
index fbdac2a51..3a2fadde1 100644
--- a/src/tests/functional/frame_navigation_tests.ts
+++ b/src/tests/functional/frame_navigation_tests.ts
@@ -1,5 +1,5 @@
 import { test } from "@playwright/test"
-import { getFromLocalStorage, nextEventNamed, nextEventOnTarget, pathname } from "../helpers/page"
+import { getFromLocalStorage, nextEventNamed, nextEventOnTarget, pathname, scrollToSelector } from "../helpers/page"
 import { assert } from "chai"
 
 test("test frame navigation with descendant link", async ({ page }) => {
@@ -30,6 +30,18 @@ test("test frame navigation emits fetch-request-error event when offline", async
   await nextEventOnTarget(page, "tab-frame", "turbo:fetch-request-error")
 })
 
+test("test lazy-loaded frame promotes navigation", async ({ page }) => {
+  await page.goto("/src/tests/fixtures/frame_navigation.html")
+
+  assert.equal(await page.textContent("#eager-loaded-frame h2"), "Eager-loaded frame: Not Loaded")
+
+  await scrollToSelector(page, "#eager-loaded-frame")
+  await nextEventOnTarget(page, "eager-loaded-frame", "turbo:frame-load")
+
+  assert.equal(await page.textContent("#eager-loaded-frame h2"), "Eager-loaded frame: Loaded")
+  assert.equal(pathname(page.url()), "/src/tests/fixtures/frames/frame_for_eager.html")
+})
+
 test("test promoted frame navigation updates the URL before rendering", async ({ page }) => {
   await page.goto("/src/tests/fixtures/tabs.html")