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

The 'appear' event and 'threshold' option - now there is not logic behaviour #301

Closed
Perlover opened this issue Jan 12, 2016 · 10 comments
Closed

Comments

@Perlover
Copy link

Hello!

Now if i use the threshold option, the 'appear' event occurs for image which is not only in viewport area - even for image which is in threshold area behind screen. So now there is the constant order of events for image: 'appear' -> 'load'

But i need and and i would like to suggest (i will make patch) a same behaviour:

I think that the right behaviour should be as: the 'appear' event should occur for image when it occurs in viewport area first time only, but the 'load' event should occur when image was loaded (in viewport or not if we use the 'threshold' option). So there may be same event orders: 'appear' -> 'load' and 'load' -> 'appear' and even only 'load' without 'appear' event. I think it will be logical behaviour. And a user will be able to attach handler to 'appear' event (how he can do through plugin settings) and will catch it only when image in viewport area.

What do you think about this?

If you will say that it's nice suggestion - i will make a patch for this. I don't think that this feature will break a some old code of other application based in lazyload plugin because the order of events is not described detailed in documentation now.

@Perlover
Copy link
Author

And i found doc about 'appear'
http://www.appelsiini.net/2012/lazyload-170
But now other behaviour: Handler for appear event is called when image appears to viewport or to threshold area behind viewport but before it is loaded.
So i think right behaviour of appear event should be as i suggest because i see that you wanted it from begining.

@tuupola
Copy link
Owner

tuupola commented Jan 12, 2016

I am not sure if I understand your explanation, but the point of threshold is user to be able to tune when the events happen. In other words either when something is scrolled into viewport, or if user prefers threshold amount of pixels before something is scrolled to viewport. The threshold parameter should apply for bot appear and load events.

@Perlover
Copy link
Author

Here is the example of new code:

Perlover@355053a

@Perlover
Copy link
Author

It's only draft. But i think it will work.
I think that your code of 'appear' event should have a other event name - for example 'toload'
But new code of 'appear' will reflect the essence of the name (the 'toload' event too)

P.S. There is mistake in commit description:

the draw of other behaviour
Right:
the draft of other behaviour

@Perlover
Copy link
Author

And this commit too
Perlover@da43659

@Perlover
Copy link
Author

Other words, here all my commits for this feature:
master...Perlover:right_appear_event

@Perlover
Copy link
Author

I began a testing and found the bug
As i will fix i will let to know to you
Thanks!

@Perlover
Copy link
Author

Everything done! :)
It works at me as i expected
Here all changes in one 'diff' command:
master...Perlover:right_appear_event

@tuupola
Copy link
Owner

tuupola commented Jan 12, 2016

Sorry but as I said the behaviour is now as expected. The threshold parameter should should apply for both appear and load events. BC breaking changes cannot anyway be done without updating major version number.

@Perlover
Copy link
Author

Ok. I will send to you other patch now ;-)

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

No branches or pull requests

2 participants