Closed ausi closed 10 years ago
We could also let the users decide which polyfill they want to use, by adding both scripts and let them choose one in the layout settings.
+1
Agreed! Make sure you adjust the hint "you have to activate xy.js to support responsive images..." :-)
Why would I need to choose a script if both do the same or one is superior???
Why would I need to choose a script if both do the same or one is superior???
Hm, good point actually.
If that is a valid point I wonder why we didn't go for dynamically adding the script when a picture template gets used in the frist place ;) It's way easier for the users...they don't have to do anything, it just works out of the box :)
Also, because respimage might allow us to omit passing DOM elements (although it should be possible through options.elements
). If that is the case, we should make it the default.
Why would I need to choose a script if both do the same or one is superior???
They don’t do exactly the same and IMO it depends on the use case which one you should use. Respimage is 37% larger than picturefill, but respimage seems to trash fewer images which are already loaded, see http://afarkas.github.io/responsive-image-race/. But if you omit the src
attribute, maybe picturefill works better. It may also depend on which image size you use as the fallback size.
If that is a valid point I wonder why we didn't go for dynamically adding the script when a picture template gets used in the frist place ;) It's way easier for the users...they don't have to do anything, it just works out of the box :)
<picture>
and srcset=
have a great fallback built in if the src
attribute is used. The polyfill is not required to see an image. So the user should be able decide if he wants a polyfill or not, and IMO he should also decide which one.
Also, because respimage might allow us to omit passing DOM elements (although it should be possible through
options.elements
). If that is the case, we should make it the default.
I think it is always better to call the polyfill ASAP. We could change the JS to something like this to make it work with both polyfills:
(function(p) {
p && p({
elements: [document.images[document.images.length - 1]]
});
})(window.picturefill || window.respimage);
While I think your points are all valid, I wonder if we really want to do that...I mean, how many percent of all the users do you think will actually understand what the script does and in what case you should include either of the scripts or none at all? Personally, I think we should provide a default way which does not require the user to do anything at all and is sane for most use cases. If you want to have an expert setting to adjust that behavior we can still add this but the default way should be KISS. :-)
Personally, I think we should provide a default way which does not require the user to do anything at all and is sane for most use cases.
I think this is already the case because the default is to not load the polyfill. In most use cases no polyfill is needed, and browsers without support get a great fallback.
I'm with @ausi on this one.
I'm with all of you on this, but: why do we even include a script then? If I want one or the other polyfill, I can add that to my custom scripts. As @ausi pointed out, most of the time no script is needed at all.
Because we want to make life easy for Contao users? If we decide to add a new feature, we should support it thoroughly and not force people to install external scripts in order to use it.
@ausi I'm going to take care of integrating the second script. Can you please fix the open issues in your pull request? Would be great if we could publish the release candidate next week.
Can you please fix the open issues in your pull request?
@leofeyer Yes, if open issues regards https://github.com/contao/core/issues/7351#issuecomment-58797974
Exactly :)
Would be great to have the option to load the polyfills with the async
attribute.
Implemented in 11b6b0f13818d86a0300f88f8390a48ceb7f1a8f.
@leofeyer Can we make the async flag optional? Like 'options'=> array('picturefill.js', 'picturefill.js async', 'respimage.js', 'respimage.js async‘)
Why?
Because |static
and |static|async
produce two separate script tags and therefore two requests. Depending on how many scripts are added to |static
and |static|async
, loading everything via |static
may be better for performance. On the other hand if many other scripts are loaded via |static|async
it may be better to load the polyfill asynchronously.
Also if the polyfill is loaded asynchronously it could result in a „flash of unreplaced images“ which might be fine or not, depending on the users preference and the settings of the image size.
Also if the polyfill is loaded asynchronously it could result in a „flash of unreplaced images“
Hm, then I actually would prefer not to load the script asynchronously. I doubt that anyone would prefer a "flash of unreplaced images" :)
What if you have something like srcset="foo.jpg 1x, bar.jpg 2x"
? A „flash of unreplaced images“ would be no problem here. It would be problematic if the proportion of the replacing image is different than the fallback image.
Once many browsers support <picture>
it may be OK to have a „flash of unreplaced images“ in older browsers, to get faster load times in new browsers.
There is a new
<picture>
polyfill available called respimage from aFarkas (the author of html5shiv). It’s based on picturefill.Respimage should be better at detecting not to download a source candidate, because an image with a good resolution was already downloaded, which is important, because we don’t omit the
src
attribute.We could also let the users decide which polyfill they want to use, by adding both scripts and let them choose one in the layout settings.
Related: #7296