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): add mouse events and some labels #880

Merged
merged 8 commits into from
Nov 7, 2020
Merged

feat(sunburst): add mouse events and some labels #880

merged 8 commits into from
Nov 7, 2020

Conversation

juliankrieger
Copy link
Contributor

@juliankrieger juliankrieger commented Feb 24, 2020

Because of @mjsarfatti 's absence, I have continued to implement what you recommended @plouc.

Please review the changes and tell me what you think


  • Add slice labels at the first level
  • Add onClick/onMouseEnter/onMouseLeave events

@mkazlauskas
Copy link

What's holding this from being merged?

@plouc
Copy link
Owner

plouc commented Apr 15, 2020

Thank you @juliankrieger! The build is failing, do you think you could you have a look?

@juliankrieger
Copy link
Contributor Author

Yes @plouc , the build fails because of 45:29 error 'centerRadius' is constant no-const-assign. Should be an easy fix.

Julian Krieger and others added 2 commits May 22, 2020 09:58
@juliankrieger
Copy link
Contributor Author

@plouc Build is successful

Copy link
Contributor

@wyze wyze left a comment

Choose a reason for hiding this comment

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

Need to pull it down and check out storybook/website updates, but code looks good. Just some minor changes.

@@ -8,6 +8,7 @@
*/
import React from 'react'
import PropTypes from 'prop-types'
import { noop } from '@nivo/core'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be with the other imports from @nivo/core.

export default class SunburstLabels extends Component {
static propTypes = {
nodes: PropTypes.arrayOf(PropTypes.shape({})).isRequired,
label: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be updated to just a func and be required, based on usage below.

@juliankrieger
Copy link
Contributor Author

I am knee deep in my thesis, maybe after I am done with it.

@epentin
Copy link

epentin commented Oct 16, 2020

Does anyone have an update on the status of this pr?

@epentin epentin mentioned this pull request Oct 19, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 7, 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 907f4d5:

Sandbox Source
nivo Configuration
nivo-website Configuration

@wyze wyze changed the title Feat enhanced sunburst feat(sunburst): add mouse events and some labels Nov 7, 2020
@wyze wyze merged commit 1b3dd8f into plouc:master Nov 7, 2020
@wyze
Copy link
Contributor

wyze commented Nov 7, 2020

Thanks everyone for all the work on this and sorry about the long delay on merging!

ddavydov pushed a commit to netronixgroup/nivo that referenced this pull request Apr 8, 2021
- Add slice labels at the first level
- Add onClick/onMouseEnter/onMouseLeave events

Co-authored-by: Manuele J Sarfatti <mjsarfatti@gmail.com>
Co-authored-by: Julian Krieger <julian.krieger@ergosign.de>
Co-authored-by: Asma Berkani <asma.berkani@outlook.com>
Co-authored-by: Neil Kistner <neil.kistner@gmail.com>
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.

7 participants