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

Add data-preload attribute for iframes #2354

Merged
merged 1 commit into from
Mar 28, 2019
Merged

Add data-preload attribute for iframes #2354

merged 1 commit into from
Mar 28, 2019

Conversation

maxrothman
Copy link
Contributor

Allows lazy-loaded (i.e. data-src) iframes to be preloaded when they
come within the viewDistance, rather than once they're visible.

Replaces #2353, which was against master rather than dev.

@maxrothman
Copy link
Contributor Author

A couple of interesting things I noticed while working on this change:

  • <img>s aren't unloaded when they leave the viewDistance. I assume this is intentional, since the cost of downloading them has already been paid?
  • When preloaded iframes are loaded, the src attribute replaces the data-src attribute. For non-preloaded iframes (i.e. the previous behavior), the src attribute is added in addition to the data-src attribute. Was this intentional? Is it worth the effort to make them behave the same way?

@chreds
Copy link

chreds commented Mar 22, 2019

This is great! Thanks for the commit. Should the console.log() be removed from line 3718?

@maxrothman
Copy link
Contributor Author

Yes, whoops 😳

Allows lazy-loaded (i.e. data-src) iframes to be preloaded when they
come within the viewDistance, rather than once they're visible.
@maxrothman
Copy link
Contributor Author

Pinging @hakimel who reviewed a previous iteration of this PR

@hakimel
Copy link
Owner

hakimel commented Mar 25, 2019

Looks great! Will find time to review and test this soon.

No need to ping me btw – I receive notifications for everything in this repo :)

@hakimel hakimel merged commit 93b1abc into hakimel:dev Mar 28, 2019
hakimel added a commit that referenced this pull request Mar 28, 2019
@hakimel
Copy link
Owner

hakimel commented Mar 28, 2019

This has been merged now!

I added some tests and made a few smaller changes in d6f0f41

@maxrothman
Copy link
Contributor Author

Awesome! Any sense of when the next release is going out?

@hakimel
Copy link
Owner

hakimel commented Apr 1, 2019

3.8.0 was just released and includes this update.

@maxrothman
Copy link
Contributor Author

party

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants