sal0max / grav-plugin-shortcode-gallery-plusplus

A Shortcode extension to add sweet galleries with a lightbox to your Grav website.
MIT License
33 stars 8 forks source link

Use <img srcset=""> for thumbnails #32

Closed ghost closed 1 year ago

ghost commented 1 year ago

Hello, it's me again ! 😆 Let me summarize : In the PR #16, I proposed my thumbnail generation system, in order to optimize the page loading. The problem with this system is that the thumbnails were also displayed in the RSS or Atom feed aggregators. So in the PR #25, I proposed to disable the caching of the page in order to generate a special version without thumbnails for the feeds.

But I don't like this solution. It's a hack, and weakens Grav's biggest strength : Caching. The ideal solution would be to generate thumbnails for browsers, while keeping the original image in the "src" field for aggregators. Where I was stupid, is that this solution already exists in HTML5 : It's the srcset attribute of the <img> element !

The problem with this attribute is that the browser chooses the resolution to load, and it depends on the pixel density of the device (DPR). So I decided to generate two thumbnails: One for devices with DPR=2, and one for DRP=3. The ones above will load the original image.

There are also some problems with Justified Gallery which does not support srcset, I let you look at my commits for more technical details.

By the way, I left the functionality to not generate the gallery in the feeds when caching is disabled, because it's still nicer and cleaner, but I no longer encourage the user to disable caching just for that.

Anyway, it is increasingly complicated, if you do not want to merge this PR, I understand perfectly. Have a nice day ! 😀

ghost commented 1 year ago

Okay, I think it's good, and I don't think we can do better. I tried to tweak something with thumbnailPath and lazy-loading, but it doesn't work, the browser always loads the images before Justified Gallery initializes. And anyway I think it would be a bigger hack than what I propose here with srcset. So I'll leave it at that ("Je vais en rester lĂ "). ^^

When you'll merge (Or reject) this PR, I'll have another PR to finish what I did at #17 : It will move the inline JS code at the bottom of the HTML, as it was in your original version. 😉

PS : I also notice that my last commit ( d6b01ee01c85a073494dd5bbd7b981c1a0ddb617 ) allows the gallery to stay pretty much clean even if the JS has not been executed. That's cool. I wasn't convinced by what I coded, but actually it's not bad. Better than disabling caching anyway ! 😆

ghost commented 1 year ago

I didn't know it did that...

sal0max commented 1 year ago

Thanks for your outstanding work! Gladly merging this. ❤️

ghost commented 1 year ago

Thanks, and your welcome ! ^^