-
-
Notifications
You must be signed in to change notification settings - Fork 303
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
feat: add repeat forever option #72
Conversation
Codecov Report
@@ Coverage Diff @@
## master #72 +/- ##
==========================================
- Coverage 64.01% 63.04% -0.97%
==========================================
Files 7 7
Lines 653 663 +10
==========================================
Hits 418 418
- Misses 235 245 +10
Continue to review full report at Codecov.
|
@AadumKhor, this PR looks good to me. Can you also share your opinions? |
@aagarwal1012 I was going to address this issue in my next PR. My approach however was removing the count factor when I like the approach used in this PR as well since it maintains the way our package is used currently and at the same time allows more customization. Personally I like the former approach since repeating animation is this particular use case should not be restricted by count. Also this might help in resolving #52 since we are not keeping track of count anymore which might be causing the problem. |
@AadumKhor My main consideration here is that there's no unexpected breaking changes for current users, since if we change the behaviour of |
@yiminghan, your argument seems right to me. @AadumKhor, should I merge this PR and make a new release? |
@aagarwal1012 We can add this to the new release as suggested. |
@allcontributors add @yiminghan for feature enhancement. |
I couldn't determine any contributions to add, did you specify any contributions? |
@allcontributors add @yiminghan for Ideas & Planning. |
I've put up a pull request to add @yiminghan! 🎉 |
Pull Request Process