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.

Sign up for GitHub

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

Jump to bottom

Frames with loading lazy not working when src changes after restoration visit #886

Open
night91 opened this issue Mar 2, 2023 · 5 comments · Fixed by avo-hq/avo#3160 · May be fixed by #1227
Open

Frames with loading lazy not working when src changes after restoration visit #886

night91 opened this issue Mar 2, 2023 · 5 comments · Fixed by avo-hq/avo#3160 · May be fixed by #1227

Comments

@night91
Copy link

night91 commented Mar 2, 2023

I have been having troubles with frames with lazy loading after a restoration visit. It doesn't load when src changes (using a link with a data-turbo-frame). Removing the lazy in the inspector, it makes the frame to work again.

turbo/src/core/frames/frame_controller.ts

Line 116 in 4593d06

if (this.loadingStyle == FrameLoadingStyle.eager || this.hasBeenLoaded) {

if (this.loadingStyle == FrameLoadingStyle.eager || this.hasBeenLoaded) {

I have been digging into the code and after a restoration visit the loadSourceURL() doesn't get called because of loading lazy and the status of the hasBeenLoaded is false (lost value after the cache is loaded).

I am not sure about this because I don't have the full scope. Can someone verify this?

Version: v7.3.0
@kevinmcconnell
Copy link
Collaborator

@night91 yes, I can reproduce this. It does look like a bug to me.

With markup like the following, the link stops working after a restoration visit:

<body>
  <turbo-frame id="frame" src="/frame1.html" loading="lazy"></turbo-frame>

  <a href="/frame2.html" data-turbo-frame="frame">another frame</a>
</body>

And as you say, the problem is only when loading="lazy". Verified on both 7.2.0 & 7.3.0.

Thanks for reporting! 🙏

@AnomalousBit
Copy link

Just ran into this issue, thought I might share my quick fix until things get ironed out in the library.

Before the lazy loaded turbo-frame gets cached, remove the loading attribute but only if the frame is marked as complete. This allows frames that weren't yet lazy loaded to continue to function as expected.

addEventListener('turbo:before-cache', () => {
  document
    .querySelectorAll('turbo-frame[loading="lazy"][complete]')
    .forEach(frame => frame.removeAttribute('loading'))
})

@dineshpanda
Copy link

dineshpanda commented Nov 15, 2023 •
edited
Loading

It doesn't seem to be working with eager loaded frames as well.

class ArticlesController < ApplicationController
  def index
  end

  def source
    @articles = Article.all
  end
end

app/views/articles/index.html.erb

<%= turbo_frame_tag "articles", src: source_articles_path do %>
  Loading...
<% end %>

app/views/articles/source.html.erb

<%= turbo_frame_tag "articles" do %>
  <%= link_to "sort by title", articles_path(sort: :title), data: { turbo_action: "advance" } %>
  <%= render @articles %>
<% end %>

Clicking on "sort by title" link seems to be stuck at loading.
loading

Using turbo-rails (1.5.0)

@PedroAugustoRamalhoDuarte
Copy link

Just ran into this issue, thought I might share my quick fix until things get ironed out in the library.

Before the lazy loaded turbo-frame gets cached, remove the loading attribute but only if the frame is marked as complete. This allows frames that weren't yet lazy loaded to continue to function as expected.

addEventListener('turbo:before-cache', () => {
  document
    .querySelectorAll('turbo-frame[loading="lazy"][complete]')
    .forEach(frame => frame.removeAttribute('loading'))
})

I had a similar issue, this code that work for me:

// Handles turbo bug with lazy loading
// https://github.com/hotwired/turbo/issues/886
document.addEventListener('turbo:load', () => {
  document
    .querySelectorAll('turbo-frame[loading="lazy"][complete]')
    .forEach((frame) => frame.removeAttribute('loading'));
});

@yasu551 yasu551 linked a pull request Mar 19, 2024 that will close this issue
Open
This was referenced Aug 21, 2024
Merged
Closed
@adrianthedev
Copy link

You are a legend @PedroAugustoRamalhoDuarte! Thanks for the fix.
We are applying it to avo as we speak (avo-hq/avo#3160).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
6 participants
@kevinmcconnell @adrianthedev @AnomalousBit @dineshpanda @night91 @PedroAugustoRamalhoDuarte