Skip to content

Commit a5d5b0f

Browse files
jtblinjtblin
committed
Refactor to reduce cyclomatic complexity
1 parent 6009cc7 commit a5d5b0f

8 files changed

Lines changed: 195 additions & 162 deletions

File tree

‎README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ there are numerous breaking changes in this version notably:
2626
* events emitted on creation and update are now prefixed with `chart-` e.g. `chart-create`
2727
* `$scope.$apply` is not called anymore on mouse hover functions calls
2828
* obviously all Chart.js breaking changes as well in how options are set, etc.
29+
* disabling the `responsive` option doesn't work via global `Chart.defaults.global.responsive` anymore,
30+
but must be set via standard options e.g. `ChartJsProvider.setOptions({ responsive: false });`
2931

3032
### npm
3133

‎angular-chart.js‎

Lines changed: 94 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@
5555
* Allows configuring chart js using the provider
5656
*
5757
* angular.module('myModule', ['chart.js']).config(function(ChartJsProvider) {
58-
* ChartJsProvider.setOptions({ responsive: true });
59-
* ChartJsProvider.setOptions('Line', { responsive: false });
58+
* ChartJsProvider.setOptions({ responsive: false });
59+
* ChartJsProvider.setOptions('Line', { responsive: true });
6060
* })))
6161
*/
6262
function ChartJsProvider () {
63-
var options = {};
63+
var options = { responsive: true };
6464
var ChartJs = {
6565
Chart: Chart,
6666
getOptions: function (type) {
@@ -105,92 +105,82 @@
105105
chartDatasetOverride: '=?'
106106
},
107107
link: function (scope, elem/*, attrs */) {
108-
var chart;
109-
110108
if (useExcanvas) window.G_vmlCanvasManager.initElement(elem[0]);
111109

112110
// Order of setting "watch" matter
111+
scope.$watch('chartData', watchData, true);
112+
scope.$watch('chartSeries', watchOther, true);
113+
scope.$watch('chartLabels', watchOther, true);
114+
scope.$watch('chartOptions', watchOther, true);
115+
scope.$watch('chartColors', watchOther, true);
116+
scope.$watch('chartDatasetOverride', watchOther, true);
117+
scope.$watch('chartType', watchType, false);
118+
119+
scope.$on('$destroy', function () {
120+
destroyChart(scope);
121+
});
122+
123+
scope.$on('$resize', function () {
124+
if (scope.chart) scope.chart.resize();
125+
});
113126

114-
scope.$watch('chartData', function (newVal, oldVal) {
127+
function watchData (newVal, oldVal) {
115128
if (! newVal || ! newVal.length || (Array.isArray(newVal[0]) && ! newVal[0].length)) {
116-
destroyChart(chart, scope);
129+
destroyChart(scope);
117130
return;
118131
}
119132
var chartType = type || scope.chartType;
120133
if (! chartType) return;
121134

122-
if (chart && canUpdateChart(newVal, oldVal))
123-
return updateChart(chart, newVal, scope);
124-
125-
createChart(chartType);
126-
}, true);
135+
if (scope.chart && canUpdateChart(newVal, oldVal))
136+
return updateChart(newVal, scope);
127137

128-
scope.$watch('chartSeries', resetChart, true);
129-
scope.$watch('chartLabels', resetChart, true);
130-
scope.$watch('chartOptions', resetChart, true);
131-
scope.$watch('chartColors', resetChart, true);
132-
scope.$watch('chartDatasetOverride', resetChart, true);
133-
134-
scope.$watch('chartType', function (newVal, oldVal) {
135-
if (isEmpty(newVal)) return;
136-
if (angular.equals(newVal, oldVal)) return;
137-
createChart(newVal);
138-
});
139-
140-
scope.$on('$destroy', function () {
141-
destroyChart(chart, scope);
142-
});
143-
144-
scope.$on('$resize', function () {
145-
if (chart) chart.resize();
146-
});
138+
createChart(chartType, scope, elem);
139+
}
147140

148-
function resetChart (newVal, oldVal) {
141+
function watchOther (newVal, oldVal) {
149142
if (isEmpty(newVal)) return;
150143
if (angular.equals(newVal, oldVal)) return;
151144
var chartType = type || scope.chartType;
152145
if (! chartType) return;
153146

154147
// chart.update() doesn't work for series and labels
155148
// so we have to re-create the chart entirely
156-
createChart(chartType);
149+
createChart(chartType, scope, elem);
157150
}
158151

159-
function createChart (type) {
160-
// TODO: check parent?
161-
if (isResponsive(type, scope) && elem[0].clientHeight === 0) {
162-
return $timeout(function () {
163-
createChart(type);
164-
}, 50, false);
165-
}
166-
if (! hasData(scope)) return;
167-
scope.chartGetColor = typeof scope.chartGetColor === 'function' ? scope.chartGetColor : getRandomColor;
168-
var colors = getColors(type, scope);
169-
var cvs = elem[0], ctx = cvs.getContext('2d');
170-
var data = Array.isArray(scope.chartData[0]) ?
171-
getDataSets(scope.chartLabels, scope.chartData, scope.chartSeries || [], colors, scope.chartDatasetOverride) :
172-
getData(scope.chartLabels, scope.chartData, colors, scope.chartDatasetOverride);
173-
174-
var options = angular.extend({}, ChartJs.getOptions(type), scope.chartOptions);
175-
// Destroy old chart if it exists to avoid ghost charts issue
176-
// https://github.com/jtblin/angular-chart.js/issues/187
177-
destroyChart(chart, scope);
178-
179-
chart = new ChartJs.Chart(ctx, {
180-
type: type,
181-
data: data,
182-
options: options
183-
});
184-
scope.$emit('chart-create', chart);
185-
186-
// Bind events
187-
cvs.onclick = scope.chartClick ? getEventHandler(scope, chart, 'chartClick', false) : angular.noop;
188-
cvs.onmousemove = scope.chartHover ? getEventHandler(scope, chart, 'chartHover', true) : angular.noop;
152+
function watchType (newVal, oldVal) {
153+
if (isEmpty(newVal)) return;
154+
if (angular.equals(newVal, oldVal)) return;
155+
createChart(newVal, scope, elem);
189156
}
190157
}
191158
};
192159
};
193160

161+
function createChart (type, scope, elem) {
162+
var options = getChartOptions(type, scope);
163+
if (! hasData(scope) || ! canDisplay(type, scope, elem, options)) return;
164+
165+
var cvs = elem[0];
166+
var ctx = cvs.getContext('2d');
167+
168+
scope.chartGetColor = getChartColorFn(scope);
169+
var data = getChartData(type, scope);
170+
171+
// Destroy old chart if it exists to avoid ghost charts issue
172+
// https://github.com/jtblin/angular-chart.js/issues/187
173+
destroyChart(scope);
174+
175+
scope.chart = new ChartJs.Chart(ctx, {
176+
type: type,
177+
data: data,
178+
options: options
179+
});
180+
scope.$emit('chart-create', scope.chart);
181+
bindEvents(cvs, scope);
182+
}
183+
194184
function canUpdateChart (newVal, oldVal) {
195185
if (newVal && oldVal && newVal.length && oldVal.length) {
196186
return Array.isArray(newVal[0]) ?
@@ -205,12 +195,12 @@
205195
return carry + val;
206196
}
207197

208-
function getEventHandler (scope, chart, action, triggerOnlyOnChange) {
198+
function getEventHandler (scope, action, triggerOnlyOnChange) {
209199
var lastState = null;
210200
return function (evt) {
211-
var atEvent = chart.getElementsAtEvent || chart.getPointsAtEvent;
201+
var atEvent = scope.chart.getElementsAtEvent || scope.chart.getPointsAtEvent;
212202
if (atEvent) {
213-
var activePoints = atEvent.call(chart, evt);
203+
var activePoints = atEvent.call(scope.chart, evt);
214204
if (triggerOnlyOnChange === false || angular.equals(lastState, activePoints) === false) {
215205
lastState = activePoints;
216206
scope[action](activePoints, evt);
@@ -280,6 +270,17 @@
280270
scope.chartLabels && scope.chartLabels.length;
281271
}
282272

273+
function getChartColorFn (scope) {
274+
return typeof scope.chartGetColor === 'function' ? scope.chartGetColor : getRandomColor;
275+
}
276+
277+
function getChartData (type, scope) {
278+
var colors = getColors(type, scope);
279+
return Array.isArray(scope.chartData[0]) ?
280+
getDataSets(scope.chartLabels, scope.chartData, scope.chartSeries || [], colors, scope.chartDatasetOverride) :
281+
getData(scope.chartLabels, scope.chartData, colors, scope.chartDatasetOverride);
282+
}
283+
283284
function getDataSets (labels, data, series, colors, datasetOverride) {
284285
return {
285286
labels: labels,
@@ -315,17 +316,26 @@
315316
return dataset;
316317
}
317318

318-
function updateChart (chart, values, scope) {
319+
function getChartOptions (type, scope) {
320+
return angular.extend({}, ChartJs.getOptions(type), scope.chartOptions);
321+
}
322+
323+
function bindEvents (cvs, scope) {
324+
cvs.onclick = scope.chartClick ? getEventHandler(scope, 'chartClick', false) : angular.noop;
325+
cvs.onmousemove = scope.chartHover ? getEventHandler(scope, 'chartHover', true) : angular.noop;
326+
}
327+
328+
function updateChart (values, scope) {
319329
if (Array.isArray(scope.chartData[0])) {
320-
chart.data.datasets.forEach(function (dataset, i) {
330+
scope.chart.data.datasets.forEach(function (dataset, i) {
321331
dataset.data = values[i];
322332
});
323333
} else {
324-
chart.data.datasets[0].data = values;
334+
scope.chart.data.datasets[0].data = values;
325335
}
326336

327-
chart.update();
328-
scope.$emit('chart-update', chart);
337+
scope.chart.update();
338+
scope.$emit('chart-update', scope.chart);
329339
}
330340

331341
function isEmpty (value) {
@@ -334,15 +344,21 @@
334344
(typeof value === 'object' && ! Object.keys(value).length);
335345
}
336346

337-
function isResponsive (type, scope) {
338-
var options = angular.extend({}, Chart.defaults.global, ChartJs.getOptions(type), scope.chartOptions);
339-
return options.responsive;
347+
function canDisplay (type, scope, elem, options) {
348+
// TODO: check parent?
349+
if (options.responsive && elem[0].clientHeight === 0) {
350+
$timeout(function () {
351+
createChart(type, scope, elem);
352+
}, 50, false);
353+
return false;
354+
}
355+
return true;
340356
}
341357

342-
function destroyChart(chart, scope) {
343-
if(! chart) return;
344-
chart.destroy();
345-
scope.$emit('chart-destroy', chart);
358+
function destroyChart(scope) {
359+
if(! scope.chart) return;
360+
scope.chart.destroy();
361+
scope.$emit('chart-destroy', scope.chart);
346362
}
347363
}
348364
}));

0 commit comments

Comments
 (0)