Skip to content
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(sunburst): use arcs package #1350

Merged
merged 12 commits into from
Apr 23, 2021
Merged

feat(sunburst): use arcs package #1350

merged 12 commits into from
Apr 23, 2021

Conversation

plouc
Copy link
Owner

@plouc plouc commented Dec 16, 2020

This PR makes use of the new @nivo/arcs package:

  • this way we can remove some components such as labels
  • we have more efficient transitions which are now handled globally instead of using useSpring for every element
  • we also have enter/exit transitions (can be seen in action in the drill down story :))
  • we now have a transitionMode which can be used to control the way nodes enter/exit
  • some types can be used directly from this package such as the arc labels properties, which are now identical with @nivo/pie
  • the arc label component can optionally use a custom component and the position relative to the radius can be configured via arcLabelsRadiusOffset

I've also revised the types to use generic types from both @nivo/core and @nivo/arcs, also changed the ComputedDatum to be more aligned with other packages.
I've added a path property to the nodes, this way we can easily get a list of all the ancestors of a node, I thought it could be useful.

There was a bug with the data generator for this chart, if you look at the current website, you'll see that the generated data get nested under the root name property each time you click roll the dice.

I realized that the pie package no more require lodash (at least not as a direct dependency), and same for react-spring, so I removed them.

There's also another change completely unrelated on the references page.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 16, 2020

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 dd603c8:

Sandbox Source
nivo Configuration
nivo-website Configuration

Base automatically changed from arcs to master December 18, 2020 00:47
packages/sunburst/src/hooks.ts Outdated Show resolved Hide resolved
packages/sunburst/src/hooks.ts Outdated Show resolved Hide resolved
packages/sunburst/src/types.ts Outdated Show resolved Hide resolved
packages/sunburst/src/types.ts Outdated Show resolved Hide resolved
})
})

describe('patterns & gradients', () => {
it('should support patterns', () => {
xit('should support patterns', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess these tests no longer passed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the changes to bindDefs cause this to fail. What is the reason behind those changes?

@TheMatrixMaster
Copy link

TheMatrixMaster commented Feb 20, 2021

When will this PR be merged? Can't wait to add arc labels to sunburst!

@wyze wyze force-pushed the improve-sunburst branch from 8839716 to 96835a0 Compare April 23, 2021 18:31
@wyze wyze force-pushed the improve-sunburst branch from cba842a to dd603c8 Compare April 23, 2021 19:23
@wyze wyze merged commit b058f7b into master Apr 23, 2021
@wyze wyze deleted the improve-sunburst branch April 23, 2021 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants