-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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(arcs): introduce @nivo/arcs package #1331
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 03f8702:
|
sounds really cool 👍 |
@wyze if you could have a look at this PR, that would be great, I'm sorry, it's huge 🙇🏻 |
Of course @plouc, but only if you look at my axes PR next. :) |
Right, sorry also for that, in fact I already started, I'm gonna try to finish reviewing it in the next upcoming days. |
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 great! Just minor grammar changes and question about directory names.
packages/arcs/src/arc_link_labels/useArcLinkLabelsTransition.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Neil Kistner <[email protected]>
@wyze, thank you for the review, and the fixes! regarding directory name, I tend to use underscores in most of my projects, so I would prefer to use them. |
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.
Works for me then. Good to go!
The purpose of this PR was initially to add transitions to
@nivo/pie
, but when starting to implement it, I realized that this should also be useful for a few existing packages:@nivo/pie
,@nivo/chord
and@nivo/sunburst
, which led me to think of introducing a@nivo/arc
package.This also allows us to move some logic specific to arcs in this new package like cursor detection for canvas charts, hooks for configuring and memoizing a d3 arc generator... while making the core package a bit lighter.
I haven't integrated it yet in the sunburst package as it might require a bit of work, and just replaced the cursor detection in the chord package, but ultimately, we should use more from this new package.
It could also help building new chart types such as a Rose chart.