From 59a3eb1d1a7413fe55d73374e2a3ec27cb32bd4c Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Fri, 29 Mar 2019 18:37:55 -0400 Subject: [PATCH 01/12] replace HTML string array with DOM node creation for legendCallback --- src/plugins/plugin.legend.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 91ff07a20bd..df05edc78af 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -78,17 +78,17 @@ defaults._set('global', { }, legendCallback: function(chart) { - var text = []; - text.push(''); - return text.join(''); + return text; } }); From 5b3a13afd80bfaabcea9e5b26a98f87f43c3888b Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Fri, 29 Mar 2019 19:05:22 -0400 Subject: [PATCH 02/12] fix: don't nest labels inside spans --- src/plugins/plugin.legend.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index df05edc78af..0638b5e7890 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -82,10 +82,10 @@ defaults._set('global', { text.setAttribute('class', chart.id + '-legend'); for (var i = 0; i < chart.data.datasets.length; i++) { var listItem = text.appendChild(document.createElement('li')); - var listItemText = listItem.appendChild(document.createElement('span')); - listItemText.style.backgroundColor = chart.data.datasets[i].backgroundColor; + var listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; if (chart.data.datasets[i].label) { - listItemText.appendChild(document.createTextNode(chart.data.datasets[i].label)); + listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); } } return text; From d9b24b46d97360be145a931422003c1cfc51fdd7 Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Fri, 29 Mar 2019 19:14:48 -0400 Subject: [PATCH 03/12] replace HTML string array with DOM node creation for doughnut and polarArea legendCallback --- src/controllers/controller.doughnut.js | 14 +++++++------- src/controllers/controller.polarArea.js | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index de418e7edbc..9d52ee1a75a 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -22,8 +22,8 @@ defaults._set('doughnut', { mode: 'single' }, legendCallback: function(chart) { - var text = []; - text.push('
    '); + var text = document.createElement('ul'); + text.setAttribute('class', chart.id + '-legend'); var data = chart.data; var datasets = data.datasets; @@ -31,16 +31,16 @@ defaults._set('doughnut', { if (datasets.length) { for (var i = 0; i < datasets[0].data.length; ++i) { - text.push('
  • '); + var listItem = text.appendChild(document.createElement('li')); + var listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; if (labels[i]) { - text.push(labels[i]); + listItem.appendChild(document.createTextNode(labels[i])); } - text.push('
    • '); } } - text.push('
'); - return text.join(''); + return text; }, legend: { labels: { diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 1e0e30ba2dd..df136c82e5f 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -32,8 +32,8 @@ defaults._set('polarArea', { startAngle: -0.5 * Math.PI, legendCallback: function(chart) { - var text = []; - text.push('
    '); + var text = document.createElement('ul'); + text.setAttribute('class', chart.id + '-legend'); var data = chart.data; var datasets = data.datasets; @@ -41,16 +41,16 @@ defaults._set('polarArea', { if (datasets.length) { for (var i = 0; i < datasets[0].data.length; ++i) { - text.push('
  • '); + var listItem = text.appendChild(document.createElement('li')); + var listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; if (labels[i]) { - text.push(labels[i]); + listItem.appendChild(document.createTextNode(labels[i])); } - text.push('
    • '); } } - text.push('
