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

Unload on scrollout #39

Merged
merged 8 commits into from
May 10, 2014
Merged

Conversation

raphaeleidus
Copy link
Contributor

This pull requests add 2 configurations option unload and placeholder
unload is a boolean option
placeholder is a string
When unload is true, now when the image scrolls out off the screen it will change the image back to the placeholder image. This is valuable for devices with limited memory (mobile devices). Images that are not on display don't need to be kept in memory.

In order to make this work I had to fix a bug where every image above the screen was automatically loaded. If you scroll to the bottom of a echo.js enabled page and refresh, when the page loads you will be at the bottom of the page but all the images above the screen are loaded immediately instead of lazily.

I added two other options for flexibility, but they aren't really needed, offsetBot and offsetTop. They allow you to individually set the padding above and below the window. They fallback to use the offset option if one of them is not provided.

@toddmotto
Copy link
Owner

Thanks for this, some great stuff.

I have a question or two. Would it be better do you think to cache the initial image source (which might be blank.gif or whatever...), and store that reference on a holder attribute, and unload to that reference rather than another placeholder, as this introduces a third item (unless I've misunderstood your PR).

That way there'd be no need for a placeholder, and we just use their original image src (which would work great if the src was the same, which the assumed use case generally is). It'd most likely be quicker on a performance perspective using the placeholder method, however (no batching of read/write). But it allows users to then load a third image - which I'm not overly keen on as we want to use that initial 1kb image already in memory for example.

Thoughts appreciated!

@raphaeleidus
Copy link
Contributor Author

Hey todd, my first attempt read a data-echo-placeholder attribute which wasn't terrible but made the markup ugly. I will make it write that to that so the original markup doesn't bloat.

@toddmotto
Copy link
Owner

Yep I saw that. Open to both options, just wondering which you think is best, and of course with heavy images we want to think about mobiles - so the better on mobile option might sway it. :)

@raphaeleidus
Copy link
Contributor Author

I think it makes more sense to store it on the element, so that it retains the flexibility of being able to specify different placeholder for different images, which is doable now. This is a pretty happy medium, so I am comfortable with it.

@raphaeleidus
Copy link
Contributor Author

okay my last commit made that change

@raphaeleidus
Copy link
Contributor Author

@toddmotto when merging in the callback work I made it so the callback function would be passed two parameters, the element and the operation being done, "load" or "unload" I figured this would be preferable to having to pass in 2 callbacks one for load and one for unload, but let me know what you think.

@raphaeleidus
Copy link
Contributor Author

@toddmotto can this be merged?

toddmotto added a commit that referenced this pull request May 10, 2014
@toddmotto toddmotto merged commit 16f5237 into toddmotto:master May 10, 2014
@toddmotto
Copy link
Owner

Thanks @raphaeleidus ! 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants