Pseudo: throttle ignores events in last timeout #1048

Open
RiZKiT opened this Issue Oct 13, 2011 · 3 comments

Projects

None yet

2 participants

@RiZKiT
RiZKiT commented Oct 13, 2011 edited

I use scroll:throttle to calculate the position of an element. The problem is that the scroll events in the last timeout are ignored. I don't know if that is the planned functionality or a bug.

Example:

  • I use scroll:throttle(1000) and a scroll occurs which takes 1.9 seconds.
  • As a result the associated function is executed after 0 seconds and after 1 second. But the scroll isn't finished yet, 0.9 seconds later it's finished, but the calculation function will not be executed, because scroll events in the last timeout are ignored.

I know the timeout (1000 miliseconds) is quite high, but it's just an example. Even if I set the timeout to 100 miliseconds, the calculated position would be wrong. And calculation on every scroll event isn't an option.

Here is my suggestion which solves the problem "above". Now the function will be executed after 1 second and after 2 seconds.

DOMEvent.definePseudo('throttle', function(split, fn, args){
    if (!fn._throttled){
        fn._throttled = (function(){
            fn._throttled = false;
            fn.apply(this, args);
        }).delay(split.value || 250)
    }
});

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/359917-pseudo-throttle-ignores-events-in-last-timeout?utm_campaign=plugin&utm_content=tracker%2F22069&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F22069&utm_medium=issues&utm_source=github).
@thatmarvin

This is a problem for me too. For example (using the same 1000ms timeout like above):

window.addEvent('scroll:throttle(1000)', function (event) {
    var y = window.getScroll().y;

    // Set some element position or lazy-load images based on scroll position y,
});

When scrolling stops at 1.9s, my element is still stuck at where it was at the 1s mark, or my images won't get lazy-loaded correctly. 0.9s worth of events are basically ignored.

An equivalent jQuery implementation solves this by executing the callback one last time after the event stops firing (check out the illustration for no_trailing = false).

@RiZKiT The existing throttle executes the callback at the very beginning of the event, but your implementation would only start executing the callback 1 second after the events start firing. I don't think that's expected behavior?

I'll submit a pull request with logic ported from the jQuery implementation.

@thatmarvin thatmarvin pushed a commit to thatmarvin/mootools-more that referenced this issue Feb 1, 2012
Marvin Tam Fixes #1048: execute event handler one last time after event stops fi…
…ring
a56cca7
@thatmarvin

D'oh this breaks the unit test, but the new behavior is correct as far as I can tell. Some help with the unit test please?

@RiZKiT
RiZKiT commented Feb 2, 2012

@thatmarvin Thanks for your help to that problem!

Yes I know my solutions starts later, but it solved the described problem for me. The jQuery alike solution sounds even better, but i'm unable to code that or help you with the unit test.

@GCheung55 GCheung55 pushed a commit to GCheung55/mootools-more that referenced this issue May 16, 2012
@arian arian Fix #416 - Fx.Scroll:toElement which didn't go to the element correctly
It got the position of the element, wich would be {x: 0, y: 0} when the scrollable element was already scrolled to the element, so it would scroll back to (0, 0) instead of staing at the current position.

We have to change this because the behavior of getPosition changed in 1.3, see MooTools Core ticket #1048
ab2afb3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment