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 onProgress to track the scroll progress between enter / leave #304

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

Conversation

TN1ck
Copy link

@TN1ck TN1ck commented Apr 26, 2019

Hey all,

this PR adds a really useful feature to react-waypoint. An onProgress handler.
This is useful when one wants to animate something regarding the scroll position, imagine for example a progress indicator how far you've read the page or similar.

I did not write the logic here myself, but used https://github.com/davidkofahl/react-waypoint-with-progress/commits/master as base and fixed it up a bit so a PR could be made.

I created a test page here: https://waypoints-on-progress.netlify.com/, the code for it can be found in this branch: https://github.com/TN1ck/react-waypoint-with-progress/tree/progress-test-page

I added some docs, typescript definitions and more.
There should be no performance hit, the only thing that was added on every scroll are some simple math calculations.

If there is something I can add to improve this PR, please tell me!

David T. Kofahl and others added 3 commits April 26, 2019 10:58
fix bug in progress calculation

make progress 0 through 1

fix progress to 2 digits

rm fixed decimal place for current scroll progress; allow more precision

update progress to only call callback when current step within viewport

bump version to 9.0.6; make progress callable only when inside

revert changes to package.json

chore: please the linter

chore: please the linter again
@TN1ck TN1ck changed the title Add progress Add onProgress to track the scroll progress between enter / leave Apr 26, 2019
@@ -211,12 +209,25 @@ export class Waypoint extends React.PureComponent {
viewportTop: bounds.viewportTop,
viewportBottom: bounds.viewportBottom,
};

if (currentPosition === INSIDE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can this be combined with the same if-statement a few lines down on 225?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it out and it needs to be done before the previousPosition === currentPosition check, as these are then "inside" and "inside" and thus the onProgress wouldn't be called.

onProgress.call(this, Object.assign({}, callbackArg, { progress }));
}

if (previousPosition === currentPosition) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to make this check happen first?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately not, as inside === inside is always true and breaks the onProgress logic to fire an event on every scroll.

@nickhsine
Copy link

This PR is really helpful.
What could we do to have this PR done?

@MatthewHerbst
Copy link
Contributor

cc @trotzig

@trotzig
Copy link
Collaborator

trotzig commented Nov 12, 2020

While I think the work @TN1ck did here is good, I feel like this library isn't the right tool for the job. I would recommend looking into Intersection Observer for fine-grained scrolling control like the one described here.

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

4 participants