Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigate a lazy-loaded turbo-frame #212

Merged
merged 1 commit into from Apr 7, 2021

Conversation

seanpdoyle
Copy link
Contributor

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?

@seanpdoyle seanpdoyle added the bug Something isn't working label Apr 1, 2021
@sstephenson
Copy link
Contributor

Intuitively I'd expect setting src on a lazy frame to take effect as soon as the element intersects the viewport, but a quick test shows that <img loading=lazy> seems to behave the way you describe above. So, let's roll with it. Could you rebase this against main?

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?
@seanpdoyle
Copy link
Contributor Author

@sstephenson I've rebased and updated this branch.

@sstephenson sstephenson merged commit db96ea9 into hotwired:main Apr 7, 2021
@sstephenson
Copy link
Contributor

Thank you!

@seanpdoyle seanpdoyle deleted the frame-loading-lazy-anchor branch April 7, 2021 03:46
en30 added a commit to en30/turbo that referenced this pull request Jul 24, 2021
`<turbo-frame>`'s loading behaviour was disscued in
hotwired#212 .
This commit try to represent the intent as code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants