From a202f39794ab610b7e4365c781b4238a98d21945 Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Tue, 4 Dec 2018 14:20:38 +0000 Subject: [PATCH 1/7] Add support for hiding axis when a dataset visibility is toggled --- docs/axes/radial/linear.md | 1 + src/core/core.scale.js | 30 +++++++++++++++++++++++++++++- src/plugins/plugin.legend.js | 17 ++++++++++++++++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/docs/axes/radial/linear.md b/docs/axes/radial/linear.md index d1edd18be54..661a08f91c1 100644 --- a/docs/axes/radial/linear.md +++ b/docs/axes/radial/linear.md @@ -12,6 +12,7 @@ The axis has configuration properties for ticks, angle lines (line that appear i | ---- | ---- | ----------- | `angleLines` | `Object` | Angle line configuration. [more...](#angle-line-options) | `gridLines` | `Object` | Grid line configuration. [more...](../styling.md#grid-line-configuration) +| `hideAxisOnDataHide` | `Boolean` | `false` | If true, axis will not be drawn when a dataset is toggled off / hidden | `pointLabels` | `Object` | Point label configuration. [more...](#point-label-options) | `ticks` | `Object` | Tick configuration. [more...](#tick-options) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 8f7df002162..b4b088f97e9 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -685,7 +685,35 @@ module.exports = Element.extend({ draw: function(chartArea) { var me = this; var options = me.options; - if (!options.display) { + + var atLeastOneDatasetForAxisIsVisible; + if (me.type === 'linear' && options.hideAxisOnDataHide) { + var datasetIndexesForAxis = me.chart.data.datasets.reduce(function(acc, record, idx) { + if (record.yAxisID === me.id) { + acc.push(idx); + } + return acc; + }, []); + + var datasetsMetaForAxis = datasetIndexesForAxis.map(function(index) { + return me.chart.getDatasetMeta(index); + }); + + datasetsMetaForAxis.forEach(function(meta, dsmIdx) { + if (meta.hidden === null) { + var datasetIndex = datasetIndexesForAxis[dsmIdx]; + if (!me.chart.data.datasets[datasetIndex].hidden) { + atLeastOneDatasetForAxisIsVisible = true; + } + } else if (!meta.hidden) { + atLeastOneDatasetForAxisIsVisible = true; + } + }); + } else { + atLeastOneDatasetForAxisIsVisible = true; + } + + if (!options.display || !atLeastOneDatasetForAxisIsVisible) { return; } diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 8e6b75f65fe..ff724fe88d6 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -22,7 +22,22 @@ defaults._set('global', { var meta = ci.getDatasetMeta(index); // See controller.isDatasetVisible comment - meta.hidden = meta.hidden === null ? !ci.data.datasets[index].hidden : null; + var isHidden = meta.hidden; + var labelAxisConfig = ci.scales[meta.yAxisID].options; + var doHideAxisOnDataHide = labelAxisConfig.hideAxisOnDataHide; + var isDatasetHidden = ci.data.datasets[index].hidden; + + if (isHidden === null) { + meta.hidden = !isDatasetHidden; + if (doHideAxisOnDataHide) { + labelAxisConfig.display = !meta.hidden; + } + } else { + meta.hidden = null; + if (doHideAxisOnDataHide) { + labelAxisConfig.display = !isDatasetHidden; + } + } // We hid a dataset ... rerender the chart ci.update(); From eea441984eaa49930220b87730f3931d6c7f0692 Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Tue, 4 Dec 2018 16:26:45 +0000 Subject: [PATCH 2/7] Reduce code complexity --- src/plugins/plugin.legend.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index ff724fe88d6..d27d03c11a0 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -29,14 +29,12 @@ defaults._set('global', { if (isHidden === null) { meta.hidden = !isDatasetHidden; - if (doHideAxisOnDataHide) { - labelAxisConfig.display = !meta.hidden; - } } else { meta.hidden = null; - if (doHideAxisOnDataHide) { - labelAxisConfig.display = !isDatasetHidden; - } + } + + if (doHideAxisOnDataHide) { + labelAxisConfig.display = isHidden === null ? !meta.hidden : !isDatasetHidden; } // We hid a dataset ... rerender the chart From cdb27b516c79084602a4cdfa6ae1f6dbc1706e85 Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Thu, 6 Dec 2018 22:45:00 +0000 Subject: [PATCH 3/7] Handle multiple datasets against one axis when hiding axis --- src/plugins/plugin.legend.js | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index d27d03c11a0..350952d5ec2 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -25,15 +25,20 @@ defaults._set('global', { var isHidden = meta.hidden; var labelAxisConfig = ci.scales[meta.yAxisID].options; var doHideAxisOnDataHide = labelAxisConfig.hideAxisOnDataHide; + var allDatasetsForAxis = ci.data.datasets.filter(function(dataset) { + return dataset.yAxisID === meta.yAxisID; + }); var isDatasetHidden = ci.data.datasets[index].hidden; - if (isHidden === null) { - meta.hidden = !isDatasetHidden; - } else { - meta.hidden = null; - } + var isAtLeastOneDatasetForAxisVisible = allDatasetsForAxis.some(function(dataset) { + return !dataset.hidden; + }); + + meta.hidden = isHidden === null ? !isDatasetHidden : null; - if (doHideAxisOnDataHide) { + if (isAtLeastOneDatasetForAxisVisible || !doHideAxisOnDataHide) { + labelAxisConfig.display = true; + } else { labelAxisConfig.display = isHidden === null ? !meta.hidden : !isDatasetHidden; } From 99feb7a6506ff7dedb223dea3208eeee997cad27 Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Fri, 7 Dec 2018 19:34:48 +0000 Subject: [PATCH 4/7] Coderev --- docs/axes/README.md | 2 +- docs/axes/radial/linear.md | 1 - src/core/core.scale.js | 55 ++++++++++++++++++------------------ src/plugins/plugin.legend.js | 20 +------------ 4 files changed, 29 insertions(+), 49 deletions(-) diff --git a/docs/axes/README.md b/docs/axes/README.md index 549732812c3..9a1f5149f17 100644 --- a/docs/axes/README.md +++ b/docs/axes/README.md @@ -16,7 +16,7 @@ The following properties are common to all axes provided by Chart.js. | Name | Type | Default | Description | ---- | ---- | ------- | ----------- -| `display` | `Boolean` | `true` | If set to `false` the axis is hidden from view. Overrides *gridLines.display*, *scaleLabel.display*, and *ticks.display*. +| `display` | `Boolean (or "auto")` | `true` | If set to `false` the axis is hidden from view. Overrides *gridLines.display*, *scaleLabel.display*, and *ticks.display*. | `callbacks` | `Object` | | Callback functions to hook into the axis lifecycle. [more...](#callbacks) | `weight` | `Number` | `0` | The weight used to sort the axis. Higher weights are further away from the chart area. diff --git a/docs/axes/radial/linear.md b/docs/axes/radial/linear.md index 661a08f91c1..d1edd18be54 100644 --- a/docs/axes/radial/linear.md +++ b/docs/axes/radial/linear.md @@ -12,7 +12,6 @@ The axis has configuration properties for ticks, angle lines (line that appear i | ---- | ---- | ----------- | `angleLines` | `Object` | Angle line configuration. [more...](#angle-line-options) | `gridLines` | `Object` | Grid line configuration. [more...](../styling.md#grid-line-configuration) -| `hideAxisOnDataHide` | `Boolean` | `false` | If true, axis will not be drawn when a dataset is toggled off / hidden | `pointLabels` | `Object` | Point label configuration. [more...](#point-label-options) | `ticks` | `Object` | Tick configuration. [more...](#tick-options) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index b4b088f97e9..774e05452d3 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -680,40 +680,39 @@ module.exports = Element.extend({ return result; }, - // Actually draw the scale on the canvas - // @param {rectangle} chartArea : the area of the chart to draw full grid lines on - draw: function(chartArea) { + /** + * @private + */ + _isVisible: function() { var me = this; - var options = me.options; - - var atLeastOneDatasetForAxisIsVisible; - if (me.type === 'linear' && options.hideAxisOnDataHide) { - var datasetIndexesForAxis = me.chart.data.datasets.reduce(function(acc, record, idx) { - if (record.yAxisID === me.id) { - acc.push(idx); - } - return acc; - }, []); + var chart = me.chart; + var display = me.options.display; + var i, ilen, meta; - var datasetsMetaForAxis = datasetIndexesForAxis.map(function(index) { - return me.chart.getDatasetMeta(index); - }); + if (display !== 'auto') { + return !!display; + } - datasetsMetaForAxis.forEach(function(meta, dsmIdx) { - if (meta.hidden === null) { - var datasetIndex = datasetIndexesForAxis[dsmIdx]; - if (!me.chart.data.datasets[datasetIndex].hidden) { - atLeastOneDatasetForAxisIsVisible = true; - } - } else if (!meta.hidden) { - atLeastOneDatasetForAxisIsVisible = true; + // When 'auto', the scale is visible if at least one associated dataset is visible. + for (i = 0, ilen = chart.data.datasets.length; i < ilen; ++i) { + if (chart.isDatasetVisible(i)) { + meta = chart.getDatasetMeta(i); + if (meta.xAxisID === me.id || meta.yAxisID === me.id) { + return true; } - }); - } else { - atLeastOneDatasetForAxisIsVisible = true; + } } - if (!options.display || !atLeastOneDatasetForAxisIsVisible) { + return false; + }, + + // Actually draw the scale on the canvas + // @param {rectangle} chartArea : the area of the chart to draw full grid lines on + draw: function(chartArea) { + var me = this; + var options = me.options; + + if (!me._isVisible()) { return; } diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index 350952d5ec2..8e6b75f65fe 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -22,25 +22,7 @@ defaults._set('global', { var meta = ci.getDatasetMeta(index); // See controller.isDatasetVisible comment - var isHidden = meta.hidden; - var labelAxisConfig = ci.scales[meta.yAxisID].options; - var doHideAxisOnDataHide = labelAxisConfig.hideAxisOnDataHide; - var allDatasetsForAxis = ci.data.datasets.filter(function(dataset) { - return dataset.yAxisID === meta.yAxisID; - }); - var isDatasetHidden = ci.data.datasets[index].hidden; - - var isAtLeastOneDatasetForAxisVisible = allDatasetsForAxis.some(function(dataset) { - return !dataset.hidden; - }); - - meta.hidden = isHidden === null ? !isDatasetHidden : null; - - if (isAtLeastOneDatasetForAxisVisible || !doHideAxisOnDataHide) { - labelAxisConfig.display = true; - } else { - labelAxisConfig.display = isHidden === null ? !meta.hidden : !isDatasetHidden; - } + meta.hidden = meta.hidden === null ? !ci.data.datasets[index].hidden : null; // We hid a dataset ... rerender the chart ci.update(); From 9cb217bff4f6aa176580776f7171a56a2f2b12a8 Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Sat, 8 Dec 2018 10:46:56 +0000 Subject: [PATCH 5/7] Update scale.fit() --- src/core/core.scale.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/core.scale.js b/src/core/core.scale.js index 774e05452d3..afb4dc42c11 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -396,7 +396,7 @@ module.exports = Element.extend({ var tickOpts = opts.ticks; var scaleLabelOpts = opts.scaleLabel; var gridLineOpts = opts.gridLines; - var display = opts.display; + var display = this._isVisible(); var position = opts.position; var isHorizontal = me.isHorizontal(); From e3181a9466f0fd96d6d38f4eab9ead91c780668d Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Mon, 10 Dec 2018 16:41:15 +0000 Subject: [PATCH 6/7] Coderev, add tests --- docs/axes/README.md | 2 +- src/core/core.scale.js | 2 +- test/specs/core.scale.tests.js | 130 +++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 2 deletions(-) diff --git a/docs/axes/README.md b/docs/axes/README.md index 9a1f5149f17..90b2525e79e 100644 --- a/docs/axes/README.md +++ b/docs/axes/README.md @@ -16,7 +16,7 @@ The following properties are common to all axes provided by Chart.js. | Name | Type | Default | Description | ---- | ---- | ------- | ----------- -| `display` | `Boolean (or "auto")` | `true` | If set to `false` the axis is hidden from view. Overrides *gridLines.display*, *scaleLabel.display*, and *ticks.display*. +| `display` | `Boolean`/`String` | `true` | Controls the axis global visibility (visible when `true`, hidden when `false`). When `display: 'auto'`, the axis is visible only if at least one associated dataset is visible. | `callbacks` | `Object` | | Callback functions to hook into the axis lifecycle. [more...](#callbacks) | `weight` | `Number` | `0` | The weight used to sort the axis. Higher weights are further away from the chart area. diff --git a/src/core/core.scale.js b/src/core/core.scale.js index afb4dc42c11..0707c72bf3b 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -396,7 +396,7 @@ module.exports = Element.extend({ var tickOpts = opts.ticks; var scaleLabelOpts = opts.scaleLabel; var gridLineOpts = opts.gridLines; - var display = this._isVisible(); + var display = me._isVisible(); var position = opts.position; var isHorizontal = me.isHorizontal(); diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index 573c132ae07..dfd1db7fcdc 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -152,4 +152,134 @@ describe('Core.scale', function() { })).toEqual(test.expected); }); }); + + describe('given the axes display option is set to auto', function() { + describe('for the x axes', function() { + it('should draw the axes if at least one associated dataset is visible', function() { + var chart = window.acquireChart({ + type: 'line', + data: { + datasets: [{ + data: [100, 200, 100, 50], + xAxisId: 'foo', + hidden: true, + labels: ['Q1', 'Q2', 'Q3', 'Q4'] + }, { + data: [100, 200, 100, 50], + xAxisId: 'foo', + labels: ['Q1', 'Q2', 'Q3', 'Q4'] + }] + }, + options: { + scales: { + xAxes: [{ + id: 'foo', + display: 'auto' + }], + yAxes: [{ + type: 'category', + id: 'yScale0' + }] + } + } + }); + + var scale = chart.scales.foo; + scale.ctx = window.createMockContext(); + chart.draw(); + + expect(scale.ctx.getCalls().length > 0).toBe(true); + }); + + it('should not draw the axes if no associated datasets are visible', function() { + var chart = window.acquireChart({ + type: 'line', + data: { + datasets: [{ + data: [100, 200, 100, 50], + xAxisId: 'foo', + hidden: true, + labels: ['Q1', 'Q2', 'Q3', 'Q4'] + }] + }, + options: { + scales: { + xAxes: [{ + id: 'foo', + display: 'auto' + }] + } + } + }); + + var scale = chart.scales.foo; + scale.ctx = window.createMockContext(); + chart.draw(); + + expect(scale.ctx.getCalls().length).toBe(0); + }); + }); + + describe('for the y axes', function() { + it('should draw the axes if at least one associated dataset is visible', function() { + var chart = window.acquireChart({ + type: 'line', + data: { + datasets: [{ + data: [100, 200, 100, 50], + yAxisId: 'foo', + hidden: true, + labels: ['Q1', 'Q2', 'Q3', 'Q4'] + }, { + data: [100, 200, 100, 50], + yAxisId: 'foo', + labels: ['Q1', 'Q2', 'Q3', 'Q4'] + }] + }, + options: { + scales: { + yAxes: [{ + id: 'foo', + display: 'auto' + }] + } + } + }); + + var scale = chart.scales.foo; + scale.ctx = window.createMockContext(); + chart.draw(); + + expect(scale.ctx.getCalls().length > 0).toBe(true); + }); + + it('should not draw the axes if no associated datasets are visible', function() { + var chart = window.acquireChart({ + type: 'line', + data: { + datasets: [{ + data: [100, 200, 100, 50], + yAxisId: 'foo', + hidden: true, + labels: ['Q1', 'Q2', 'Q3', 'Q4'] + }] + }, + options: { + scales: { + yAxes: [{ + id: 'foo', + display: 'auto' + }] + } + } + }); + + var scale = chart.scales.foo; + scale.ctx = window.createMockContext(); + chart.draw(); + + expect(scale.ctx.getCalls().length).toBe(0); + }); + }); + }); }); From 65c9f57b1a6ee6564cc5ef21d90d1ab88c385dc6 Mon Sep 17 00:00:00 2001 From: Dave Salomon Date: Sat, 15 Dec 2018 19:53:18 +0000 Subject: [PATCH 7/7] Coderev - Tests --- test/specs/core.scale.tests.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/specs/core.scale.tests.js b/test/specs/core.scale.tests.js index dfd1db7fcdc..0595566cbff 100644 --- a/test/specs/core.scale.tests.js +++ b/test/specs/core.scale.tests.js @@ -188,7 +188,8 @@ describe('Core.scale', function() { scale.ctx = window.createMockContext(); chart.draw(); - expect(scale.ctx.getCalls().length > 0).toBe(true); + expect(scale.ctx.getCalls().length).toBeGreaterThan(0); + expect(scale.height).toBeGreaterThan(0); }); it('should not draw the axes if no associated datasets are visible', function() { @@ -217,6 +218,7 @@ describe('Core.scale', function() { chart.draw(); expect(scale.ctx.getCalls().length).toBe(0); + expect(scale.height).toBe(0); }); }); @@ -250,7 +252,8 @@ describe('Core.scale', function() { scale.ctx = window.createMockContext(); chart.draw(); - expect(scale.ctx.getCalls().length > 0).toBe(true); + expect(scale.ctx.getCalls().length).toBeGreaterThan(0); + expect(scale.width).toBeGreaterThan(0); }); it('should not draw the axes if no associated datasets are visible', function() { @@ -279,6 +282,7 @@ describe('Core.scale', function() { chart.draw(); expect(scale.ctx.getCalls().length).toBe(0); + expect(scale.width).toBe(0); }); }); });