Skip to content

Commit

Permalink
Fix: Promoting lazy-loaded Frames (hotwired#790)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
seanpdoyle authored Nov 8, 2022
1 parent 22fd88a commit 00b287a
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 10 deletions.
8 changes: 5 additions & 3 deletions src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export type TurboFrameMissingEvent = CustomEvent<{ response: Response; visit: Vi

export class FrameController
implements
AppearanceObserverDelegate,
AppearanceObserverDelegate<FrameElement>,
FetchRequestDelegate,
FormSubmitObserverDelegate,
FormSubmissionDelegate,
Expand All @@ -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
Expand Down Expand Up @@ -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()
}

Expand Down
12 changes: 6 additions & 6 deletions src/observers/appearance_observer.ts
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
7 changes: 7 additions & 0 deletions src/tests/fixtures/frame_navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
14 changes: 13 additions & 1 deletion src/tests/functional/frame_navigation_tests.ts
Original file line number Diff line number Diff line change
@@ -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 }) => {
Expand Down Expand Up @@ -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")

Expand Down

0 comments on commit 00b287a

Please sign in to comment.