Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions src/core/core.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,42 @@ module.exports = function(Chart) {
return position === 'top' || position === 'bottom';
}

/**
* Return the y data of the label, if no this label, return null
* @private
* @param {string} label the label in the labels array
* @param {array} dataArray the data in the datasets.
* @return {number} the y data according to the label
*/
function getY(label, dataArray) {
for (var i = 0; i < dataArray.length; i++) {
var item = dataArray[i];
if (item.x === label) {
return item.y;
}
}
return null;
}
/**
* Find if the data in datasets exists in the labels
* @private
* @param {array} dataArray the data in the datasets
* @param {array} labelArray the label in the labels array
* @return {boolean} the match result, true is included
*/
function dataInLabel(dataArray, labelArray) {
for (var i = 0; i < dataArray.length; i++) {
var item = dataArray[i];
if (item === null || item === undefined || item.x === undefined) {
break;
}
if (labelArray.indexOf(item.x) > -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this will return true if a single item matches. is that what you want?

if you want to see if all items match then you could return false if < 0 here and return true at the end of the function

return true;
}
}
return false;
}

helpers.extend(Chart.prototype, /** @lends Chart */ {
/**
* @private
Expand Down Expand Up @@ -691,7 +727,11 @@ module.exports = function(Chart) {
if (!dataset._meta) {
dataset._meta = {};
}

if (!dataset.data) {
dataset.data = [];
}
// Format the data if the data array is missing some point according to the labels
dataset.data = me._formatDataset(dataset.data);
var meta = dataset._meta[me.id];
if (!meta) {
meta = dataset._meta[me.id] = {
Expand Down Expand Up @@ -933,7 +973,26 @@ module.exports = function(Chart) {
me.lastActive = me.active;

return changed;
}
},
/**
* Format the data list if the data is missing some points in the datasets.
* @private
* @param {array} dataArray array to format
* @return {array} the formated array
*/
_formatDataset: function(dataArray) {
var labels = this.chart.data.labels;
var tmp = dataArray.slice(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

there's no reason to create tmp here as a copy of dataArray. the only actions taken on it are read-only. just use dataArray directly and remove tmp

var result = dataArray;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is where you should make the defensive copy. don't modify the input to the function. instead do var result = dataArray.slice(0); // create copy of input

var match = dataInLabel(tmp, labels);
if (match && dataArray.length < labels.length) {
for (var i = 0; i < labels.length; i++) {
var label = labels[i];
result[i] = {x: label, y: getY(label, tmp)};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to check if it's a horizontal or vertical chart here and can't just assume that the label will be on the x-axis

And getY should probably be called getValue and also do the same check

}
}
return result;
},
});

/**
Expand Down