-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[charts] Add line animation #11620
[charts] Add line animation #11620
Conversation
Deploy preview: https://deploy-preview-11620--material-ui-x.netlify.app/ Updated pages: |
a3d7876
to
bde2f6c
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
20f16bb
to
635707b
Compare
Pick<AreaElementProps, 'slots' | 'slotProps'> {} | ||
Pick<AreaElementProps, 'slots' | 'slotProps' | 'skipAnimation'> {} | ||
|
||
const useCompletedData = () => { |
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.
That's an extraction of the data computation to keep props propagation as clean as possible.
The initial reason was a debugging session, where I suspected to have a forgotten props somwhere
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.
Great work! 👍 💯
LGTM, leaving some suggestion and nitpicks. 🙈
By the way, do you really think that it is worth treating it as a breaking change? 🤔
Is there something that will no longer work or be completely different after this change?
Co-authored-by: Lukas <[email protected]> Signed-off-by: Alexandre Fauquette <[email protected]>
Co-authored-by: Lukas <[email protected]> Signed-off-by: Alexandre Fauquette <[email protected]>
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
The slot was getting the animated path. So I modified the code to avoid this modification. THe adventage is that if you don't want the animation, you could use the
|
Signed-off-by: Alexandre Fauquette <[email protected]> Co-authored-by: Lukas <[email protected]>
How animation works
Animating a line means modifying a lot of points. To avoid modifying too many data point, I took the approach of Nivo which is:
Have a value animated from 0 to 1 and then perform an interpolation of the path between it previous
d
value to its next one.In addition, each element is wrapped with a clip path that goes from left to right when the component is created
Tricks
chartId
that I store with the drawing area. This id is used to define clip path ids. Otherwise, two charts with same series ids would have sameclipPath
id which will lead to inconsistent behavior.changelog
The line chart now have animation by default.
You can disable them with
skipAnimation
prop.See next.mui.com/x/react-charts/lines/#animation for more information.