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.

Sign up for GitHub

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

Jump to bottom

feat(sunburst): migrate package to typescript #1264

Merged
merged 18 commits into from
Dec 2, 2020
Merged

feat(sunburst): migrate package to typescript #1264

merged 18 commits into from
Dec 2, 2020

Conversation

wyze
Copy link
Contributor

@wyze wyze commented Nov 10, 2020

Close #1240

This was referenced Nov 10, 2020
Closed
Closed
@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 10, 2020 •
edited
Loading

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

Sandbox Source
nivo Configuration
nivo-website Configuration

@wyze wyze force-pushed the sunburst-typescript branch 3 times, most recently from e275b0c to a0d011b Compare November 20, 2020 05:48
@wyze wyze force-pushed the sunburst-typescript branch from a0d011b to 29cb046 Compare November 23, 2020 16:04
@wyze wyze marked this pull request as ready for review November 24, 2020 17:30
@wyze wyze requested a review from plouc November 24, 2020 17:30
plouc
plouc requested changes Nov 29, 2020
View reviewed changes
packages/generators/src/index.js Show resolved Hide resolved
packages/sunburst/src/Sunburst.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/Sunburst.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/Sunburst.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/Sunburst.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/SunburstLabels.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/SunburstLabels.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/SunburstTooltip.tsx Outdated Show resolved Hide resolved
packages/sunburst/src/hooks.ts Outdated Show resolved Hide resolved
packages/sunburst/src/hooks.ts Outdated Show resolved Hide resolved
wyze added 17 commits November 30, 2020 17:49
@wyze
feat(sunburst): init package migration to typescript
e2bab89
@wyze
fix(pie): fix types related to d3-shape
fa3163d
@wyze
feat(sunburst): remove recompose
9608728
@wyze
fix(sunburst): fix with child color modifier story
06a67b8
@wyze
fix(sunburst): get build passing again
b57611c
@wyze
chore(sunburst): rename slice label props for consistency
4df91ed
@wyze
refactor(sunburst): cleanup comments and make a custom hook
a162a7e
@wyze
refactor(sunburst): migrate stories to typescript
10b479a
@wyze
feat(sunburst): add pattern and gradient support
b3130b2
@wyze
fix(generators): change duplicate keys in tree data
b4a2e2a
@wyze
feat(sunburst): add animation support
ebb3525
@wyze
fix(sunburst): fix logic with radius and centerX/Y
52094a5
@wyze
feat(sunburst): add parent to node data
8785564
@wyze
feat(sunburst): add valueFormat prop and move event handlers to hook
8f5e3ce
@wyze
feat(sunburst): add tests
de3732e
@wyze
refactor(sunburst): respond to PR feedback
64420d5
@wyze
feat(sunburst): add layers support
65a097d
@wyze wyze force-pushed the sunburst-typescript branch from c6cd7ce to 65a097d Compare December 1, 2020 03:14
@wyze wyze requested a review from plouc December 1, 2020 03:34
@wyze
Copy link
Contributor Author

wyze commented Dec 1, 2020

Thanks for the review @plouc. I responded to all the changes!

plouc
plouc approved these changes Dec 2, 2020
View reviewed changes
Copy link
Owner

@plouc plouc left a comment

Choose a reason for hiding this comment

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

LGTM

@plouc
Copy link
Owner

plouc commented Dec 2, 2020 •
edited
Loading

@wyze, thank you for the hard work, it's way more powerful! I just noticed that the website throws an error as soon as we hover an element, I think there's an issue with the tooltip.

@wyze
Copy link
Contributor Author

wyze commented Dec 2, 2020

Okay, good catch @plouc. I was doing defaultProps different in the component and wasn't working as expected. So I updated it to make it match the style used in Pie and is working fine now.

Also noticed, default value for pie accessor was wrong on the website so updated that as well. :)

@plouc
Copy link
Owner

plouc commented Dec 2, 2020

Great, thank you! I'm gonna merge as soon as tests pass then, I think this PR alone deserves a new release :)

@plouc plouc merged commit 952ad50 into master Dec 2, 2020
@plouc plouc deleted the sunburst-typescript branch December 2, 2020 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plouc plouc plouc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate @nivo/sunburst to TypeScript
2 participants
@wyze @plouc