'); - return text.join(''); + return text; }, legend: { labels: { From a9b1b01ce3b6c94e2a0883b668e767f9937f5cae Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Fri, 29 Mar 2019 19:17:27 -0400 Subject: [PATCH 04/12] suggestion: use for ul element variable, instead of for previous HTML string version --- src/controllers/controller.doughnut.js | 8 ++++---- src/controllers/controller.polarArea.js | 8 ++++---- src/plugins/plugin.legend.js | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 9d52ee1a75a..a3835b87844 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -22,8 +22,8 @@ defaults._set('doughnut', { mode: 'single' }, legendCallback: function(chart) { - var text = document.createElement('ul'); - text.setAttribute('class', chart.id + '-legend'); + var list = document.createElement('ul'); + list.setAttribute('class', chart.id + '-legend'); var data = chart.data; var datasets = data.datasets; @@ -31,7 +31,7 @@ defaults._set('doughnut', { if (datasets.length) { for (var i = 0; i < datasets[0].data.length; ++i) { - var listItem = text.appendChild(document.createElement('li')); + var listItem = list.appendChild(document.createElement('li')); var listItemSpan = listItem.appendChild(document.createElement('span')); listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; if (labels[i]) { @@ -40,7 +40,7 @@ defaults._set('doughnut', { } } - return text; + return list; }, legend: { labels: { diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index df136c82e5f..9c15f7cf353 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -32,8 +32,8 @@ defaults._set('polarArea', { startAngle: -0.5 * Math.PI, legendCallback: function(chart) { - var text = document.createElement('ul'); - text.setAttribute('class', chart.id + '-legend'); + var list = document.createElement('ul'); + list.setAttribute('class', chart.id + '-legend'); var data = chart.data; var datasets = data.datasets; @@ -41,7 +41,7 @@ defaults._set('polarArea', { if (datasets.length) { for (var i = 0; i < datasets[0].data.length; ++i) { - var listItem = text.appendChild(document.createElement('li')); + var listItem = list.appendChild(document.createElement('li')); var listItemSpan = listItem.appendChild(document.createElement('span')); listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; if (labels[i]) { @@ -50,7 +50,7 @@ defaults._set('polarArea', { } } - return text; + return list; }, legend: { labels: { diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 0638b5e7890..e0535beb2b2 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -78,17 +78,17 @@ defaults._set('global', { }, legendCallback: function(chart) { - var text = document.createElement('ul'); - text.setAttribute('class', chart.id + '-legend'); + var list = document.createElement('ul'); + list.setAttribute('class', chart.id + '-legend'); for (var i = 0; i < chart.data.datasets.length; i++) { - var listItem = text.appendChild(document.createElement('li')); + var listItem = list.appendChild(document.createElement('li')); var listItemSpan = listItem.appendChild(document.createElement('span')); listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; if (chart.data.datasets[i].label) { listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); } } - return text; + return list; } }); From c20aa3e8f975d7605e74e54e9ba6901ac4bfea44 Mon Sep 17 00:00:00 2001 From: dkoo Date: Mon, 1 Apr 2019 13:45:48 -0600 Subject: [PATCH 05/12] Update documentation for generateLegend method; fix corresponding unit tests --- docs/configuration/legend.md | 4 ++-- src/controllers/controller.doughnut.js | 5 ++++- src/controllers/controller.polarArea.js | 7 +++++-- src/plugins/plugin.legend.js | 21 ++++++++++++++------- test/specs/global.defaults.tests.js | 16 ++++++++++++---- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/docs/configuration/legend.md b/docs/configuration/legend.md index 5caef61759d..c5cb2109b7f 100644 --- a/docs/configuration/legend.md +++ b/docs/configuration/legend.md @@ -164,7 +164,7 @@ Now when you click the legend in this chart, the visibility of the first two dat ## HTML Legends -Sometimes you need a very complex legend. In these cases, it makes sense to generate an HTML legend. Charts provide a `generateLegend()` method on their prototype that returns an HTML string for the legend. +Sometimes you need a very complex legend. In these cases, it makes sense to generate an HTML legend. Charts provide a `generateLegend()` method on their prototype that returns an HTML fragment for the legend. To configure how this legend is generated, you can change the `legendCallback` config property. @@ -174,7 +174,7 @@ var chart = new Chart(ctx, { data: data, options: { legendCallback: function(chart) { - // Return the HTML string here. + // Return the HTML fragment here. } } }); diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index a3835b87844..7a2c20c286f 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -22,6 +22,7 @@ defaults._set('doughnut', { mode: 'single' }, legendCallback: function(chart) { + var fragment = document.createDocumentFragment(); var list = document.createElement('ul'); list.setAttribute('class', chart.id + '-legend'); @@ -38,9 +39,11 @@ defaults._set('doughnut', { listItem.appendChild(document.createTextNode(labels[i])); } } + + fragment.appendChild(list); } - return list; + return fragment; }, legend: { labels: { diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 9c15f7cf353..af74dd2501e 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -31,7 +31,8 @@ defaults._set('polarArea', { }, startAngle: -0.5 * Math.PI, - legendCallback: function(chart) { + legendCallback: function (chart) { + var fragment = document.createDocumentFragment(); var list = document.createElement('ul'); list.setAttribute('class', chart.id + '-legend'); @@ -48,9 +49,11 @@ defaults._set('polarArea', { listItem.appendChild(document.createTextNode(labels[i])); } } + + fragment.appendChild(list); } - return list; + return fragment; }, legend: { labels: { diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index e0535beb2b2..0a75292e10a 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -78,17 +78,24 @@ defaults._set('global', { }, legendCallback: function(chart) { + var fragment = document.createDocumentFragment(); var list = document.createElement('ul'); list.setAttribute('class', chart.id + '-legend'); - for (var i = 0; i < chart.data.datasets.length; i++) { - var listItem = list.appendChild(document.createElement('li')); - var listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; - if (chart.data.datasets[i].label) { - listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); + + if (datasets.length) { + for (var i = 0; i < chart.data.datasets.length; i++) { + var listItem = list.appendChild(document.createElement('li')); + var listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; + if (chart.data.datasets[i].label) { + listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); + } } + + fragment.appendChild(list); } - return list; + + return fragment; } }); diff --git a/test/specs/global.defaults.tests.js b/test/specs/global.defaults.tests.js index a01284c1a0b..38a0a74f17f 100644 --- a/test/specs/global.defaults.tests.js +++ b/test/specs/global.defaults.tests.js @@ -102,8 +102,12 @@ describe('Default Configs', function() { options: config }); - var expectedLegend = '
  • label1
  • label2
