guardian / frontend

The Guardian DotCom.
https://theguardian.com
Other
5.84k stars 554 forks source link

Lightbox is requesting too big images. Lightbox is requesting too small images. #12216

Open paperboyo opened 8 years ago

paperboyo commented 8 years ago

Hello,

Related: https://github.com/guardian/frontend/pull/12079

There is so much more we could do to the Lightbox starting with fixing the mobile experience. Or small things like still occurring https://github.com/guardian/frontend/issues/6144. Or the big things. Really big :-)

Regards Mateusz

rich-nguyen commented 8 years ago

This issue is two-fold:

The advantage to using srcsets is we don't account for viewport, the browser does. But this assumes the srcset is correct, and its not possible for the browser to do anything sensible for this image because the srcset is specifying 'height-over-sized' image content, as mentioned.

Broadly, here are the goals for this task:

  1. Update the lightbox and gallery systems to use <picture> and media=.
  2. Modify server logic to use a new concept, HeightsByBreakpoint, to complement the existing WidthsByBreakpoint. This would fix all portrait images on the website.

Sounds simple!

paperboyo commented 8 years ago

Big thanks for the thorough explanation and a plan. One small thing we may consider only for Lightbox images is using fm=pjpg imgIX parameter to (only perceptually?) speed up the carousel. Would require some tests, as I'm pretty sure, q is not quality-normalised between jpg and pjpg (not that it is among most other formats...).

rich-nguyen commented 8 years ago

That sounds like something we could try on certain breakpoints, so maybe use pjpg when the <source> element's media= attribute is targeting either large breakpoints or dpr > 1.25?

paperboyo commented 8 years ago

Not sure if I would like to see images loosing blurriness in random order on a front or in an article... Also, as I said, I don't think imgIX guarantees same perceptual quality for the same q parameter among jpg and pjpg. In Lightbox, some initial blurriness would only affect one image and, hopefully, make it appear sooner (that would be the whole, and only, point here). Also, in Lightbox, because the images are so big and beautiful (esp. when they will suddenly support dpr=2), the possible lowering of quality due to pjpg would be easier to live with.

That was only a thought, if it presents any additional work to make it work only for Lightbox - let's forget about it, I would say :-)

oliverlloyd commented 1 year ago

I wouldn't say this is perfection but this change might help here.

It's all a bit arbitrary and depends on consistent ratios and having a width (and maybe height as well if we're not assuming ratio) set for master images but I think these assumptions play out