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

Fix for #25 (and afterAnimate) #26

Merged
merged 4 commits into from
Apr 6, 2018
Merged

Fix for #25 (and afterAnimate) #26

merged 4 commits into from
Apr 6, 2018

Conversation

xehpuk
Copy link
Contributor

@xehpuk xehpuk commented Apr 6, 2018

animateScroll previously returned immediately which resulted in updateHistory and afterAnimate to run before the animation. It nor returns a Promise which is resolved when the animation ends and is rejected if there isn't an element with the given ID.
This previously was a warning emitted by fbjs (and its last usage). So now it's kicked out of the readme.

The package-lock.json should be committed into source repositories.

@bySabi
Copy link
Collaborator

bySabi commented Apr 6, 2018

@xehpuk your commit are absolutely great.
Make animateScroll async was a good catch, I forget the non-blocking nature of setTimeout

Only one thing. Could you reimplement animateScroll with async/await? . I thing this will make the code more readable. If you have the time, of course :-)

@bySabi bySabi merged commit 95b03af into some-react-components:master Apr 6, 2018
@xehpuk
Copy link
Contributor Author

xehpuk commented Apr 6, 2018

Phew, not sure how to do it or if it's even possible without using a Promise for the "callback nature" of setTimeout. Maybe you have an idea?

@bySabi
Copy link
Collaborator

bySabi commented Apr 6, 2018

At the end, the solution, promisify setTimeout, looks more tricky.

Thanks for take your time on this.

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.

2 participants