bbc / Imager.js

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

Interpolation of src-attribute #124

Open mindplay-dk opened 9 years ago

mindplay-dk commented 9 years ago

Please consider adding an interpolation hook for the src attribute value, e.g.:

@@ -124,6 +124,7 @@
         this.widthsMap        = {};
         this.refreshPixelRatio();
         this.widthInterpolator = opts.widthInterpolator || returnFn;
+               this.srcInterpolator   = opts.srcInterpolator || returnFn;

         // Needed as IE8 adds a default `width`/`height` attribute…
         this.gif.removeAttribute('height');
@@ -359,9 +360,12 @@
     };

     Imager.prototype.changeImageSrcToUseNewImageDimensions = function (src, selectedWidth) {
-        return src
-            .replace(/{width}/g, Imager.transforms.width(selectedWidth, this.widthsMap))
-            .replace(/{pixel_ratio}/g, Imager.transforms.pixelRatio(this.devicePixelRatio));
+               var width      = Imager.transforms.width(selectedWidth, this.widthsMap),
+                       pixelRatio = Imager.transforms.pixelRatio(this.devicePixelRatio);
+
+               return this.srcInterpolator(src
+            .replace(/{width}/g, width)
+            .replace(/{pixel_ratio}/g, pixelRatio), width, pixelRatio);
     };

I had to add this because the images in my scenario have unpredictable filenames, which cannot be computed on the client-side. I think this is a pretty common scenario? Other responsive image loaders provide some means of handling this scenario.

My precise solution was to use image placeholder with like this:

<div data-src='{"low":"/photo/cache/16/16_280_0_12_0_576_360_2_fb45.jpg","high":"/photo/cache/16/16_720_0_12_0_576_360_2_e2b9.jpg"}' class="responsive-image"></div>

With Imager configured like this:

            var images = new Imager('.responsive-image', {
                availableWidths: { 280: "low", 720: "high" },
                srcInterpolator: function(src, width) {
                    return jQuery.parseJSON(src)[width];
                }
            });

It's a fairly simple solution. The only alternative I could come up with, was to create a dedicated instance of Imager for every individual image, which seems really ineffecient, and would mean I'd have to generate JS to load every individual image.

thom4parisot commented 9 years ago

Interesting use case. Although would not availableWidths and widthInterpolator be a good fit? But indeed it could not, as you have no other ways to alter other parts of the URL.

We could either make the tokens system pluggable or have a last resort src.replace(//

<div data-src='/photo/cache/16/16_{width}_0_12_0_576_360_2_{cache-token}.jpg' data-cache-tokens='{"high": "e2b9", "low": "fb45"}' class="responsive-image"></div>
var imgr = new Imager({
  availableWidths: { 280: "low", 720: "high" },
  expressions: {
    //width: Imager.transforms.width,             # provided by Imager.js
    //pixel_ratio: Imager.transforms.pixelRatio,  # provided by Imager.js
    'cache-token': function(image, width){ return JSON.parse(image.dataset.cacheTokens)[width]; }
  }
});

Let me know your feelings about it.

mindplay-dk commented 9 years ago

To use this approach, I would have to first break down the URLs and extract the cache tokens - that's more complexity, and also, these URLs are coming from an external API; the URL format is just an artifact, which could change in the future.

What you propose could work for me though, I would just use data-src="{url}" and something like expressions: { url: function(...) { ... } }.

On the other hand, if you did have {width} and {cache-token} or a bunch of other URL pieces, what I proposed would work just as well, just with a single callback and doing my own str.replace() calls to fill the placeholders.

The net result is a URL based on image and width either way - personally I'd prefer the simpler approach over locking in a token format and adding the concept of "expressions". Everyone knows what a URL is :-)

thom4parisot commented 9 years ago

Well then in that case this is the whole src attribute which should be replaceable to implement your own logic, but with sizes computed by Imager.js (which is what you want in the end).

If you have loads of URL, it would be better to have a single URL hash store somewhere in the page rather than to parse JSON on every image replacement. This will not scale well in a page.

mindplay-dk commented 9 years ago

Could we cache the source for each width? Parsing JSON (or any other costly operation) should then be a non-issue - compute the source once for every width of every image, rather than computing it for every image every time. It's not like the source changes for the same width. (does it?)

thom4parisot commented 9 years ago

Yes we could but again, it does not sound like a generic use case: with a pluggable src interpolator, you do whatever you want:

var imageCache = {}; // or whatever, like a WeakMap or something
var imgr = new Imager({ srcInterpolator: customFn });

function customFn(src, image, computedWidth){
  if (!imageCache[image.id]) {
    imageCache[image.id] = JSON.parse(image.dataset.src);
  }

  return imageCache[image.id][computedWidth].replace(/{pixel_ratio}/, Imager.transforms.pixelRatio);
}

// Constructor
function Imager(selector, options) {
  // ...
  this.srcInterpolator = (options.srcInterpolator || Imager.interpolators.src).bind(this);
  // ...
}

// Imager default interpolator
Imager.interpolators.src = function (src, image, selectedWidth) {
        return src
            .replace(/{width}/g, Imager.transforms.width(selectedWidth, this.widthsMap))
            .replace(/{pixel_ratio}/g, Imager.transforms.pixelRatio(this.devicePixelRatio));
    };
mindplay-dk commented 9 years ago

I don't mean just caching the result of the src-interpolator, I mean caching the resulting src attribute fully - I don't see why an optimization isn't a generic use case?

Unless you think there are really cases where somebody might want different sources at different times? I can't think of any.

Caching the resulting source should speed things up when resizing the browser window, with or without src-interpolation. (?)

thom4parisot commented 9 years ago

Yep caching per image and size could be a good addition indeed. That sounds like a good idea :-)

The cache should work with any kind of src interpolator. It is just a hashmap of strings in the end, ahead of the interpolation.

mindplay-dk commented 9 years ago

I added caching to my fork with src-interpolation.