Add support for lazy image loading

[lunks]

Description

Adds lazy support to ix_image_tag as described on #60. We need this in our project due to the amount of images being loaded on a single webpage, often from very bad network scenarios.

Checklist: New Feature

• Each commit follows the Conventional Commit format: e.g. chore(readme): fixed typo. See the end of this file for more information.
• Any breaking changes are specified on the commit on which they are introduced with BREAKING CHANGE in the body of the commit.
• If this is a big feature with breaking changes, consider opening an issue to discuss first. This is completely up to you, but please keep in mind that your PR might not be accepted.
• Run unit tests to ensure all existing tests are still passing
• Add new passing unit tests to cover the code introduced by your PR
• Update the readme
• Add some steps so we can test your cool new feature!

Steps to Test

Remap any img attributes needed for the lazy-loading solution of your choice via attribute_options on an ix_image_tag:

<%= ix_image_tag('image.jpg',
	attribute_options: {
		src: "data-src",
		srcset: "data-srcset",
		sizes: "data-sizes"
	},
	url_params: { w: 1000 },
	tag_options: { src: "lqip.jpg" }) %>

Related issue: #60

[sherwinski]

Hey @lunks, thanks for the PR and apologies for the delayed response here.

Initial thoughts: I think it would be worth adding extra documentation to guide users on how to integrate lazysizes into their app -- even if that just includes a link to an external guide. Since without it enabled, this feature would not work correctly.

[lunks]

Hey @lunks, thanks for the PR and apologies for the delayed response here.

Initial thoughts: I think it would be worth adding extra documentation to guide users on how to integrate lazysizes into their app -- even if that just includes a link to an external guide. Since without it enabled, this feature would not work correctly.

Trying to follow what your peers did at react-imgix: https://github.com/imgix/react-imgix#lazy-loading is it ok to add a section to README similarly? An external guide could make sense, but I think it's out of the scope of the library or the PR. Most people as you can see on #60 already found a lazy-loading library suit to their causes, they just need imgix-rails to support the standard.

[sherwinski]

Yes, I think an added section to the README (with example) would suffice 👍

[lunks]

Done @sherwinski, can you take a look?

[sherwinski]

Thanks for adding that @lunks.
Just a couple of small nitpicks to change. After that, I'd like to finish testing the feature out myself before merging.

[lunks]

Thanks for adding that @lunks.
Just a couple of small nitpicks to change. After that, I'd like to finish testing the feature out myself before merging.

Thanks for the suggestions, @sherwinski. It reads much better now.

[lunks]

Hey @sherwinski any updates?

[sherwinski]

Hey @lunks,

I tested the feature and it works great 👍 I did find having to add lazysizes manually to my app a little awkward so I was curious if you knew of a way to ship this gem with the library baked in? An added benefit of that would be that we can control the version of the dependent library to ensure that it works as expected.

[lunks]

Hey @lunks,

I tested the feature and it works great 👍 I did find having to add lazysizes manually to my app a little awkward so I was curious if you knew of a way to ship this gem with the library baked in? An added benefit of that would be that we can control the version of the dependent library to ensure that it works as expected.

Like I mentioned in the other reply, I believe this would be out of the scope of the gem. See react-imgix, it tries to support it but be agnostic enough that you can use other libs that do a similar job.

[sherwinski]

I see your point. In that case, the interface should match how we implemented it in react-imgix, which allows the user to set the attributes themselves. I like the proposed interface here but I think it would make more sense if we were going to ship it with a lazy loading library baked in. Right now, it feels a bit too in between being configurable and a full feature. To make the interface more generalized we should:

• Replace the lazy field with attribute_options, which allows the user to remap attributes like data-src, data-srcset, etc. See the example of this on react-imgix.
• Set the LQIP via tag_options[:src]

[lunks]

I see your point. In that case, the interface should match how we implemented it in react-imgix, which allows the user to set the attributes themselves. I like the proposed interface here but I think it would make more sense if we were going to ship it with a lazy loading library baked in. Right now, it feels a bit too in between being configurable and a full feature. To make the interface more generalized we should:

• Replace the lazy field with attribute_options, which allows the user to remap attributes like data-src, data-srcset, etc. See the example of this on react-imgix.
• Set the LQIP via tag_options[:src]

I disagree as most libraries besides lazysizes use this very same API.

To list a few:
https://github.com/verlok/vanilla-lazyload
https://github.com/ivopetkov/responsively-lazy

I'll go forward with the proposed API but I think it's a worse API. This works for most cases with lazysizes and other similar plugins. People that need this (as stated on #60) would benefit from this as-is.

[lunks]

@sherwinski done, let me know if you have any other requests.

[sherwinski]

Thanks a lot for your patience and working with me on this @lunks. This looks ready to merge now 👍

[lunks]

Hey @sherwinski could you please release a new gem version with this change?

[sherwinski]

Of course and sorry about @lunks, that part somehow slipped past me.
It is now released in v4.2.0

[lunks]

Of course and sorry about @lunks, that part somehow slipped past me.
It is now released in v4.2.0

Thanks, happy new year for you and your team!