react-native-pathjs-charts icon indicating copy to clipboard operation
react-native-pathjs-charts copied to clipboard

A couple of small issues in this package

Open balaskumar opened this issue 7 years ago • 6 comments

Before filing an issue please ensure the following boxes are checked, if applicable:

  • [x] I have searched for existing issues
  • [ ] I have provided detailed instructions that can reproduce the issue (including code and data necessary)

This is a wonderful package. Thanks for picking it up and sustaining/enhancing it.

I have a couple of minor requests in the rendering of line charts:

  1. When y-values are missing (not specified) it would be nice if the package connected the existing points on either side of the missing y-value. Currently the missing point is instead rendered with a y-value of 0.
  2. When "tickValues" are specified AND they happen to be numeric strings, the rendering of ticks and tick labels is messed up.

For example, look at the attached screen-shot:

screen shot 2017-03-19 at 5 24 54 pm

The intent of this plot was to supply the days of the month as the tick values (in this case 28 for Feb). However because of this bug, the labels are rendered one tick displaced to the right.

This can be solved in Axis.js by replacing the two occurrences of the statement:

let scaleBase = isNaN(c) ? i : c

with:

let scaleBase = (this.options.tickValues == undefined ? i : c)

I would be happy to make and debug this change, but I am not familiar with code development/modification in gitHub. If I could get some pointers on how to do it, I would be glad to help. Thanks.

balaskumar avatar Mar 20 '17 00:03 balaskumar

First, thanks for the great feedback.

On 1, that seems reasonable and this is seemingly a common problem with several things that draw line charts. Besides the current behavior (assume 0s for missing values) and your proposed behavior (drawing a line connecting the area of the gap), there's also the possibility of not drawing a line at all (so no line between 5 and 6 nor 13 and 14). It seems like this is an enhancement that should default to the current behavior and be configurable to use your propose behavior (and possibly the third possibility I suggest as well).

Yet another option on 1 is to consider using a SmoothLine chart over a StockLine chart - not sure if you considered that and already dismissed it or not?

On 2, what happens if you start your data with an x value of 0 instead of 1? This is how it is done in the StockLineChartStaticTickLabels chart in the example app. I think that will solve your issue. However, if you want to try to continue with your proposed solution (add address the first item), I can help you through the process of making a contribution. This project (like many others) follows a traditional "fork and pull" model. So, you will want to read through the Github documentation and follow these steps:

Thanks for giving it a shot! It can seem daunting at first, but we can help you through it.

marzolfb avatar Mar 21 '17 06:03 marzolfb

Thanks for the advice. On your comment on Issue #2, I DO start the x-values at 0. However the renderer seems to use the tickValues array (if they happen to be numeric) to compute the location of that x point on the x-axis. It seems to me that it should ALWAYS use the x-value - the tickValue array entry for that point is just a label. Let me try and fix that. Once I fork the project, how do I tell my react-native project to use this forked version of the package rather the one that I downloaded from gitHub?

balaskumar avatar Mar 22 '17 16:03 balaskumar

Sorry to bother you again, but if you could send me an answer to the question at the end of my last comment, I could get started on branching and modifying the software. Thanks.

balaskumar avatar Mar 25 '17 21:03 balaskumar

You are going to want to use git remote to adjust where you're pointing to. I did a simple google "change url fork after cloning" and I came up with a couple of useful hits:

On 2, I was incorrectly guessing at what your data looked like based on the image you provided. However, I tried out exactly what you described in the example app (changing tickValues: [ {value:'name1'}, {value:'name2'}... to tickValues: [ {value:1}, {value:2}... on the "StockLineChartStaticTickLabels" chart) and wasn't able to reproduce the behavior you mention. I sense there is something else going on that is the culprit. Could you compare what your chart source and data looks like compared to the StockLineChartStaticTickLabels chart in the example app and see what might be the cause the behavior you see on your second issue?

marzolfb avatar Mar 26 '17 08:03 marzolfb

When I changed the tickValues in StockLineChartStaticTickLabels to tickValues: [ {value:1}, {value:2}..., here is what I got. This is what I saw in my code. What did you see? screen shot 2017-03-26 at 2 13 06 pm

balaskumar avatar Mar 26 '17 21:03 balaskumar

I know, a rather old topic, but I have the exact same issues. Was there any resolution so far @balaskumar, @marzolfb ? Especially the "connecting missing values" drives me nuts ;-)

Softrabbit-de avatar Oct 05 '17 06:10 Softrabbit-de