'; - expect(chart.generateLegend()).toBe(expectedLegend); + var expectedLegend = document.createDocumentFragment(); + var expectedLegendList = document.createElement('ul'); + expectedLegendList.setAttribute('class', chart.id + '-legend'); + expectedLegendList.innerHTML = '
  • label1
  • label2
    • '; + expectedLegend.appendChild(expectedLegendList); + expect(chart.generateLegend().firstElementChild.outerHTML).toBe(expectedLegend.firstElementChild.outerHTML); }); it('should return correct legend label objects', function() { @@ -218,8 +222,12 @@ describe('Default Configs', function() { options: config }); - var expectedLegend = '
    • label1
    • label2
    '; - expect(chart.generateLegend()).toBe(expectedLegend); + var expectedLegend = document.createDocumentFragment(); + var expectedLegendList = document.createElement('ul'); + expectedLegendList.setAttribute('class', chart.id + '-legend'); + expectedLegendList.innerHTML = '
  • label1
  • label2
    • '; + expectedLegend.appendChild(expectedLegendList); + expect(chart.generateLegend().firstElementChild.outerHTML).toBe(expectedLegend.firstElementChild.outerHTML); }); it('should return correct legend label objects', function() { From 9dbaed5066163a23ff8482a51dac0ca7483eb82d Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Wed, 3 Apr 2019 20:33:56 -0400 Subject: [PATCH 06/12] CR fix: Use DOM node constructors, but return HTML string for backwards compatibility. Reverts test updates --- docs/configuration/legend.md | 4 ++-- src/controllers/controller.doughnut.js | 5 +---- src/controllers/controller.polarArea.js | 7 ++----- src/plugins/plugin.legend.js | 5 +---- test/specs/global.defaults.tests.js | 16 ++++------------ 5 files changed, 10 insertions(+), 27 deletions(-) diff --git a/docs/configuration/legend.md b/docs/configuration/legend.md index c5cb2109b7f..5caef61759d 100644 --- a/docs/configuration/legend.md +++ b/docs/configuration/legend.md @@ -164,7 +164,7 @@ Now when you click the legend in this chart, the visibility of the first two dat ## HTML Legends -Sometimes you need a very complex legend. In these cases, it makes sense to generate an HTML legend. Charts provide a `generateLegend()` method on their prototype that returns an HTML fragment for the legend. +Sometimes you need a very complex legend. In these cases, it makes sense to generate an HTML legend. Charts provide a `generateLegend()` method on their prototype that returns an HTML string for the legend. To configure how this legend is generated, you can change the `legendCallback` config property. @@ -174,7 +174,7 @@ var chart = new Chart(ctx, { data: data, options: { legendCallback: function(chart) { - // Return the HTML fragment here. + // Return the HTML string here. } } }); diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 7a2c20c286f..0cad4f9122a 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -22,7 +22,6 @@ defaults._set('doughnut', { mode: 'single' }, legendCallback: function(chart) { - var fragment = document.createDocumentFragment(); var list = document.createElement('ul'); list.setAttribute('class', chart.id + '-legend'); @@ -39,11 +38,9 @@ defaults._set('doughnut', { listItem.appendChild(document.createTextNode(labels[i])); } } - - fragment.appendChild(list); } - return fragment; + return list.outerHTML; }, legend: { labels: { diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index af74dd2501e..25208a07a5d 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -31,8 +31,7 @@ defaults._set('polarArea', { }, startAngle: -0.5 * Math.PI, - legendCallback: function (chart) { - var fragment = document.createDocumentFragment(); + legendCallback: function(chart) { var list = document.createElement('ul'); list.setAttribute('class', chart.id + '-legend'); @@ -49,11 +48,9 @@ defaults._set('polarArea', { listItem.appendChild(document.createTextNode(labels[i])); } } - - fragment.appendChild(list); } - return fragment; + return list.outerHTML; }, legend: { labels: { diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 0a75292e10a..53de2256aac 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -78,7 +78,6 @@ defaults._set('global', { }, legendCallback: function(chart) { - var fragment = document.createDocumentFragment(); var list = document.createElement('ul'); list.setAttribute('class', chart.id + '-legend'); @@ -91,11 +90,9 @@ defaults._set('global', { listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); } } - - fragment.appendChild(list); } - return fragment; + return list.outerHTML; } }); diff --git a/test/specs/global.defaults.tests.js b/test/specs/global.defaults.tests.js index 38a0a74f17f..a01284c1a0b 100644 --- a/test/specs/global.defaults.tests.js +++ b/test/specs/global.defaults.tests.js @@ -102,12 +102,8 @@ describe('Default Configs', function() { options: config }); - var expectedLegend = document.createDocumentFragment(); - var expectedLegendList = document.createElement('ul'); - expectedLegendList.setAttribute('class', chart.id + '-legend'); - expectedLegendList.innerHTML = '
  • label1
  • label2
    • '; - expectedLegend.appendChild(expectedLegendList); - expect(chart.generateLegend().firstElementChild.outerHTML).toBe(expectedLegend.firstElementChild.outerHTML); + var expectedLegend = '
    • label1
    • label2
    '; + expect(chart.generateLegend()).toBe(expectedLegend); }); it('should return correct legend label objects', function() { @@ -222,12 +218,8 @@ describe('Default Configs', function() { options: config }); - var expectedLegend = document.createDocumentFragment(); - var expectedLegendList = document.createElement('ul'); - expectedLegendList.setAttribute('class', chart.id + '-legend'); - expectedLegendList.innerHTML = '
  • label1
  • label2
    • '; - expectedLegend.appendChild(expectedLegendList); - expect(chart.generateLegend().firstElementChild.outerHTML).toBe(expectedLegend.firstElementChild.outerHTML); + var expectedLegend = '
    • label1
    • label2
    '; + expect(chart.generateLegend()).toBe(expectedLegend); }); it('should return correct legend label objects', function() { From e5ad5f3adaed9d67bf006e432cc37a7c658bae7b Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Wed, 3 Apr 2019 21:35:27 -0400 Subject: [PATCH 07/12] Update unit test expected inline style spacing. --- test/specs/global.defaults.tests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/specs/global.defaults.tests.js b/test/specs/global.defaults.tests.js index a01284c1a0b..afef1aabf2a 100644 --- a/test/specs/global.defaults.tests.js +++ b/test/specs/global.defaults.tests.js @@ -102,7 +102,7 @@ describe('Default Configs', function() { options: config }); - var expectedLegend = '
    • label1
    • label2
    '; + var expectedLegend = '
    • label1
    • label2
    '; expect(chart.generateLegend()).toBe(expectedLegend); }); @@ -218,7 +218,7 @@ describe('Default Configs', function() { options: config }); - var expectedLegend = '
    • label1
    • label2
    '; + var expectedLegend = '
    • label1
    • label2
    '; expect(chart.generateLegend()).toBe(expectedLegend); }); From 6fe8ed902e62a852811d47c0f167951912cd3e60 Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Thu, 4 Apr 2019 16:34:18 -0400 Subject: [PATCH 08/12] CR fixes: variable declaration, conditional and logic control formatting --- src/controllers/controller.doughnut.js | 20 ++++++++++---------- src/controllers/controller.polarArea.js | 20 ++++++++++---------- src/plugins/plugin.legend.js | 16 ++++++++-------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 0cad4f9122a..0a3cecfbf87 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -23,20 +23,20 @@ defaults._set('doughnut', { }, legendCallback: function(chart) { var list = document.createElement('ul'); - list.setAttribute('class', chart.id + '-legend'); - var data = chart.data; var datasets = data.datasets; var labels = data.labels; - if (datasets.length) { - for (var i = 0; i < datasets[0].data.length; ++i) { - var listItem = list.appendChild(document.createElement('li')); - var listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; - if (labels[i]) { - listItem.appendChild(document.createTextNode(labels[i])); - } + var i, ilen, listItem, listItemSpan; + + list.setAttribute('class', chart.id + '-legend'); + + for (i = 0, ilen = datasets[0].data.length || 0; i < ilen; ++i) { + listItem = list.appendChild(document.createElement('li')); + listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; + if (labels[i]) { + listItem.appendChild(document.createTextNode(labels[i])); } } diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 25208a07a5d..20d336e13c3 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -33,20 +33,20 @@ defaults._set('polarArea', { startAngle: -0.5 * Math.PI, legendCallback: function(chart) { var list = document.createElement('ul'); - list.setAttribute('class', chart.id + '-legend'); - var data = chart.data; var datasets = data.datasets; var labels = data.labels; - if (datasets.length) { - for (var i = 0; i < datasets[0].data.length; ++i) { - var listItem = list.appendChild(document.createElement('li')); - var listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; - if (labels[i]) { - listItem.appendChild(document.createTextNode(labels[i])); - } + var i, ilen, listItem, listItemSpan; + + list.setAttribute('class', chart.id + '-legend'); + + for (i = 0, ilen = datasets[0].data.length || 0; i < ilen; ++i) { + listItem = list.appendChild(document.createElement('li')); + listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; + if (labels[i]) { + listItem.appendChild(document.createTextNode(labels[i])); } } diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 53de2256aac..aeae2493880 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -79,16 +79,16 @@ defaults._set('global', { legendCallback: function(chart) { var list = document.createElement('ul'); + var i, ilen, listItem, listItemSpan; + list.setAttribute('class', chart.id + '-legend'); - if (datasets.length) { - for (var i = 0; i < chart.data.datasets.length; i++) { - var listItem = list.appendChild(document.createElement('li')); - var listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; - if (chart.data.datasets[i].label) { - listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); - } + for (i = 0, ilen = chart.data.datasets.length || 0; i < ilen; i++) { + listItem = list.appendChild(document.createElement('li')); + listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; + if (chart.data.datasets[i].label) { + listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); } } From b547c1cfda491e64b2a7ec1abe41559e3c629f66 Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Fri, 10 May 2019 16:25:56 -0400 Subject: [PATCH 09/12] add check for datasets values in doughnut and polar area --- src/controllers/controller.doughnut.js | 20 +++++++++++--------- src/controllers/controller.polarArea.js | 20 +++++++++++--------- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 0a3cecfbf87..083c492dad2 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -25,18 +25,20 @@ defaults._set('doughnut', { var list = document.createElement('ul'); var data = chart.data; var datasets = data.datasets; - var labels = data.labels; - var i, ilen, listItem, listItemSpan; + if (datasets.length) { + var labels = data.labels; + var i, ilen, listItem, listItemSpan; - list.setAttribute('class', chart.id + '-legend'); + list.setAttribute('class', chart.id + '-legend'); - for (i = 0, ilen = datasets[0].data.length || 0; i < ilen; ++i) { - listItem = list.appendChild(document.createElement('li')); - listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; - if (labels[i]) { - listItem.appendChild(document.createTextNode(labels[i])); + for (i = 0, ilen = datasets[0].data.length; i < ilen; ++i) { + listItem = list.appendChild(document.createElement('li')); + listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; + if (labels[i]) { + listItem.appendChild(document.createTextNode(labels[i])); + } } } diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 20d336e13c3..da4c416cfeb 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -35,18 +35,20 @@ defaults._set('polarArea', { var list = document.createElement('ul'); var data = chart.data; var datasets = data.datasets; - var labels = data.labels; - var i, ilen, listItem, listItemSpan; + if (datasets.length) { + var labels = data.labels; + var i, ilen, listItem, listItemSpan; - list.setAttribute('class', chart.id + '-legend'); + list.setAttribute('class', chart.id + '-legend'); - for (i = 0, ilen = datasets[0].data.length || 0; i < ilen; ++i) { - listItem = list.appendChild(document.createElement('li')); - listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; - if (labels[i]) { - listItem.appendChild(document.createTextNode(labels[i])); + for (i = 0, ilen = datasets[0].data.length; i < ilen; ++i) { + listItem = list.appendChild(document.createElement('li')); + listItemSpan = listItem.appendChild(document.createElement('span')); + listItemSpan.style.backgroundColor = datasets[0].backgroundColor[i]; + if (labels[i]) { + listItem.appendChild(document.createTextNode(labels[i])); + } } } From 82aa7ee93192336211014afdca6007cb8324e3fd Mon Sep 17 00:00:00 2001 From: Ian Del Giudice Date: Fri, 10 May 2019 19:51:05 -0400 Subject: [PATCH 10/12] declare variables at the top --- src/controllers/controller.doughnut.js | 6 ++---- src/controllers/controller.polarArea.js | 6 ++---- src/plugins/plugin.legend.js | 2 +- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 083c492dad2..4f63e284fc4 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -25,13 +25,11 @@ defaults._set('doughnut', { var list = document.createElement('ul'); var data = chart.data; var datasets = data.datasets; + var labels = data.labels; + var i, ilen, listItem, listItemSpan; if (datasets.length) { - var labels = data.labels; - var i, ilen, listItem, listItemSpan; - list.setAttribute('class', chart.id + '-legend'); - for (i = 0, ilen = datasets[0].data.length; i < ilen; ++i) { listItem = list.appendChild(document.createElement('li')); listItemSpan = listItem.appendChild(document.createElement('span')); diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index da4c416cfeb..90734d9a929 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -35,13 +35,11 @@ defaults._set('polarArea', { var list = document.createElement('ul'); var data = chart.data; var datasets = data.datasets; + var labels = data.labels; + var i, ilen, listItem, listItemSpan; if (datasets.length) { - var labels = data.labels; - var i, ilen, listItem, listItemSpan; - list.setAttribute('class', chart.id + '-legend'); - for (i = 0, ilen = datasets[0].data.length; i < ilen; ++i) { listItem = list.appendChild(document.createElement('li')); listItemSpan = listItem.appendChild(document.createElement('span')); diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index aeae2493880..604c1c2c50a 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -83,7 +83,7 @@ defaults._set('global', { list.setAttribute('class', chart.id + '-legend'); - for (i = 0, ilen = chart.data.datasets.length || 0; i < ilen; i++) { + for (i = 0, ilen = chart.data.datasets.length; i < ilen; i++) { listItem = list.appendChild(document.createElement('li')); listItemSpan = listItem.appendChild(document.createElement('span')); listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; From 673bcc26854e8188f179d0046d2ac529ee832208 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Thu, 29 Aug 2019 09:08:02 -0700 Subject: [PATCH 11/12] Ensure class is set when datasets is empty --- src/controllers/controller.doughnut.js | 2 +- src/controllers/controller.polarArea.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 4f63e284fc4..7cee5444457 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -28,8 +28,8 @@ defaults._set('doughnut', { var labels = data.labels; var i, ilen, listItem, listItemSpan; + list.setAttribute('class', chart.id + '-legend'); if (datasets.length) { - list.setAttribute('class', chart.id + '-legend'); for (i = 0, ilen = datasets[0].data.length; i < ilen; ++i) { listItem = list.appendChild(document.createElement('li')); listItemSpan = listItem.appendChild(document.createElement('span')); diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 90734d9a929..76ca3b70cae 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -38,8 +38,8 @@ defaults._set('polarArea', { var labels = data.labels; var i, ilen, listItem, listItemSpan; + list.setAttribute('class', chart.id + '-legend'); if (datasets.length) { - list.setAttribute('class', chart.id + '-legend'); for (i = 0, ilen = datasets[0].data.length; i < ilen; ++i) { listItem = list.appendChild(document.createElement('li')); listItemSpan = listItem.appendChild(document.createElement('span')); From 736f96bcdb389e988d0945ab25217c7fc493e3cc Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Fri, 6 Sep 2019 06:20:49 -0700 Subject: [PATCH 12/12] Cache datasets --- src/plugins/plugin.legend.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 604c1c2c50a..dc5b9325d6f 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -79,16 +79,17 @@ defaults._set('global', { legendCallback: function(chart) { var list = document.createElement('ul'); + var datasets = chart.data.datasets; var i, ilen, listItem, listItemSpan; list.setAttribute('class', chart.id + '-legend'); - for (i = 0, ilen = chart.data.datasets.length; i < ilen; i++) { + for (i = 0, ilen = datasets.length; i < ilen; i++) { listItem = list.appendChild(document.createElement('li')); listItemSpan = listItem.appendChild(document.createElement('span')); - listItemSpan.style.backgroundColor = chart.data.datasets[i].backgroundColor; - if (chart.data.datasets[i].label) { - listItem.appendChild(document.createTextNode(chart.data.datasets[i].label)); + listItemSpan.style.backgroundColor = datasets[i].backgroundColor; + if (datasets[i].label) { + listItem.appendChild(document.createTextNode(datasets[i].label)); } }