Issue 82

[jgwillard]

Fix https://github.com/kurkle/chartjs-chart-sankey/issues/81 and https://github.com/kurkle/chartjs-chart-sankey/issues/82.

Currently, in the case of user-provided columns, the maxX value returned by calculateX can be computed incorrectly, because it always returns the final x value created in the while loop that processes each column. In the case of user-provided columns, the nodes already have x values that may be higher than this value. Take for example the following data from https://github.com/kurkle/chartjs-chart-sankey/issues/82:

    data: {
      datasets: [
        {
          data: [
            {from: 'Oil', to: 'Energy', flow: 15},
            {from: 'Natural Gas', to: 'Energy', flow: 20},
            {from: 'Coal', to: 'Energy', flow: 25},
            {from: 'Electricity', to: 'Energy', flow: 25},
          ],
          column: {
            Oil: 0,
            'Natural Gas': 1,
            Coal: 2,
            Electricity: 3,
            Energy: 4,
          },
        },
      ],
    },

In this case, the highest value of x in the node collection will be 4, but because all 4 edges lead to a single node, calculateX will return 1. This change fixes this issue by iterating over all nodes and taking the maximum value of x instead, so even in the case of user-provided columns the correct value for maxX will be returned.

This pull request also fixes a runtime error that can occur in the case that there is no node whose x value matches a given column.

Thank you for your consideration.

[kurkle]

Thank you, good findings 👍

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[jgwillard]

Sorry, there was a small bug that I fixed because we were calling reduce on a Map instead of an array. All checks are passing now, if you can approve again I'm happy to merge.

[kurkle]

Merged!

I'm not able to do a version bump for couple of days though. So if you're eager to get this released, a pr for version bumb would help 😉