Skip to content
This repository has been archived by the owner on Nov 27, 2019. It is now read-only.

Add Retina support #146

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

mkoistinen
Copy link
Contributor

Solves #77

Yes, this is an opinionated PR, but we're in the latter half of 2014 and we still don't have Retina support for cmsplugin_filer_image. Some say that we should be doing retina images in CSS. Sure, but what about cases where operators need to upload images? This is precisely what filer is for, and this is precisely where support for Retina/High-DPI belongs.

This is a relatively simple change that does the following:

  1. Adds a checkbox in FilerImage model/plugin labeled "Create a pair of images for Retina/HiDPI.";
  2. When this is checked, the template for filer will add an image attribute data-src with the normal source, and data-src2x with the Retina/HiDPI image source at 2X the supplied dimensions. The normal img tag’s src will reference a 1x1 transparent GIF in cmsplugin-filer's static files;
  3. Also when it is checked, uses Sekazai, to add a very small snippet of JS to insert the correct image into the src, based on whether window.devicePixelRatio is greater than 1.5.

NOTE: This is a change as I've now noticed that Sekazi and jQuery are already dependencies in this project. If jQuery is not desired/required in the project, then overriding the cmsplugin_filer_image/plugins/images/default.html template is all that is required. Here's some appropriate, pure JS one could use:

//
// Enables Hi-DPI images in pure JS, however, please note:
// - Not compatible with IE7 or lower because of querySelectorAll().
// - Not compatible with IE6 or lower because of attribute selectors.
//  '[data-src][data-src2x]', to get around this, filter by class name
//  '.filer_image_hidpi' instead.
//
(function(){
  var use_hidpi = (window.devicePixelRatio > 1.5),
      imgs = document.querySelectorAll('[data-src][data-src2x]');
  for (var i=0, img; i<imgs.length; i++){
    img = imgs[i];
    img.setAttribute('src', use_hidpi ? img.getAttribute('data-src2x') : img.getAttribute('data-src'));
  }
}());

Yes, there will be issues with non-JS environments... most likely screen readers, which won't need/want the images anyway. Also, there's a myth that some people don't enable JS. I've checked on a pretty busy site... there were exactly zero non-bots (i.e., real people) who disabled their JS. Sure, there are cases where the site's audience is likely to not use JS, in these cases, the web developer should simply not check the box labeled: "Create a pair of images for Retina/HiDPI." checkbox and the behavior will just remain as it is now.

Happy to hear suggestions on how to improve this, but its pretty simple and very effective already and has a very low footprint on the overall solution. I'm using it now with very nice results, and the result has brought my PageSpeed Insights score from 65 to 97 (it really didn't like my oversized images before...)

@yakky
Copy link
Member

yakky commented Sep 15, 2014

👍

@mkoistinen
Copy link
Contributor Author

@stefanfoulis any thoughts on this PR? Its pretty solid at this time. What can I do to get this blessed (aka committed)?

@jrief
Copy link

jrief commented Sep 17, 2014

Just my two cents:

Easy-Thumbnails already supports this, using the global options
THUMBNAIL_HIGHRES_INFIX and THUMBNAIL_HIGH_RESOLUTION, so there is not need to make things more complicate, by changing the model and adding an option for highres images to each image. With THUMBNAIL_HIGH_RESOLUTION set, both images are created using the infix @2x or alternatively _2x to distinguish them.

On the Javascript side, there is a well proven library named: http://imulus.github.io/retinajs/ for those using jQuery. For those who prefer AngularJS, there is https://github.com/jrief/angular-retina

Sorry to say this, but I currently don't see any reason to pull this request, so this gets a -1 from my side.

@yakky
Copy link
Member

yakky commented Sep 17, 2014

@jrief does we have documentation on how to implement retina images with cmsplugin_filer + easy-thumbnails + retina.js?

@mkoistinen
Copy link
Contributor Author

Hi Jacob, I'm well aware of both Easy Thumbnails feature and retina.js. However, Easy thumbnails needs special configuration to make it create these 2X versions–and the worst part is, once you do, it creates 2X versions for everything, even if you never use them or needed them in the first place. Then, you have to also install a 3rd party JS library to make it go (retina.js, as you pointed out above). This PR is all inclusive and very light weight. You don't need to install anything more than filer to make it work (since jQuery is already required, if not, then its simple to replace the jQuery with the JS I posted above), and it doesn't mean configuring your easy-thumbnails to make 2X of everything your clients upload. Perhaps this is nice for you, but it isn't for every project.

@jrief
Copy link

jrief commented Sep 17, 2014

@yakky
in settings.py add

