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

Only trigger fresh waypoints that are within the viewable area #509

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brendon
Copy link

@brendon brendon commented Jul 16, 2016

It doesn’t make sense to trigger fresh waypoints that are already
outside of the visible area of the scrollable context. The waypoints
may be initialised when the context has already been scrolled somewhat.

This change will only trigger a fresh waypoint if that waypoint is
between the top and bottom of the visible area of the scrollable
context. It triggers both forward and back because there is no concept
of just ‘visible’.

It doesn’t make sense to trigger fresh waypoints that are already
outside of the visible area of the scrollable context. The waypoints
may be initialised when the context has already been scrolled somewhat.

This change will only trigger a fresh waypoint if that waypoint is
between the top and bottom of the visible area of the scrollable
context. It triggers both forward and back because there is no concept
of just ‘visible’.
@brendon
Copy link
Author

brendon commented Jul 16, 2016

Deals with #508. No tests yet as I wasn't able to get the test suite running locally. I wanted to table this change for discussion as it changes previous behaviour that may have been unintentional but still relied upon.

@brendon
Copy link
Author

brendon commented Jun 5, 2017

@imakewebthings, were you interested in looking into this change further?

@dzajic
Copy link

dzajic commented Sep 22, 2017

I ran into this, in a painful way. I think it's totally surprising for it to behave the way it does. I tried your fix and it seems to work. The last commit was over a year ago (9/2016), and there over a thousand forks, so I'm guessing this is 💀. Thanks for the fix, @brendon!

@brendon
Copy link
Author

brendon commented Sep 23, 2017

Thanks @dzajic. It's a great project. @imakewebthings Caleb, would you be interested in bringing on some extra maintainers? I hate to see good software die! :)

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