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 issues with timeout #43

Merged
merged 4 commits into from
Dec 14, 2019
Merged

Fix issues with timeout #43

merged 4 commits into from
Dec 14, 2019

Conversation

kenime
Copy link
Contributor

@kenime kenime commented Dec 2, 2019

No description provided.

@kenime
Copy link
Contributor Author

kenime commented Dec 5, 2019

@braposo Greeting! let me know if you need any further description on this PR.

@braposo
Copy link
Owner

braposo commented Dec 5, 2019

Hi @kenime! Thanks for creating the PR!

Could you please describe in more detail what kind of issues are you referring to? I think your changes look sensible, but just want to understand exactly what are you fixing with this.

Thanks!

@kenime
Copy link
Contributor Author

kenime commented Dec 6, 2019

@braposo I was trying to pause the text loop by updating the interval to 0, but then I found that the timeout cannot be cleared such that the text loop will still go to the next child and then stopped.

By including this PR, the timeout will be cleared as expected, such that the text loop will immediately stop when interval is changed to 0.

The reason is that with animationframe, the loop function will update the requestId value after it is returned to caller, hence we need to create an object to hold the requestId and return the reference to object instead of directly return as value.

@kenime
Copy link
Contributor Author

kenime commented Dec 13, 2019

bump

@braposo
Copy link
Owner

braposo commented Dec 14, 2019

sorry for the delay @kenime! All sounds quite sensible, thanks for taking the time to explain and fix the issue! Going to merge and release it as a new version!

@braposo braposo merged commit 44adf05 into braposo:master Dec 14, 2019
@braposo
Copy link
Owner

braposo commented Dec 14, 2019

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