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

Parity/shape pie #292

Merged
merged 3 commits into from
May 17, 2018
Merged

Parity/shape pie #292

merged 3 commits into from
May 17, 2018

Conversation

yuchi
Copy link
Contributor

@yuchi yuchi commented May 17, 2018

💥 Breaking Changes

No breaking change.

🚀 Enhancements

  • Add support for startAngle and endAngle props in the Pie component.
  • Add support for pieSortValues prop which maps to D3’s pie.sortValues() which let the developer sort by extracted values instead of data.

📝 Documentation

  • No documentation change required since the Pie component is not documented at all!

🐛 Bug Fix

  • Add actual support for startAngle and endAngle props in the Pie component.
  • Check for != null for numeric props in Pie component. (See commit message)

✅ Tests

  • Add tests for sort callbacks in the Pie component.

yuchi added 3 commits May 17, 2018 14:37
- The `startAngle` and `endAngle` were already extracted but not passed
  down to D3’s pie. This made the component unusable for sunburst
  visualizations when constructed with multiple pies stacked one on
  another.
- Use `n != null` instead of coercing them to boolean so that zero
  values are passed anyway. The fact that for `startAngle` and
  `padAngle` they currently are the default on D3 side should not be
  a reason for skipping passing them. On `endAngle` this is really
  important to collapse a pie when both start and end are 0.
@hshoff hshoff added this to the v0.0.164 milestone May 17, 2018
@hshoff
Copy link
Member

hshoff commented May 17, 2018

This is great! Thanks for the contribution @yuchi!

@hshoff hshoff merged commit 15b4e8e into airbnb:master May 17, 2018
@yuchi
Copy link
Contributor Author

yuchi commented May 17, 2018

Glad to help!

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.

2 participants