-
Notifications
You must be signed in to change notification settings - Fork 798
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
Lazy Images: Drop jQuery, convert to vanilla JS #14886
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
81c1260
Lazy Images: Drop jQuery, convert to vanilla JS
dero cff56d8
Lazy Images: Use the native events system
dero 16e6bfc
Lazy Images: Replace the image inline, don't clone
dero 3bf681a
Lazy Images: Back-compat for events
dero 485b0cc
Lazy Images: Fix the jQuery check
dero File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
/* globals IntersectionObserver, jQuery */ | ||
/* globals IntersectionObserver */ | ||
|
||
var jetpackLazyImagesModule = function( $ ) { | ||
var jetpackLazyImagesModule = function() { | ||
var images, | ||
config = { | ||
// If the image gets within 200px in the Y axis, start the download. | ||
|
@@ -12,15 +12,16 @@ var jetpackLazyImagesModule = function( $ ) { | |
image, | ||
i; | ||
|
||
$( document ).ready( function() { | ||
lazy_load_init(); | ||
lazy_load_init(); | ||
|
||
var bodyEl = document.querySelector( 'body' ); | ||
if ( bodyEl ) { | ||
// Lazy load images that are brought in from Infinite Scroll | ||
$( 'body' ).bind( 'post-load', lazy_load_init ); | ||
bodyEl.addEventListener( 'is.post-load', lazy_load_init ); | ||
|
||
// Add event to provide optional compatibility for other code. | ||
$( 'body' ).bind( 'jetpack-lazy-images-load', lazy_load_init ); | ||
} ); | ||
bodyEl.addEventListener( 'jetpack-lazy-images-load', lazy_load_init ); | ||
} | ||
|
||
function lazy_load_init() { | ||
images = document.querySelectorAll( 'img.jetpack-lazy-image:not(.jetpack-lazy-image--handled)' ); | ||
|
@@ -96,40 +97,48 @@ var jetpackLazyImagesModule = function( $ ) { | |
* @param {object} image The image object. | ||
*/ | ||
function applyImage( image ) { | ||
var theImage = $( image ), | ||
srcset, | ||
var srcset, | ||
sizes, | ||
theClone; | ||
lazyLoadedImageEvent; | ||
|
||
if ( ! theImage.length ) { | ||
if ( ! image instanceof HTMLImageElement ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea |
||
return; | ||
} | ||
|
||
srcset = theImage.attr( 'data-lazy-srcset' ); | ||
sizes = theImage.attr( 'data-lazy-sizes' ); | ||
theClone = theImage.clone(true); | ||
srcset = image.getAttribute( 'data-lazy-srcset' ); | ||
sizes = image.getAttribute( 'data-lazy-sizes' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
// Remove lazy attributes from the clone. | ||
theClone.removeAttr( 'data-lazy-srcset' ), | ||
theClone.removeAttr( 'data-lazy-sizes' ); | ||
theClone.removeAttr( 'data-lazy-src' ); | ||
// Remove lazy attributes. | ||
image.removeAttribute( 'data-lazy-srcset' ), | ||
image.removeAttribute( 'data-lazy-sizes' ); | ||
image.removeAttribute( 'data-lazy-src' ); | ||
|
||
// Add the attributes we want. | ||
image.classList.add( 'jetpack-lazy-image--handled' ); | ||
image.setAttribute( 'data-lazy-loaded', 1 ); | ||
|
||
// Add the attributes we want on the finished image. | ||
theClone.addClass( 'jetpack-lazy-image--handled' ); | ||
theClone.attr( 'data-lazy-loaded', 1 ); | ||
if ( ! srcset ) { | ||
theClone.removeAttr( 'srcset' ); | ||
} else { | ||
theClone.attr( 'srcset', srcset ); | ||
} | ||
if ( sizes ) { | ||
theClone.attr( 'sizes', sizes ); | ||
image.setAttribute( 'sizes', sizes ); | ||
} | ||
|
||
theImage.replaceWith( theClone ); | ||
if ( ! srcset ) { | ||
image.removeAttribute( 'srcset' ); | ||
} else { | ||
image.setAttribute( 'srcset', srcset ); | ||
} | ||
|
||
// Fire an event so that third-party code can perform actions after an image is loaded. | ||
theClone.trigger( 'jetpack-lazy-loaded-image' ); | ||
try { | ||
lazyLoadedImageEvent = new Event( 'jetpack-lazy-loaded-image', { | ||
bubbles: true, | ||
cancelable: true | ||
} ); | ||
} catch ( e ) { | ||
lazyLoadedImageEvent = document.createEvent( 'Event' ) | ||
lazyLoadedImageEvent.initEvent( 'jetpack-lazy-loaded-image', true, true ); | ||
} | ||
|
||
image.dispatchEvent( lazyLoadedImageEvent ); | ||
} | ||
}; | ||
|
||
|
@@ -865,4 +874,8 @@ var jetpackLazyImagesModule = function( $ ) { | |
}(window, document)); | ||
|
||
// Let's kick things off now | ||
jetpackLazyImagesModule( jQuery ); | ||
if ( document.readyState === 'interactive' || document.readyState === "complete" ) { | ||
jetpackLazyImagesModule(); | ||
} else { | ||
document.addEventListener( 'DOMContentLoaded', jetpackLazyImagesModule ); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -351,7 +351,7 @@ public function enqueue_assets() { | |
'_inc/build/lazy-images/js/lazy-images.min.js', | ||
'modules/lazy-images/js/lazy-images.js' | ||
), | ||
array( 'jquery' ), | ||
array(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
JETPACK__VERSION, | ||
true | ||
); | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note:
While it may feel like it, switching from jQuerytrigger
to dispatching native events does not introduce a regression. There are two notable differences in the usage of both:jQuerytrigger
creates proprietary events that are only observable using jQuery (on
,bind
etc.). Meaning that there is no 3rd party code out there usingaddEventListener
to observe them.The nativedispatchEvent
is observable using bothaddEventListener
andjQuery
methods. Importantly, jQuery will normalize the incoming event and expose any additionaldetail
data in a jQuery way.This means any pre-existing handler is not going to be affected by a change from the proprietary🎉trigger
to nativedispatchEvent
.All of the above is incorrect. See #14886 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the issue reported by @jeherve disproves the above. It now seems jQuery listeners (
on
,bind
) do not pass thedetail
data as a second parameter to event handlers after all. I've however tested that behavior before and I'm not certain why I'm seeing a different outcome now. I will revisit this issue tomorrow.