-
Notifications
You must be signed in to change notification settings - Fork 55
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: pie animations #451
feat: pie animations #451
Conversation
it messes with the initial animation
🦋 Changeset detectedLatest commit: 41c084b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
const { slice } = usePieSliceContext(); | ||
const [path, insetPaint] = useSliceAngularInsetPath({ slice, angularInset }); | ||
|
||
// If the path is empty, don't render anything | ||
if (path.toSVGString() === "M0 0L0 0M0 0L0 0") { |
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.
is this a default value of a path?
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.
It's the value of an empty path of the angular inset, yes. I think there's an initial render that happens when all the values haven't been set/received yet higher up so it renders just a point. It's not the best solution, but it fixes the issue.
return ( | ||
<Path path={path} paint={insetPaint} {...rest}> | ||
<Component path={path} paint={insetPaint} animate={animate} {...rest}> |
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.
thinking out loud. Would be nice in future cleanup if we could just always use AnimatedPath
so we don't have to always check which component to render.
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.
Agreed. There's potentially a perf hit of some sort, but it would definitely keep things a bit simpler.
Description
Adds animations for pie charts
Screen.Recording.2024-12-12.at.11.54.16.AM.mov
Fixes #279
Type of Change
How Has This Been Tested?
Added simple animations to Donut Chart example
Checklist: (Feel free to delete this section upon completion)
yarn run check:code
and all checks pass