bbc / Imager.js

Responsive images while we wait for srcset to finish cooking
Apache License 2.0
3.84k stars 224 forks source link

Measuring element width in a layout that uses flexbox layout #123

Open mindplay-dk opened 9 years ago

mindplay-dk commented 9 years ago

Working on a site that uses flexbox CSS layout, I ran into an issue where only the lowest-resolution image would get loaded, even after resizing.

I narrowed it down to image.parentNode.clientWidth always returning zero in the determineAppropriateResolution function.

Adding a second fallback to offsetWidth seems to fix the issue:

Imager.prototype.determineAppropriateResolution = function (image) {
  return Imager.getClosestValue(image.getAttribute('data-width') || image.parentNode.clientWidth || image.parentNode.offsetWidth, this.availableWidths);
};

My problem isn't completely fixed- it still doesn't initially load the correct size image, but this seems to be an important step on the way.

mindplay-dk commented 9 years ago

Okay, it seems the parent node was an <a> tag which did not have display:block, and therefore did not have any measurable size - having fixed that, it now works without the added image.parentNode.offsetWidth fallback.

I wonder if the script ought to handle this case, and what would be the appropriate way to handle it? Throw an exception? Log the error to the console?

This sort of issue going to be common for sure, and should be handled somehow, I think?

thom4parisot commented 9 years ago

Throwing an exception would be aggressive but failing silently is neither good.

So I would suggest to opt in for an explicitly written exception message, with an additional console.error (if available) message to help debugging the troublesome DOM element.

Otherwise, ideally another solution would be to walk up the DOM tree until we find an element with a width.

mindplay-dk commented 9 years ago

ideally another solution would be to walk up the DOM tree until we find an element with a width

Won't that lead to even-harder to debug cases? where you can't figure out where the width is coming from or why. I like simple, predictable, consistent behavior - walking up the tree means any of a number of elements could end up determining the width. Using the immediate parent, in my opinion, is cleaner and more predictable.

mindplay-dk commented 9 years ago

Maybe close this issue and open a new issue for improved error-handling?