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

StackedAreaChart doesn't properly render all data points #35

Closed
mrbodoia opened this issue Jan 29, 2018 · 7 comments
Closed

StackedAreaChart doesn't properly render all data points #35

mrbodoia opened this issue Jan 29, 2018 · 7 comments

Comments

@mrbodoia
Copy link

What is the problem?

StackedAreaChart doesn't properly render all the data points. In particular, it only renders the first keys.length data points.

When does it happen?

The following code should render a StackedAreaChart that looks like the one from the examples, but with one additional data point (making five total).

class StackedAreaExample extends React.PureComponent {

    render() {

        const data = [
            {
                month: new Date(2015, 0, 1),
                apples: 3840,
                bananas: 1920,
                cherries: 960,
                dates: 400,
            },
            {
                month: new Date(2015, 1, 1),
                apples: 1600,
                bananas: 1440,
                cherries: 960,
                dates: 400,
            },
            {
                month: new Date(2015, 2, 1),
                apples: 640,
                bananas: 960,
                cherries: 3640,
                dates: 400,
            },
            {
                month: new Date(2015, 3, 1),
                apples: 3320,
                bananas: 480,
                cherries: 640,
                dates: 400,
            },
            {
                month: new Date(2015, 4, 1),
                apples: 2320,
                bananas: 780,
                cherries: 440,
                dates: 400,
            },
        ]

        const colors = [ '#ED5314', '#FFB92A', '#FEEB51', '#9BCA3E' ]
        const keys   = [ 'apples', 'bananas', 'cherries', 'dates' ]

        return (
            <StackedAreaChart
                style={ { height: 200, paddingVertical: 16 } }
                data={ data }
                keys={ keys }
                colors={ colors }
                curve={ shape.curveNatural }
                showGrid={ false }
            />
        )
    }
}

However, this code only renders the first four data points.

What platform?

Occurs on both iOS and Android platforms.

Additional comments

I think the problem has something to do with this line, which seems to be using keys.length when it should be using data.length. However, changing that line alone does not fix the issue, so there must be another place where the number of data points is being calculated incorrectly.

@JesperLekland
Copy link
Owner

Hi @mrbodoia! I'll take a look and see what I can do. Could you perhaps illustrate your problem with a screenshot?

@mrbodoia
Copy link
Author

Sure! Here is a screenshot of what I get when I run the code directly from the example, which contains four data points:
example code 4 data points
This corresponds to the following code:

class StackedAreaExample extends React.PureComponent {

    render() {

        const data = [
            {
                month: new Date(2015, 0, 1),
                apples: 3840,
                bananas: 1920,
                cherries: 960,
                dates: 400,
            },
            {
                month: new Date(2015, 1, 1),
                apples: 1600,
                bananas: 1440,
                cherries: 960,
                dates: 400,
            },
            {
                month: new Date(2015, 2, 1),
                apples: 640,
                bananas: 960,
                cherries: 3640,
                dates: 400,
            },
            {
                month: new Date(2015, 3, 1),
                apples: 3320,
                bananas: 480,
                cherries: 640,
                dates: 400,
            },
        ]

        const colors = [ '#8800cc', '#aa00ff', '#cc66ff', '#eeccff' ]
        const keys   = [ 'apples', 'bananas', 'cherries', 'dates' ]

        return (
            <StackedAreaChart
                style={ { height: 200, paddingVertical: 16 } }
                data={ data }
                keys={ keys }
                colors={ colors }
                curve={ shape.curveNatural }
                showGrid={ false }
            />
        )
    }
}

And here is what I get when I run a modified version of the code that has six data points rather than four:
modified code 6 data points
The corresponding code is as follows:

class StackedAreaExample extends React.PureComponent {

    render() {

        const data = [
            {
                month: new Date(2015, 0, 1),
                apples: 3840,
                bananas: 1920,
                cherries: 960,
                dates: 400,
            },
            {
                month: new Date(2015, 1, 1),
                apples: 1600,
                bananas: 1440,
                cherries: 960,
                dates: 400,
            },
            {
                month: new Date(2015, 2, 1),
                apples: 640,
                bananas: 960,
                cherries: 3640,
                dates: 400,
            },
            {
                month: new Date(2015, 3, 1),
                apples: 3320,
                bananas: 480,
                cherries: 640,
                dates: 400,
            },
            {
                month: new Date(2015, 3, 1),
                apples: 2320,
                bananas: 980,
                cherries: 740,
                dates: 100,
            },
            {
                month: new Date(2015, 3, 1),
                apples: 1560,
                bananas: 230,
                cherries: 1200,
                dates: 450,
            },
        ]

        const colors = [ '#8800cc', '#aa00ff', '#cc66ff', '#eeccff' ]
        const keys   = [ 'apples', 'bananas', 'cherries', 'dates' ]

        return (
            <StackedAreaChart
                style={ { height: 200, paddingVertical: 16 } }
                data={ data }
                keys={ keys }
                colors={ colors }
                curve={ shape.curveNatural }
                showGrid={ false }
            />
        )
    }
}

As you can see, the graph looks exactly the same in both cases. Unless I'm completely misunderstanding the syntax of the library, those two extra data points should have been rendered as part of the StackedAreaChart.

You can reproduce the error by just copying the code from the StackedAreaChart example and then adding more data points. Also, if I remove data points (to try and graph three rather than four), I get TypeError: undefined is not an object (evaluating 'd[0]'). You can verify this by removing the last data point from the example code.

As I mentioned, I believe this is because stacked-area-chart.js is using the keys list in some places where it should be using data instead, but I wasn't able to fully diagnose the problem.

@mrbodoia
Copy link
Author

Okay I made some modifications to stacked-area-chart.js and I think I may have fixed the problem. The chart now renders properly for both three and six data points.

Here is the diff of the changes that I made:

Maxs-MacBook-Pro:src mrbodoia$ diff new-stacked-area-chart.js stacked-area-chart.js
87c87
<             .domain([ 0, data.length - 1 ])
---
>             .domain([ 0, keys.length - 1 ])
96c96
<                 (data.map((_, index) => serie[ index ]))
---
>                 (keys.map((_, index) => serie[ index ]))
143c143
<                             return data.map((key, index) => {
---
>                             return keys.map((key, index) => {

You're obviously more familiar with the codebase so perhaps you can verify that these changes solve the problem and don't break anything else.

@JesperLekland
Copy link
Owner

Great job! I'll go ahead and verify the changes during the day. If you would like to you are more than welcome to create a PR and become an official contributor :)

@JesperLekland
Copy link
Owner

Verified 🥇 Opened PR #38. Will merge if you don't oppose

@mrbodoia
Copy link
Author

Looks good to me. Thanks!

@JesperLekland
Copy link
Owner

Released in v.2.2.2. Thank you for reporting the issue and helping with the fix 🥇

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

No branches or pull requests

2 participants