Add background image support through optional background param

[darcyclarke]

So I've gotten a bit tired of seeing continued discussion on a very old issue (#29); Either way, I decided to do something about it. I've included background-image support through an optional background param that you can set when creating a new imagesLoaded instance. Examples of how to use this:

// 1. Find elements with class .bg-image that have a background image
var imgLoader = new imagesLoaded('.bg-image', { background: true }); 

// 2. Find *any* children with a background image inside #container or #container itself
var imgLoader = new imagesLoaded('#container', { background: true });

The implementation for this has been simplified to allow for the most flexibility. That said, it’s better to be specific with your selector strings (as shown in example 1). Otherwise, we have no way of filtering an elements children and are subsequently forced to iterate and check each.

Please let me know if you’d like me to make any changes to the implementation, add more tests etc..

[arxpoetica]

Yeah!

[desandro]

Thanks so much for kicking this off. I've been reluctant to get into this. Having someone step up to get the ball rolling is a huge morale boost.

Your contribution is a good start, but I feel there's plenty of more work to be done. Some issues

1. Check for local urls
2. Only checking child elements is limiting. I think proving a way to select or filter for an child element would be suitable.
3. ImagesLoaded only tells you which image has loaded. For elements with multiple background images, it would be useful to know which element has had its images load

1 is a simple fix. 2 is a bit more complex, but can be done. 3 requires serious consideration. It would mean changing the API to allow for this functionality. So I'm considering doing a bigger overhaul that could accommodate background images in a way that I feel completely addresses these issues.

I've started background branch for this work. I'll be working on this in the weeks to come.

[darcyclarke]

Great feedback @desandro! Some thoughts...

1. Done!
2. My original implementation actually utilized a className string, instead of the background boolean; and I actually did use it as a way to filter child elements instead of iterating over all the children. That said, it seemed a bit redundant when a developer was already passing a collection of elements or a selector string on instantiation. In my mind, the best way to filter elements is to just be more specific with your selector string and not rely on element discovery (img elements or otherwise). Removing child filtering, as a whole, makes sense to me (ie. https://github.com/desandro/imagesloaded/blob/master/imagesloaded.js#L145-L150 too). Not sure what the benefit is of being able to write a #container selector vs. #container img or #container img, #container .bg-image
3. Done-ish! (depending on your feedback/thoughts). I think ImagesLoaded is fine as it is. I think over engineering for a very specific use-case isn't that helpful (especially when ImagesLoaded really is about images... loading... and not elements).
   That said, I have added a LoadingImage.img.originalElement property that references an image's origin, if it has one (ie. only background images). It doesn't solve the problem of firing an event when all of an element's background images have loaded but does give context which wasn't there before. The naming convention was chosen as I thought the utility would be somewhat similar to jQuery's event.originalEvent (ish). I'm open to changing the name or implementation but think this does somewhat solve the problem.

If you really needed to know when all of an element's background images were loaded you could write something like:

var container = document.querySelector( '#container' );
var element = container.querySelector( 'div' );
var imgLoader = new imagesLoaded( container , { background: true } );

var progress = 0;
var regex = /\burl\s*\(\s*["']?([^"'\r\n\)\(]+)["']?\s*\)/gi;
var total = element.style.background.match( regex ).length;

imgLoader.on( 'progress' , function ( instance, image ) {
  if ( image.img.originalElement ===  element) {
    progress++;
  }
  if ( progress >= total ) {
    // all background images are loaded for that element...
  }
});

Or, even simpler... just create a new imagesLoaded instance for each element with background images.

var elements = document.querySelectorAll( '.bg-image' );
for ( var i=0, len = elements.length; i < len; i++ ) {
    var imgLoader = new imagesLoaded( elements[i], { background: true } ); 
    imgLoader.on( 'done', function( instance ) { 
      // all background images are loaded for that element...
    });
}

Again, to me, it just seems simpler if the developer writes this kind of specific use-case code.

Either way, please definitely feel free to look over the code again as I've rewritten history a bit here (ie. rebased, squashed, force-pushed etc.) and let me know your thoughts.

[mc-zone]

Cool! This support is exactly what I am looking for! Would be a great help for me. Thank you all!

[darcyclarke]

@desandro Like I said above, I think this is a good stop-gap for any kind of bigger refactor you're going to do in terms of firing events on both an image load and all images loaded on a multi-background image element. Definitely let me know if there's something I'm missing or anything terribly wrong with this implementation. Keys here are that it doesn't change the API at all but just adds unobtrusive support for background images.

Note: Again, the code was updated from the original pull request (and probably the last time you perused it)... I rebased and squashed my commits down so that there was still only a single commit (apologize beforehand as it doesn't make it very easy to see where I made changes).

[desandro]

Adding background images is a huge API change. While this may work as a stop gap, I don't feel comfortable implementing a stop-gap solution like this for such an important feature.

• The API might change with
• I haven't done extensive browser testing
• There's other concerns I don't feel settled on: like if this should target only elements, and not their children; and if it's worth specifying when all the background images of an element have been loaded

In short, if we're going to add this feature, I want to be comprehensive about it.

I realize the waiting game is painful. But good news is that I have more personal availability to devote to code. I do appreciate your patience. This will happen.

[darcyclarke]

Should I just close this PR then?

[desandro]

Let's keep it open. It'll help others to chime in or use your fork in the meantime.

[darcyclarke]

Sounds good. Removed background image detection of child elements, for my own sanities sake.

[darcyclarke]

Ping @desandro... it looks like no work has been done on this project in awhile and I'd like to think we don't want to fragment efforts. It looks like there's a number of PRs that could be merged in safely/easily. Any update, specifically, on the work relevant to this PR? I guess I still don't see why the work I've done can't be merged in.

[desandro]

Hello! Thanks for the ping. Man, I've let things get unkept here. Frustration is justified.

Thinking it over, I feel best course of action is to maintain your own fork if you want to see things moving. I don't feel ready to start making changes and I don't want to hold separate progress back.

This issue is just one of several that's I'm on the fence with. I needed imagesLoaded to support Masonry & other libraries, but its usage has grown outside that:

• background images
• SVG
• srcset, <picture>

The bigger issue is whether or not imagesLoaded to should support stuff that Masonry + co. don't really need.

[swampthang]

How stable is the background branch? Think it would be safe for production?

[desandro]

🎉 ⭐ 🌈 imagesLoaded v3.2.0 now supports background images 🌈 ⭐ 🎉 Try it out!

Thanks again for getting the ball rolling on this. Closing this PR now.

---

Set { background: true } to detect when the element's background image has loaded.

// jQuery
$('#container').imagesLoaded( { background: true }, function() {
  console.log('#container background image loaded');
});

// vanilla JS
imagesLoaded( '#container', { background: true }, function() {
  console.log('#container background image loaded');
});

Set to a selector string like { background: '.item' } to detect when the background images of child elements have loaded.

// jQuery
$('#container').imagesLoaded( { background: '.item' }, function() {
  console.log('all .item background images loaded');
});

// vanilla JS
imagesLoaded( '#container', { background: '.item' }, function() {
  console.log('all .item background images loaded');
});