Skip to content

Commit

Permalink
Navigate a lazy-loaded turbo-frame
Browse files Browse the repository at this point in the history
Problem
---

Consider a `<turbo-frame>` element with `loading="lazy"`. Once it
becomes visible, the `AppearanceObserver` will delegate to the
`FrameController` to load its contents.

However, once visible, subsequent navigations of that frame will
continue to be deferred, since its `[loading]` attribute is still
deferred, in spite of the fact that the element is visible on that page.

Proposed change
---

This commit proposes to immediately navigate a `<turbo-frame>` element
with `[loading="eager"]` _or_ a `<turbo-frame>` element that has been
loaded previously.

Possible alternatives
---

As an alternative to adding another conditional to the guard clause,
could it be worthwhile to treat navigational changes driven by `<a>`
clicks or `<form>` submissions different from programatically modifying
the element's `[src]` attribute? Do Custom Elements support changing an
attribute's value without also kicking off change callbacks?
  • Loading branch information
seanpdoyle committed Mar 16, 2021
1 parent 8bce5f1 commit 90a806d
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/core/frames/frame_controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
}

sourceURLChanged() {
if (this.loadingStyle == FrameLoadingStyle.eager) {
if (this.loadingStyle == FrameLoadingStyle.eager || this.hasBeenLoaded) {
this.loadSourceURL()
}
}
Expand Down Expand Up @@ -272,6 +272,10 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
return this.formSubmission !== undefined || this.loadingURL !== undefined
}

get hasBeenLoaded() {
return !!this.element.loaded
}

get isActive() {
return this.element.isActive
}
Expand Down
2 changes: 2 additions & 0 deletions src/tests/fixtures/frames/hello.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
<h1>Form</h1>
<turbo-frame id="hello">
<h2>Hello from a frame</h2>

<a href="/src/tests/fixtures/frames.html">Navigate #hello</a>
</turbo-frame>
</body>
</html>
14 changes: 14 additions & 0 deletions src/tests/functional/loading_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ export class LoadingTests extends TurboDriveTestCase {
this.assert.equal(await contents.getVisibleText(), "Hello from a frame")
}

async "test navigating a visible frame with loading=lazy navigates"() {
await this.clickSelector("#loading-lazy summary")
await this.nextBeat

const initialContents = await this.querySelector("#hello h2")
this.assert.equal(await initialContents.getVisibleText(), "Hello from a frame")

await this.clickSelector("#hello a")
await this.nextBeat

const navigatedContents = await this.querySelector("#hello h2")
this.assert.equal(await navigatedContents.getVisibleText(), "Frames: #hello")
}

async "test changing src attribute on a frame with loading=lazy defers navigation"() {
const frameContents = "#loading-lazy turbo-frame h2"
await this.nextBeat
Expand Down

0 comments on commit 90a806d

Please sign in to comment.