THUMBNAIL_HIGH_RESOLUTION = True

Then, somewhere in one of the base templates:

<script type="text/javascript" src="//cdnjs.cloudflare.com/ajax/libs/retina.js/1.3.0/retina.min.js"></script>

@mkoistinen
Copy link
Contributor Author

Bear in mind that doing the above, it means all of your uploads will now be rendered into normal and _2X variants immediately upon upload, regardless of how large they are, or even if you will never use them. So, if you want 1 thing rendered in HiDPI/Retina, Jacob's solution is to double-render all images and to install an additional dependency into your project. This is a fine solution, but not for all projects. What I am proposing is completely innocuous. If you don't want to use it, just don't check the box and things remain exactly as they did before.

Also, retina.js, as nice as it is, still has to check if there's another image. Each check is at least a HEAD request to your server. If you already know it is available, why not just provide this information in the markup?

@mkoistinen
Copy link
Contributor Author

@stefanfoulis do you have an opinion on this? You know mine already from the above.

@jrief
Copy link

jrief commented Oct 28, 2014

@mkoistinen
Today, for another project, I again came across this theme. There are few interesting things to note here. The W3C has two proposals, one is based on the <picture> element (read http://www.w3.org/html/wg/drafts/html/master/embedded-content.html#the-picture-element) and one is based on the srcset tag inside the <img ...> element (read http://www.w3.org/html/wg/drafts/html/master/embedded-content.html#attr-img-srcset).

Currently, both of them are natively implemented in Chrome (v38). For other browsers there is a Javascript implementation Picturefill to emulate this behavior.

Both implementations can handle responsive, as well as high resolution images. In my opinion, cmsplugin-filer shall be aware of these emerging standards and be implemented accordingly. Otherwise we have another island solution.

BTW: Today I reimplemented the Picture plugin in djangocms-cascade. It makes full usage of the possibilities given by the <picture> element.

@stefanfoulis
Copy link
Contributor

I like the simplicity of @mkoistinen s approach. I'm not to well versed in JS development, so I don't know how well the JS part of that solution would work in all the browsers out there. So I'd prefer using a library that is already been used a lot and is optimises for all those edge-cases.

Since easy-thumbnails already supports creating the highres version, why not use that functionality? It can also be applied on a per image basis: {% thumbnail obj.image 50x50 crop HIGH_RESOLUTION %} (http://easy-thumbnails.readthedocs.org/en/latest/ref/settings/#easy_thumbnails.conf.Settings.THUMBNAIL_HIGH_RESOLUTION), so it's not necessary to create hight-res versions of everything. I'd say for the image plugin it should generate the high-res version by default, but allow deactivating it in an "advanced" option.

@FinalAngel what do you think about retina.js vs PictureFill? Are there other solutions?

@jrief
Copy link

jrief commented Oct 29, 2014

As I wrote yesterday, in Chrome 38 Javascript is not even required, to get this working. Its already built into that browser. Please check http://caniuse.com/#search=picture for details.

I'd therefore go for a solution, which will become a new standard, rather than adding some hackish JS. Therefore picturefill.js, in my opinion is the best solution.

@mkoistinen
Copy link
Contributor Author

Since easy-thumbnails already supports creating the highres version, why not use that functionality? It can also be applied on a per image basis: {% thumbnail obj.image 50x50 crop HIGH_RESOLUTION %} (http://easy-thumbnails.readthedocs.org/en/latest/ref/settings/#easy_thumbnails.conf.Settings.THUMBNAIL_HIGH_RESOLUTION), so it's not necessary to create hight-res versions of everything. I'd say for the image plugin it should generate the high-res version by default, but allow deactivating it in an "advanced" option.

I like that approach.

@FinalAngel
Copy link
Member

hey guys,

  • srcset on the <img> is the favorite of the community currently, it has also wider support as the <picture> counterpart (http://caniuse.com/#search=srcset)
  • I do oppose the idea of implementing a feature that is javascript dependent. The myth of supporting a website with no javascript might affect your SEO is partially wrong, yet the approach is usability wise correct. Even WAI-ARIA still persists on content to be viewable without javascript.
  • I'd strive for a solution that can be handled on a global scale (such as @stefanfoulis thumbnail suggestion) rather than a specific implementation within filer

@mkoistinen
Copy link
Contributor Author

@FinalAngel, I'm not 100% clear about what the difference between the implementation I provided and the one you support with:

I'd strive for a solution that can be handled on a global scale (such as @stefanfoulis thumbnail suggestion) rather than a specific implementation within filer.

Unless you are talking about using Retina.js. But this seems to contradict your earlier point about JS dependencies. =/

What am I missing?

@jrief
Copy link

jrief commented Oct 30, 2014

@FinalAngel
I haven't seen any specification that the <picture> element is deprecated in favor of srcset. Please point me to any literature, if I'm wrong here.

From my point of view, the implementation for <picture> is cleaner and more flexible and we'll see, which version will become more popular in the future. Anyway, in cmsplugin-filer, it should be quite easy to replace one against the other using another template.

I fully agree, that we should use a method independent of Javascript. Therefore retina.js and angular-retina (which I've written myself), have to be considered as hacks. The only valid reason to use Javascript here is for a shim, such as picturefill.js. Is there anything equivalent JS for <img> with srcset?

@FinalAngel
Copy link
Member

@jrief I do not consider the picture element as its still in the HTML 5.1 working draft (http://www.w3.org/html/wg/drafts/html/master/embedded-content.html#the-picture-element) but the img element is already available and backwards compatible with most browsers without any additional javascript. <picture> and <img ... srcset=""> are both within the current 5.1 draft. They both were moved a couple of months ago to one single place, thats where my outdated information about de deprecation came from. Yet we have to see when 5.1 is fully supported by the majority of the browsers and see what will happen within the community.

@mkoistinen its fully correct that I do not suggest to use Retina.js as a general solution but to use the {% thumbnail %} tag to generate retina images and find a solution per-project basis. This field is currently going through monthly changes and the community is still quite opinionated about the approach, myself included.

@jrief
Copy link

jrief commented Nov 19, 2014

@FinalAngel
yesterday I went through the full specs for <img ... srcset=""> and this changed my mind. I now fully support this in favor of the <picture>...</picture> element, as it is less error-prone. Libraries such as Retina.js or hand written JS shall be considered as workaround to solve that problem temporarily, until a better solution is o the way.
This better solution now is <img ... srcset="">.

@mkoistinen
Since <img ... srcset="">is an emerging standard, and already supported by Google Chrome and Opera, such a solution will be future-proof and run without a single line of JavaScript. For legacy browsers use the fantastic picturefill.js library.

Therefore, my proposal is to add an extra Boolean field allowing the image to be optimized for Retina, but then render it using <img src="normal.jpg" srcset="normal.jpg 1x, double.jpg 2x" sizes="200w" alt="Alt text">. This also might work: <img src="normal.jpg" srcset="normal.jpg 1x, double.jpg 2x" width="200" height="100" alt="Alt text">, please test.
Note: normal.jpg is 200x100 pixels, while double.jpg is 400x200 pixels here.
With src="normal.jpg" this then will even work on legacy browsers without any JavaScript library such as picturefill.js - in single resolution though.

@yakky
Copy link
Member

yakky commented Mar 1, 2015

@FinalAngel @jrief @mkoistinen given that the specs has been finalised and is now a standard https://html.spec.whatwg.org/multipage/embedded-content.html#embedded-content), could we rework this PR to comply with the approved standard?

@jrief
Copy link

jrief commented Mar 1, 2015

@yakky
Using Javascript sending a HEAD request to determine, if the 2x picture is available, is a really ugly hack, invented about 2-3 years ago.

I fully support this, and would go with <img src="normalsize.jpg" srcset="doublesize.jpg 2x">. This does not require any fixes on easythumbnails, since images are already transformed as 1x and 2x there. So its only about rendering images with 2x in the admin interface, using the above technique.

BTW: djangocms-cascade already supports these kind of images.

@stefanfoulis
Copy link
Contributor

+1 for the srcset implementation. I'd recommend explicitly creating the double size image though, instead of relying on the global 2x option of easy-thumbnails, because then we're not forced to create a 2x version for every image and it will work for people who don't want the global 2x option active.

@mkoistinen
Copy link
Contributor Author

This PR does NOT "use Javascript to send a HEAD request" at all; that's what RetinaJS does, which is what @jrief proposed earlier, oddly.

This PR is quite declarative. If you need Retina, you click a box and the additional image is created, the JS determines which image is more suitable for that client, and displays that one. It only ever creates @2x versions when its been asked and works well in virtually every browser.

@yakky
Copy link
Member

yakky commented Mar 3, 2015

My opinion on this it's that declaring an opt-in for 2x images is good, but this PR should just use easy-thumbnails option to create the @2x image by passing HIGH_RESOLUTION parameter to the templatetag in case the flag is selected; I'd also move the flag to ThumbnailOption and use a future-proof attribute by using srcset to add the high resolution image and write the JS code to act as a polyfill for browser that does not support the standard syntax

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants