aryehraber / statamic-responsive-img

Statamic v2 Addon that automatically handles responsive images using srcset.
MIT License
12 stars 1 forks source link

FR: Add lazy-loading option (using data-attributes) #2

Closed aerni closed 6 years ago

aerni commented 6 years ago

In some cases it is crucial to be able to iterate over an array of assets. For instance if you want to retrieve a title or alt tag. As well I mostly need more control over the img tag. For instance to add custom data-attributes for lazy-loading. Is this possible?

My current implementation for responsive images looks something like this.

---
slider:
  - /assets/images/image-1.jpg
  - /assets/images/image-2.jpg
---

{{ assets:slider }}
    <img data-src="{{ glide:url preset="1000" }}" class="swiper-lazy"
        data-srcset="
            {{ glide:url preset="1400" }} 1400w,
            {{ glide:url preset="1200" }} 1200w,
            {{ glide:url preset="1000" }} 1000w,
            {{ glide:url preset="800" }} 800w,
            {{ glide:url preset="600" }} 600w,
            {{ glide:url preset="400" }} 400w"
        sizes="
            (min-width: 1921px) 30vw,
            (min-width: 1800px) 40vw,
            (min-width: 1200px) 50vw,
            (min-width: 940px) 60vw,
            (min-width: 640px) 85vw,
            100vw"
        title="{{ title }}"
        alt="{{ alt }}">
{{ /assets:slider }}

As well I wonder if you could add a settings file for image quality, min and max sizes and the like.

aryehraber commented 6 years ago

I built in a quality option and forgot to add it to the docs πŸ˜…

You should be able to add the option like so: {{ responsive_img:image quality="50" }} (default is 75)

To convert your current implementation, you should be able to use the tag inside the assets loop:

{{ assets:slider }}
  {{ responsive_img:id attr="class:swiper-lazy|title:{ title }|alt:{ alt }" }}
{{ /assets:slider }}

As for the title and alt attributes, I'll consider adding dedicated options for these, though for the moment you can add them via the attr option as demonstrated above.

I'll also give min and max sizes some thought -- the point of the addon is that it does that completely automatically based on the source image (max is the max image width, and it automatically reduces each version by ~30% until it's really small). Perhaps just a max option would be a good idea.

aerni commented 6 years ago

I think the max size makes sense to be the size of the uploaded image.

It would be great to be able to set the percentage each image will be reduced by. Just for some additional control.

What about the custom data-attributes for lazy-loading purposes? Can this be added? "data-src" instead of "src" and "data-srcset" instead of "srcset". The "data-" will be removed with JavaScript as soon as the image has to be loaded.

aryehraber commented 6 years ago

Ok, thanks for the feedback -- I'll have a look into adding some extra options for additional control.

Custom data-attributes for lazy-loading sounds like a good FR! Can't promise a release date, but I'll definitely add it to my todo list.

aerni commented 6 years ago

Best would be to have a settings.yaml. Thanks for your amazing work! This is such a great addon!

aryehraber commented 6 years ago

Glad to hear it, thanks!

Yeah, a global setting for the addon would be best, with the ability to overwrite on a per tag level.

Btw, if you're in a rush for this feature, I'm happy to merge a PR if you want to take a crack at it πŸ˜„

aerni commented 6 years ago

Sounds great! I would love to help you out. But I'm just a humble Front End Dev. πŸ˜…

gmcz commented 6 years ago

I'd like to use this addon, but without lazy-load I wont be able to implement it on some of the sites I manage. This is the lazy-load method I currently use. I'd be interested in getting it integrated here too.

https://varvy.com/pagespeed/defer-images.html

aryehraber commented 6 years ago

Hey guys, thanks again for your input!

I've just tagged a new release which includes support for data-attrubutes! Please let me know how this plays with each of your JS implementations and whether there are any further tweaks needed.

The new version is available on Statamic's Marketplace: https://statamic.com/marketplace/addons/responsive-img

Thanks!

aerni commented 6 years ago

I updated the addon and only get an error now. statamic-2018-09-04.log

aryehraber commented 6 years ago

@aerni90 Ah crap, I forgot to rename a file πŸ˜… Please try again in about 10 min.

aryehraber commented 6 years ago

Not sure how long it'll take for Statamic's Marketplace to pick up on the update, but for now you can download the fix directly here on GH: https://github.com/aryehraber/statamic-responsive-img/releases/tag/v1.1.1

aerni commented 6 years ago

Now I'm getting this error.

[2018-09-04 09:13:25] dev.ERROR: Symfony\Component\Debug\Exception\FatalErrorException: Method Illuminate\View\View::__toString() must not throw an exception, caught ErrorException: Undefined variable: imageWidth (View: /Users/michael/Valet/baerenmatte/site/addons/ResponsiveImg/resources/views/svg.blade.php) in /Users/michael/Valet/baerenmatte/site/addons/ResponsiveImg/ResponsiveImg.php:63
Stack trace:
#0 {main}
aryehraber commented 6 years ago

@aerni90 Sorry, missed another renamed variable. I need to get my addon workflow in place on this new laptop...

Fixed in latest release: https://github.com/aryehraber/statamic-responsive-img/releases/tag/v1.1.2

aryehraber commented 6 years ago

ps. thanks for helping debug and sending logs, really helps!

aerni commented 6 years ago

That's the least I can do! Just downloaded v.1.1.2. Still getting the same error though.

aryehraber commented 6 years ago

Hmm, that’s odd...

Can you ensure that the cache is cleared, and also use a new image for testing in case the result is cached.

If that still shows an error, please reshare the log or first few lines (even if it seemingly looks the same).

aerni commented 6 years ago

I cleared the cache and tried with a new image. Still the same problem

[2018-09-04 09:50:16] dev.ERROR: Symfony\Component\Debug\Exception\FatalErrorException: Method Illuminate\View\View::__toString() must not throw an exception, caught ErrorException: Undefined variable: imageWidth (View: /Users/michael/Valet/baerenmatte/site/addons/ResponsiveImg/resources/views/svg.blade.php) in /Users/michael/Valet/baerenmatte/site/addons/ResponsiveImg/ResponsiveImg.php:63
Stack trace:
#0 {main}  

Weird enough the image gets rendered now but with errors. Check this screenshot. bildschirmfoto 2018-09-04 um 09 51 07

aryehraber commented 6 years ago

That's so strange! The error is referring to a variable that I 100% renamed in v1.1.2.

Are you completely sure you updated correctly? (By just deleting the entire ResponsiveImg folder and then adding the new release)

aerni commented 6 years ago

Yes, I did exactly that. Deleting the old folder and adding the new one. I'm going to try it on another Statamic installation and see how that goes.

aryehraber commented 6 years ago

Can you check the contents of svg.blade.php (inside resources/views/) and check it matches the current one:

https://github.com/aryehraber/statamic-responsive-img/blob/master/ResponsiveImg/resources/views/svg.blade.php

If not, can you copy paste it in and see if it resolves?

aerni commented 6 years ago

The content of svg.blade.php are exactly the same. However I still replaced it and saved the file. Now it works. Really strange …

aryehraber commented 6 years ago

Haha, mystery!

Anyway, glad it works -- let me know how the data-attribute testing goes!

aerni commented 6 years ago

I've tested lazy-loading using this library for regular images and Swiper Slider for a slider. Both work as expected. Good job!

aryehraber commented 6 years ago

Fantastic! Thanks again for testing and reporting back.

Will close this issue and consider the feature done, though I'd still like to build out a settings area to make it easier to define some global configs, but will open a new issue for that.

Feel free to open a new issue if you come across any further problems!