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

[area-closed] add optional y0 for custom area fills #319

Merged
merged 1 commit into from
Jul 10, 2018

Conversation

rjatkinson2
Copy link

@rjatkinson2 rjatkinson2 commented Jul 10, 2018

🚀 Enhancements

  • Adds optional y0 for custom area fills
  • Primary Example: show negative area as integral
  • I know typical pattern is for y0 to be an accessor passed to yScale, which isn't flexible enough in this case. Interested to hear thoughts and happy to refactor to fit project patterns.
const stock = appleStock.slice(800).map((val, index) => ( index < 250
  ? val
  : { ...val, close: -val.close })
);

....

<AreaClosed
   data={stock}
   xScale={xScale}
   yScale={yScale}
   x={xStock}
   y={yStock}
   y0={Math.min(yScale(0), yScale.range()[0])}
   strokeWidth={1}
   stroke={'url(#gradient)'}
   fill={'url(#gradient)'}
/>

negativeasintegral

@techniq
Copy link
Collaborator

techniq commented Jul 10, 2018

@rjatkinson2 Sounds similar to the existing Threshold, which allows you to also style above/below differently.

@rjatkinson2
Copy link
Author

@techniq yea agreed and I originally went down the Threshold route. Threshold uses three Area charts, which seems superfluous for a single series. Additionally, I had trouble implementing Threshold without multiple data sets.

@techniq
Copy link
Collaborator

techniq commented Jul 10, 2018

@rjatkinson2 makes sense, and change is minor. I'll let @hshoff review, but LGTM.

Copy link
Member

@hshoff hshoff left a comment

Choose a reason for hiding this comment

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

Lovely, thank you for the contribution!

@hshoff hshoff added this to the v0.0.170 milestone Jul 10, 2018
@hshoff hshoff merged commit 2767d02 into airbnb:master Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants