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

Transformation for custom value in SparklinesReferenceLine #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Dem0n3D
Copy link

@Dem0n3D Dem0n3D commented Dec 29, 2016

  <Sparklines data={this.props.data}>

        <SparklinesLine />

        <SparklinesReferenceLine type="custom" value={point.y} />

  </Sparklines>

in this PR, point.y will be transformed with dataToPoints (max in min will be calculated automatically in Sparklines).

@@ -56,7 +56,8 @@ class Sparklines extends React.Component {
return (
<svg {...svgOpts} >
{React.Children.map(this.props.children, child =>
React.cloneElement(child, { points, width, height, margin })
React.cloneElement(child, { points, limit, width, height, margin, max: typeof max == "undefined" ? Math.max.apply(Math, data) : max,
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we can move this to some other parts of the app?
And keep this simple?

Copy link
Author

Choose a reason for hiding this comment

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

As you wish. I just want to make SparklinesReferenceLine works in expected way.

Choose a reason for hiding this comment

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

Why does the min and max need to be calculated here? The Sparklines component doesn't do any additional calculations and the dataToPoints function backfills the missing information. The second commit seems extraneous.

Copy link
Author

@Dem0n3D Dem0n3D Jan 28, 2017

Choose a reason for hiding this comment

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

Look at this:

const data = type == 'custom' ? [value] : dataProcessing[type](ypoints);

const y = dataToPoints({ data, limit, width, height, margin, max, min })[0].y;

here dataToPoints gets already transformed data (just one point in many cases), so it has no idea about original data array, and it can't calculate correct min and max.
SparklinesReferenceLine, in other side, don't gets original data too. So, we have that min and max of original data array should be calculated in the top level component.

@MattVoda
Copy link

MattVoda commented Jun 6, 2017

Hey, any progress on this?

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.

4 participants