React-native-chart-kit: decimalPlaces = 0 doesn't work correctly, cannot get rid of decimals

[erkkikeranen]

First, thank you for a great package.

There is a small bug that at least bothers our project when we get float data for the Y-values and we don't want to render decimals.

https://github.com/indiespirit/react-native-chart-kit/blob/master/src/abstract-chart.js#L85

const decimalPlaces = this.props.chartConfig.decimalPlaces || 2

if I provide decimalPlaces = 0 then the code evaluates: 0 || 2 => 2

I can workaround this with decimalPlaces = 0.1 to get rid of decimals but I don't think this is proper way to do it. (thus there would be a call for example 1.2345.toFixed(0.1))

[Hermanya]

that's an easy fix, I changed it to this.props.chartConfig.decimalPlaces === undefined ? 2 : this.props.chartConfig.decimalPlaces

[Hermanya]

I will try to release the fix tonight.

[Hermanya]

available in [email protected]

[vhesener]

@Hermanya Did this possibly regress? I'm using the LineChart sample code in the README and when I change decimalPlaces to anything, and it always shows up with 2 decimal places. I've tried setting it to 0, 1, 3, and 521, and it seems to ignore any value I throw at it. I can fix it and submit a PR, but just wondering if anyone can possibly do a quick sanity check to verify that changing decimalPlaces prop anywhere doesn't do anything? Let me know if you see anything obvious.

[Hermanya]

It's very possible this is a regression, because yesterday I merged code touching that part. But it looked good to me. I'd very much appreciate a PR to fix this 😁

[vhesener]

Can you point me to the merge so I can have a hint on where to look?

[Hermanya]

https://github.com/indiespirit/react-native-chart-kit/pull/253

[vhesener]

Awesome. I'll look into it today

[vhesener]

Ok I think I see the problem, but I have a question that hopefully you can answer.

Right here, we're missing decimalPlaces so it's essentially dropped. I found more than one place where I can inject it back in, but I think I need to know the intention of the chartConfig prop vs the base level LineChartProps.

In the README and also in the typescript types, decimalPlaces belongs to the ChartConfig type, so I can do something like this when calling renderHorizontalLabels, (or simply inject this.props.chartconfig.decimalPlaces).

Anyway, if you're not in this context and what I'm saying doesn't make any sense, I have a much simpler question: What's the logical separation for ChartConfig vs LineChartProps?

At first glance, it seems like ChartConfig should be used for svg's Text styling attributes, but I don't know if that was the intent. If I knew the intended purpose of ChartConfig, I think I can put the decimalPlaces in the correct place. Right now, it's the same amount of work to move it up one level as opposed to keeping it in ChartConfig, so if it doesn't matter either way, I would like to put it inside the type that the package logically intended to. Hopefully that makes sense the way I explained it. Any insight would be much appreciated. Thanks!

[mlorthios]

Same problem I had to modify the source with
(this.props.chartConfig && this.props.chartConfig.decimalPlaces) != undefined ? (this.props.chartConfig && this.props.chartConfig.decimalPlaces) : 2