Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to skip image src replacement if images are currently hidden #122

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

Conversation

drummerboof
Copy link

Currently, if images are hidden when Imager runs, the images are still replaced with their target src. For our site (BBC international homepage) a lot of images are hidden when viewing on a smaller device and then displayed on larger form factors.

To save initial requests, I have added a "loadHidden" option which is true by default. If this is set to false (which could become the default if you are happy with it) then images which are considered "hidden" are not replaced. The method used to determine if the element is hidden or not is taken from jQuery.

Tests have been added and are passing locally.

@mindplay-dk
Copy link

I merged this to my own fork and it's working great, thanks! 👍

@thom4parisot
Copy link
Contributor

Hey, thank you both :-)

I think this is a great addition. Any chance to provide that as a generic feature (eg: filters, which could be a function or an array of them?

new Imager({ filters: Imager.filters.visibleOnly })

Imager.filters = {
  visibleOnly: function (image, imgr) {
        return Imager.isElementVisible(image);
    }
};

Imager already has a lot of options and it would be great to scale them up without adding to much if statements here and there (which adds up complexity to read/debug/maintain on the long run).

What do you both think?

@drummerboof
Copy link
Author

I think that's a cracking idea.

I'm wondering if there is any value in passing any additional parameters to the filter functions, or perhaps an object wrapping up the image div with additional attributes - but I think just the div element should be fine. Especially for this particular filter.

What do you guys think?

@mindplay-dk
Copy link

Looks good :-)

@thom4parisot
Copy link
Contributor

@drummerboof yep we could pass an object with other infos but unless we know which ones, lets not do that — and delay this when one or many use cases show up :-)

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

Successfully merging this pull request may close these issues.

None yet

3 participants