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 animation for ErrorBar #4311
Add animation for ErrorBar #4311
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking for a few more tests here if possible.
I'd like to play with this a little before we release it to get the best possible default. From the video the appearance seems maybe not fast enough or maybe that the animation should follow the direction of the error bar? I'm not sure
Either way thanks for doing this
Thanks! I've updated the PR with more tests and also updated the default animation duration |
I wish the animation expanded from the dot up and down <3 that would be amazing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me except the setTimeout in test - please switch it to fake timers
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.x #4311 +/- ##
==========================================
+ Coverage 94.05% 94.94% +0.88%
==========================================
Files 92 92
Lines 20113 20163 +50
Branches 2769 2823 +54
==========================================
+ Hits 18918 19144 +226
+ Misses 1187 1013 -174
+ Partials 8 6 -2 ☔ View full report in Codecov by Sentry. |
Did try that initially but couldn't find a way to implement that without using css @Keyframe (I guess we shouldn't add extra css files in this project?) |
@ForestLinSen looks like tests are failing |
Hi @ckifer , I've updated the unit tests, could you approve the workflow again? Thx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - going to merge it. If we want to adjust how the animation happens we probably can before 3.x is released
@ForestLinSen these tests failed after merge, could you take a look? |
## Description The current `ErrorBar` animation transitions from left to right. According to the discussions in the previous [PR](#4311), it has now been changed to expand centrally towards both the top and bottom ## Related Issue [ErrorBar does not animate](#4055) ## Motivation and Context ## How Has This Been Tested? It was tested locally with Storybook and validated through unit tests ## Screenshots (if appropriate): https://github.com/recharts/recharts/assets/41566276/2fe5908b-4d83-4664-821d-761cd20e847e ## Types of changes <!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: <!--- Go over all the following points, and put an `x` in all the boxes that apply. --> <!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [x] I have added tests to cover my changes. - [ ] I have added a storybook story or extended an existing story to show my changes
Description
Added animation for ErrorBar component
Related Issue
ErrorBar does not animate
Motivation and Context
Previously the ErrorBar didn't have animation, now animation has been introduced to make it have a smoother visual effect
How Has This Been Tested?
It was tested locally with Storybook and validated through unit tests
Screenshots (if appropriate):
rechart-animation2.mp4
Types of changes
Checklist: