From fb0531ce22c0b040368bffcb2e686c7c22771bde Mon Sep 17 00:00:00 2001 From: deepak Date: Mon, 9 Apr 2018 07:27:52 +0530 Subject: [PATCH 01/13] Brush animation during resize. Fix issue with brush resizing on chart resizing. --- src/coordinate-grid-mixin.js | 37 +++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 5294d84c5..9b8b08c5b 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -1014,22 +1014,16 @@ dc.coordinateGridMixin = function (_chart) { return _chart.effectiveWidth(); } - _chart.renderBrush = function (g) { + _chart.renderBrush = function (g, doTransition) { if (_brushOn) { _brush.on('start brush end', _chart._brushing); - // Set boundaries of the brush, must set it before applying to _gBrush - _brush.extent([[0, 0], [brushWidth(), brushHeight()]]); - // To retrieve selection we need _gBrush _gBrush = g.append('g') .attr('class', 'brush') - .attr('transform', 'translate(' + _chart.margins().left + ',' + _chart.margins().top + ')') - .call(_brush); + .attr('transform', 'translate(' + _chart.margins().left + ',' + _chart.margins().top + ')'); - _chart.setHandlePaths(_gBrush); - - _chart.redrawBrush(_chart.filter()); + _chart.redrawBrush(_chart.filter(), doTransition); } }; @@ -1040,8 +1034,10 @@ dc.coordinateGridMixin = function (_chart) { .enter() .append('path') .attr('class', 'handle--custom') - .attr('d', _chart.resizeHandlePath) .merge(_brushHandles); + + _brushHandles + .attr('d', _chart.resizeHandlePath); }; _chart.extendBrush = function (selection) { @@ -1076,7 +1072,7 @@ dc.coordinateGridMixin = function (_chart) { selection = _chart.extendBrush(selection); - _chart.redrawBrush(selection); + _chart.redrawBrush(selection, false); if (_chart.brushIsEmpty(selection)) { dc.events.trigger(function () { @@ -1093,8 +1089,16 @@ dc.coordinateGridMixin = function (_chart) { } }; - _chart.redrawBrush = function (selection) { + _chart.redrawBrush = function (selection, doTransition) { if (_brushOn && _gBrush) { + // Set boundaries of the brush, must set it before applying to _gBrush + _brush.extent([[0, 0], [brushWidth(), brushHeight()]]); + + _gBrush + .call(_brush); + + _chart.setHandlePaths(_gBrush); + if (!selection) { _brush.move(_gBrush, null); @@ -1102,9 +1106,11 @@ dc.coordinateGridMixin = function (_chart) { .attr('display', 'none'); } else { var scaledSelection = [_x(selection[0]), _x(selection[1])]; - _brush.move(_gBrush, scaledSelection); - _brushHandles + dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_gBrush) + .call(_brush.move, scaledSelection); + + dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_brushHandles) .attr('display', null) .attr('transform', function (d, i) { return 'translate(' + _x(selection[i]) + ', 0)'; @@ -1217,7 +1223,8 @@ dc.coordinateGridMixin = function (_chart) { if (render) { _chart.renderBrush(_chart.g(), false); } else { - _chart.redrawBrush(_chart.filter()); + // Animate the brush only while resizing + _chart.redrawBrush(_chart.filter(), _resizing); } _chart.fadeDeselectedArea(_chart.filter()); _resizing = false; From 1b39fcd8bb3a692a40e13548cff4029d6d449730 Mon Sep 17 00:00:00 2001 From: deepak Date: Wed, 11 Apr 2018 13:55:53 +0530 Subject: [PATCH 02/13] - Consistent signature for redrawBrush - Fix range-series.html - avoiding infinite mutual recursion - Due to an edge case in comparing old and new domains, more `rescale` were invoked than needed - Created a new function for comparing arrays including test cases --- spec/utils-spec.js | 57 ++++++++++++++++++++++++++++++++++ src/bubble-chart.js | 2 +- src/composite-chart.js | 2 +- src/coordinate-grid-mixin.js | 31 +++++++++--------- src/scatter-plot.js | 4 +-- src/utils.js | 21 +++++++++++++ web/examples/range-series.html | 6 ++-- web/js/dc.js | 21 +++++++++++++ 8 files changed, 124 insertions(+), 20 deletions(-) diff --git a/spec/utils-spec.js b/spec/utils-spec.js index b32e29c96..fa8d663b8 100644 --- a/spec/utils-spec.js +++ b/spec/utils-spec.js @@ -130,4 +130,61 @@ describe('dc utils', function () { expect(date.toString()).toEqual(makeDate(2012, 0, 1).toString()); }); }); + describe('dc.utils.arraysEqual', function () { + it('nulls should be equal', function () { + var a1 = null; + var a2 = null; + expect(dc.utils.arraysEqual(a1, a2)).toBe(true); + }); + it('null should not be equal to any array', function () { + var a1 = null; + var a2 = []; + expect(dc.utils.arraysEqual(a1, a2)).toBe(false); + }); + it('any array should not be equal to null', function () { + var a1 = null; + var a2 = []; + expect(dc.utils.arraysEqual(a1, a2)).toBe(false); + }); + it('empty arrays should be', function () { + var a1 = []; + var a2 = []; + expect(dc.utils.arraysEqual(a1, a2)).toBe(true); + }); + it('should identify equal arrays - numbers', function () { + var a1 = [1, 2, 3]; + var a2 = [1, 2, 3]; + expect(dc.utils.arraysEqual(a1, a2)).toBe(true); + }); + it('should identify equal arrays - strings', function () { + var a1 = ['apple', 'mangoes', 'oranges', 'bananas']; + var a2 = ['apple', 'mangoes', 'oranges', 'bananas']; + expect(dc.utils.arraysEqual(a1, a2)).toBe(true); + }); + it('should identify equal arrays - dates', function () { + var a1 = [makeDate(2012, 1, 1), makeDate(2013, 10, 15)]; + var a2 = [makeDate(2012, 1, 1), makeDate(2013, 10, 15)]; + expect(dc.utils.arraysEqual(a1, a2)).toBe(true); + }); + it('should identify unequal arrays - numbers', function () { + var a1 = [4, 2, 3]; + var a2 = [1, 2, 3]; + expect(dc.utils.arraysEqual(a1, a2)).toBe(false); + }); + it('should identify unequal arrays - strings', function () { + var a1 = ['apple', 'cherries', 'oranges', 'bananas']; + var a2 = ['apple', 'mangoes', 'oranges', 'bananas']; + expect(dc.utils.arraysEqual(a1, a2)).toBe(false); + }); + it('should identify unequal arrays - dates', function () { + var a1 = [makeDate(2012, 1, 1), makeDate(2013, 10, 15)]; + var a2 = [makeDate(2012, 1, 1), makeDate(2013, 10, 16)]; + expect(dc.utils.arraysEqual(a1, a2)).toBe(false); + }); + it('should identify equal arrays with one of the elements as 0', function () { + var a1 = [0, 20]; + var a2 = [0, 20]; + expect(dc.utils.arraysEqual(a1, a2)).toBe(true); + }); + }); }); diff --git a/src/bubble-chart.js b/src/bubble-chart.js index 7ae603997..fd32a84bb 100644 --- a/src/bubble-chart.js +++ b/src/bubble-chart.js @@ -128,7 +128,7 @@ dc.bubbleChart = function (parent, chartGroup) { // override default x axis brush from parent chart }; - _chart.redrawBrush = function (g, selection, doTransition) { + _chart.redrawBrush = function (selection, doTransition) { // override default x axis brush from parent chart _chart.fadeDeselectedArea(selection); }; diff --git a/src/composite-chart.js b/src/composite-chart.js index 77fc1e248..efffc30d8 100644 --- a/src/composite-chart.js +++ b/src/composite-chart.js @@ -87,7 +87,7 @@ dc.compositeChart = function (parent, chartGroup) { } selection = _chart.extendBrush(selection); - _chart.redrawBrush(selection); + _chart.redrawBrush(selection, false); var brushIsEmpty = _chart.brushIsEmpty(selection); diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 9b8b08c5b..35214920a 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -456,11 +456,6 @@ dc.coordinateGridMixin = function (_chart) { return groups.map(_chart.keyAccessor()); }; - function compareDomains (d1, d2) { - return !d1 || !d2 || d1.length !== d2.length || - d1.some(function (elem, i) { return (elem && d2[i]) ? elem.toString() !== d2[i].toString() : elem === d2[i]; }); - } - function prepareXAxis (g, render) { if (!_chart.isOrdinal()) { if (_chart.elasticX()) { @@ -483,7 +478,7 @@ dc.coordinateGridMixin = function (_chart) { // has the domain changed? var xdom = _x.domain(); - if (render || compareDomains(_lastXDomain, xdom)) { + if (render || !dc.utils.arraysEqual(_lastXDomain, xdom)) { _chart.rescale(); } _lastXDomain = xdom; @@ -978,7 +973,7 @@ dc.coordinateGridMixin = function (_chart) { _chart._filter(_); - _chart.redrawBrush(_); + _chart.redrawBrush(_, false); return _chart; }); @@ -1023,6 +1018,8 @@ dc.coordinateGridMixin = function (_chart) { .attr('class', 'brush') .attr('transform', 'translate(' + _chart.margins().left + ',' + _chart.margins().top + ')'); + _chart.setBrushExtents(); + _chart.redrawBrush(_chart.filter(), doTransition); } }; @@ -1089,15 +1086,21 @@ dc.coordinateGridMixin = function (_chart) { } }; - _chart.redrawBrush = function (selection, doTransition) { - if (_brushOn && _gBrush) { - // Set boundaries of the brush, must set it before applying to _gBrush - _brush.extent([[0, 0], [brushWidth(), brushHeight()]]); + _chart.setBrushExtents = function () { + // Set boundaries of the brush, must set it before applying to _gBrush + _brush.extent([[0, 0], [brushWidth(), brushHeight()]]); - _gBrush - .call(_brush); + _gBrush + .call(_brush); - _chart.setHandlePaths(_gBrush); + _chart.setHandlePaths(_gBrush); + }; + + _chart.redrawBrush = function (selection, doTransition) { + if (_brushOn && _gBrush) { + if (_resizing) { + _chart.setBrushExtents(); + } if (!selection) { _brush.move(_gBrush, null); diff --git a/src/scatter-plot.js b/src/scatter-plot.js index da9610704..91ad56898 100644 --- a/src/scatter-plot.js +++ b/src/scatter-plot.js @@ -432,7 +432,7 @@ dc.scatterPlot = function (parent, chartGroup) { } selection = _chart.extendBrush(selection); - _chart.redrawBrush(selection); + _chart.redrawBrush(selection, false); if (brushIsEmpty) { dc.events.trigger(function () { @@ -451,7 +451,7 @@ dc.scatterPlot = function (parent, chartGroup) { } }; - _chart.redrawBrush = function (selection) { + _chart.redrawBrush = function (selection, doTransition) { // override default x axis brush from parent chart var _brush = _chart.brush(); var _gBrush = _chart.gBrush(); diff --git a/src/utils.js b/src/utils.js index c33eaa0ce..387113a2d 100644 --- a/src/utils.js +++ b/src/utils.js @@ -311,3 +311,24 @@ dc.utils.appendOrSelect = function (parent, selector, tag) { * @returns {Number} */ dc.utils.safeNumber = function (n) { return dc.utils.isNumber(+n) ? +n : 0;}; + +/** + * Return true if both arrays are equal, if both array are null these are considered equal + * @method arraysEqual + * @memberof dc.utils + * @param {Array|null} a1 + * @param {Array|null} a2 + * @returns {Boolean} + */ +dc.utils.arraysEqual = function (a1, a2) { + if (!a1 || !a2) { + return a1 === a2; + } + + return a1.length === a2.length && + // If elements are not integers/strings, we hope that it will match because of toString + // Test cases cover dates as well. + a1.every(function (elem, i) { + return elem === a2[i] || elem.toString() === a2[i].toString(); + }); +}; diff --git a/web/examples/range-series.html b/web/examples/range-series.html index e7d31e2dd..da5dbc4dd 100644 --- a/web/examples/range-series.html +++ b/web/examples/range-series.html @@ -84,7 +84,9 @@ }); } else if (!rangesEqual(chart.filter(), overviewChart.focusChart().filter())) { dc.events.trigger(function () { - overviewChart.focusChart().focus(chart.filter()); + // The second parameter is quite important. It asks the focus operation not to propagate events + // In absence of that it will cause infinite mutual recursion + overviewChart.focusChart().focus(chart.filter(), true); }); } }); @@ -98,7 +100,7 @@ .group(runGroup) .seriesAccessor(function(d) {return "Expt: " + d.key[0];}) .keyAccessor(function(d) {return +d.key[1];}) - .valueAccessor(function(d) {return +d.value - 500;}) + .valueAccessor(function(d) {return +d.value - 500;}); dc.renderAll(); diff --git a/web/js/dc.js b/web/js/dc.js index d5347f79a..1bc5d35a4 100644 --- a/web/js/dc.js +++ b/web/js/dc.js @@ -912,6 +912,27 @@ dc.utils.appendOrSelect = function (parent, selector, tag) { */ dc.utils.safeNumber = function (n) { return dc.utils.isNumber(+n) ? +n : 0;}; +/** + * Return true if both arrays are equal, if both array are null these are considered equal + * @method arraysEqual + * @memberof dc.utils + * @param {Array|null} a1 + * @param {Array|null} a2 + * @returns {Boolean} + */ +dc.utils.arraysEqual = function (a1, a2) { + if (!a1 || !a2) { + return a1 === a2; + } + + return a1.length === a2.length && + // If elements are not integers/strings, we hope that it will match because of toString + // Test cases cover dates as well. + a1.every(function (elem, i) { + return elem === a2[i] || elem.toString() === a2[i].toString(); + }); +}; + dc.logger = {}; dc.logger.enableDebugLog = false; From b982da57d7db1a8f2259a3a29f791117d99fc72b Mon Sep 17 00:00:00 2001 From: deepak Date: Wed, 11 Apr 2018 17:16:35 +0530 Subject: [PATCH 03/13] Inlined functions `brushWidth` and `brushHeight` --- src/coordinate-grid-mixin.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 35214920a..14908f336 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -1001,14 +1001,6 @@ dc.coordinateGridMixin = function (_chart) { return _chart; }; - function brushHeight () { - return _chart.effectiveHeight(); - } - - function brushWidth () { - return _chart.effectiveWidth(); - } - _chart.renderBrush = function (g, doTransition) { if (_brushOn) { _brush.on('start brush end', _chart._brushing); @@ -1088,7 +1080,7 @@ dc.coordinateGridMixin = function (_chart) { _chart.setBrushExtents = function () { // Set boundaries of the brush, must set it before applying to _gBrush - _brush.extent([[0, 0], [brushWidth(), brushHeight()]]); + _brush.extent([[0, 0], [_chart.effectiveWidth(), _chart.effectiveHeight()]]); _gBrush .call(_brush); @@ -1130,7 +1122,7 @@ dc.coordinateGridMixin = function (_chart) { // borrowed from Crossfilter example _chart.resizeHandlePath = function (d) { d = d.type; - var e = +(d === 'e'), x = e ? 1 : -1, y = brushHeight() / 3; + var e = +(d === 'e'), x = e ? 1 : -1, y = _chart.effectiveHeight() / 3; return 'M' + (0.5 * x) + ',' + y + 'A6,6 0 0 ' + e + ' ' + (6.5 * x) + ',' + (y + 6) + 'V' + (2 * y - 6) + From b480d1e00a94c40ca0efef13492fad3dab61d4bc Mon Sep 17 00:00:00 2001 From: deepak Date: Wed, 11 Apr 2018 23:27:03 +0530 Subject: [PATCH 04/13] CSS class name: handle--custom --> custom-brush-handle --- src/coordinate-grid-mixin.js | 5 +++-- style/dc.scss | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 14908f336..abbdbbf71 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -16,6 +16,7 @@ dc.coordinateGridMixin = function (_chart) { var VERTICAL_CLASS = 'vertical'; var Y_AXIS_LABEL_CLASS = 'y-axis-label'; var X_AXIS_LABEL_CLASS = 'x-axis-label'; + var CUSTOM_BRUSH_HANDLE_CLASS = 'custom-brush-handle'; var DEFAULT_AXIS_LABEL_PADDING = 12; _chart = dc.colorMixin(dc.marginMixin(dc.baseMixin(_chart))); @@ -1017,12 +1018,12 @@ dc.coordinateGridMixin = function (_chart) { }; _chart.setHandlePaths = function (gBrush) { - _brushHandles = gBrush.selectAll('.handle--custom').data([{type: 'w'}, {type: 'e'}]); + _brushHandles = gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS).data([{type: 'w'}, {type: 'e'}]); _brushHandles = _brushHandles .enter() .append('path') - .attr('class', 'handle--custom') + .attr('class', CUSTOM_BRUSH_HANDLE_CLASS) .merge(_brushHandles); _brushHandles diff --git a/style/dc.scss b/style/dc.scss index ca3506413..91df7af7e 100644 --- a/style/dc.scss +++ b/style/dc.scss @@ -101,7 +101,7 @@ div.dc-chart { fill-opacity: .125; } } - .handle--custom { + .custom-brush-handle { fill: $color_gallery; stroke: $color_storm_dust; cursor: ew-resize; From 5ab03bf8c01efa120a17e490c57313a48ba12c3a Mon Sep 17 00:00:00 2001 From: deepak Date: Thu, 12 Apr 2018 11:43:11 +0530 Subject: [PATCH 05/13] Brush handles transition in Y axis as well in Bar charts. --- src/coordinate-grid-mixin.js | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index abbdbbf71..a8f23805f 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -48,7 +48,6 @@ dc.coordinateGridMixin = function (_chart) { var _brush = d3.brushX(); var _gBrush; - var _brushHandles; var _brushOn = true; var _round; @@ -1013,20 +1012,22 @@ dc.coordinateGridMixin = function (_chart) { _chart.setBrushExtents(); + _chart.createBrushHandlePaths(_gBrush, doTransition); + _chart.redrawBrush(_chart.filter(), doTransition); } }; - _chart.setHandlePaths = function (gBrush) { - _brushHandles = gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS).data([{type: 'w'}, {type: 'e'}]); + _chart.createBrushHandlePaths = function (gBrush) { + var brushHandles = gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS).data([{type: 'w'}, {type: 'e'}]); - _brushHandles = _brushHandles + brushHandles = brushHandles .enter() .append('path') .attr('class', CUSTOM_BRUSH_HANDLE_CLASS) - .merge(_brushHandles); + .merge(brushHandles); - _brushHandles + brushHandles .attr('d', _chart.resizeHandlePath); }; @@ -1079,38 +1080,41 @@ dc.coordinateGridMixin = function (_chart) { } }; - _chart.setBrushExtents = function () { + _chart.setBrushExtents = function (doTransition) { // Set boundaries of the brush, must set it before applying to _gBrush _brush.extent([[0, 0], [_chart.effectiveWidth(), _chart.effectiveHeight()]]); _gBrush .call(_brush); - - _chart.setHandlePaths(_gBrush); }; _chart.redrawBrush = function (selection, doTransition) { if (_brushOn && _gBrush) { if (_resizing) { - _chart.setBrushExtents(); + _chart.setBrushExtents(doTransition); } + var gBrush = + dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_gBrush); + if (!selection) { - _brush.move(_gBrush, null); + gBrush + .call(_brush.move, null); - _brushHandles + gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS) .attr('display', 'none'); } else { var scaledSelection = [_x(selection[0]), _x(selection[1])]; - dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_gBrush) + gBrush .call(_brush.move, scaledSelection); - dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_brushHandles) + gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS) .attr('display', null) .attr('transform', function (d, i) { return 'translate(' + _x(selection[i]) + ', 0)'; - }); + }) + .attr('d', _chart.resizeHandlePath); } } _chart.fadeDeselectedArea(selection); From a98d28c60d7d4326c550eb2c414b7bc285a9e1d7 Mon Sep 17 00:00:00 2001 From: deepak Date: Thu, 12 Apr 2018 13:07:11 +0530 Subject: [PATCH 06/13] Brush handles transition in line charts (composite charts) --- src/composite-chart.js | 2 ++ src/coordinate-grid-mixin.js | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/composite-chart.js b/src/composite-chart.js index efffc30d8..041e6d33b 100644 --- a/src/composite-chart.js +++ b/src/composite-chart.js @@ -91,6 +91,8 @@ dc.compositeChart = function (parent, chartGroup) { var brushIsEmpty = _chart.brushIsEmpty(selection); + _chart.replaceFilter(brushIsEmpty ? null : selection); + for (var i = 0; i < _children.length; ++i) { _children[i].replaceFilter(brushIsEmpty ? null : selection); } diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index a8f23805f..0e3d4a5f5 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -1094,18 +1094,18 @@ dc.coordinateGridMixin = function (_chart) { _chart.setBrushExtents(doTransition); } - var gBrush = - dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_gBrush); - if (!selection) { - gBrush + _gBrush .call(_brush.move, null); - gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS) + _gBrush.selectAll('path.' + CUSTOM_BRUSH_HANDLE_CLASS) .attr('display', 'none'); } else { var scaledSelection = [_x(selection[0]), _x(selection[1])]; + var gBrush = + dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_gBrush); + gBrush .call(_brush.move, scaledSelection); From 5f446e100c8fca80bc7e9117a68babf1a225e2eb Mon Sep 17 00:00:00 2001 From: deepak Date: Thu, 12 Apr 2018 13:30:21 +0530 Subject: [PATCH 07/13] CSS class name: handle--custom --> custom-brush-handle --- spec/bar-chart-spec.js | 3 ++- spec/composite-chart-spec.js | 5 +++-- spec/line-chart-spec.js | 3 ++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/spec/bar-chart-spec.js b/spec/bar-chart-spec.js index b466462d8..40540c47d 100644 --- a/spec/bar-chart-spec.js +++ b/spec/bar-chart-spec.js @@ -716,7 +716,8 @@ describe('dc.barChart', function () { }); it('should create a fancy brush resize handle', function () { - var selectAll = chart.select('g.brush').selectAll('path.handle--custom'); + var selectAll = chart.select('g.brush').selectAll('path.custom-brush-handle'); + expect(selectAll.size()).toBe(2); selectAll.each(function (d, i) { if (i === 0) { expect(d3.select(this).attr('d')) diff --git a/spec/composite-chart-spec.js b/spec/composite-chart-spec.js index 05c99066f..8c62405a9 100644 --- a/spec/composite-chart-spec.js +++ b/spec/composite-chart-spec.js @@ -236,8 +236,9 @@ describe('dc.compositeChart', function () { }); it('should have a resize handle', function () { - expect(chart.selectAll('g.brush path.handle--custom').size()).not.toBe(0); - chart.selectAll('g.brush path.handle--custom').each(function (d, i) { + var selectAll = chart.select('g.brush').selectAll('path.custom-brush-handle'); + expect(selectAll.size()).toBe(2); + selectAll.each(function (d, i) { if (i === 0) { expect(d3.select(this).attr('d')) .toMatchPath('M-0.5,36.666666666666664A6,6 0 0 0 -6.5,42.666666666666664V67.33333333333333A6,' + diff --git a/spec/line-chart-spec.js b/spec/line-chart-spec.js index 5a9c73a02..a3e80d95b 100644 --- a/spec/line-chart-spec.js +++ b/spec/line-chart-spec.js @@ -673,7 +673,8 @@ describe('dc.lineChart', function () { }); it('should create a fancy brush resize handle', function () { - var selectAll = chart.select('g.brush').selectAll('path.handle--custom'); + var selectAll = chart.select('g.brush').selectAll('path.custom-brush-handle'); + expect(selectAll.size()).toBe(2); selectAll.each(function (d, i) { if (i === 0) { expect(d3.select(this).attr('d')) From d5551d299b3384c792402c9c05b1f472f7989a71 Mon Sep 17 00:00:00 2001 From: deepak Date: Thu, 12 Apr 2018 13:30:40 +0530 Subject: [PATCH 08/13] Enable brushing --- web/resizing/resizing-series.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/resizing/resizing-series.html b/web/resizing/resizing-series.html index 4866bd342..deba6aa15 100644 --- a/web/resizing/resizing-series.html +++ b/web/resizing/resizing-series.html @@ -34,7 +34,7 @@ .height(window.innerHeight-adjustY) .chart(function(c) { return dc.lineChart(c).curve(d3.curveCardinal); }) .x(d3.scaleLinear().domain([0,20])) - .brushOn(false) + .brushOn(true) .yAxisLabel("Measured Speed km/s") .xAxisLabel("Run") .clipPadding(10) From c001c0ebcb5a9669dc9e9d67ee5c924403cc8228 Mon Sep 17 00:00:00 2001 From: deepak Date: Thu, 12 Apr 2018 23:08:45 +0530 Subject: [PATCH 09/13] Removed duplicate code between composite chart and coordinate grid mixin related to brushing. Line chart brushing now works with resizing (http://localhost:8888/web/resizing/resizing-right-axis.html) --- spec/composite-chart-spec.js | 2 ++ spec/coordinate-grid-chart-spec.js | 2 +- src/composite-chart.js | 32 +++++------------------------- src/coordinate-grid-mixin.js | 22 ++++++++++---------- 4 files changed, 18 insertions(+), 40 deletions(-) diff --git a/spec/composite-chart-spec.js b/spec/composite-chart-spec.js index 8c62405a9..eef02dd51 100644 --- a/spec/composite-chart-spec.js +++ b/spec/composite-chart-spec.js @@ -4,6 +4,8 @@ describe('dc.compositeChart', function () { dateIdSumGroup, dateIdNegativeSumGroup, dateGroup; beforeEach(function () { + dc.constants.EVENT_DELAY = 0; // so that dc.events.trigger executes immediately + data = crossfilter(loadDateFixture()); dateDimension = data.dimension(function (d) { return d3.utcDay(d.dd); }); dateValueSumGroup = dateDimension.group().reduceSum(function (d) { return d.value; }); diff --git a/spec/coordinate-grid-chart-spec.js b/spec/coordinate-grid-chart-spec.js index 715090d2a..2d612045c 100644 --- a/spec/coordinate-grid-chart-spec.js +++ b/spec/coordinate-grid-chart-spec.js @@ -741,7 +741,7 @@ describe('dc.coordinateGridChart', function () { }); it('should update its range chart\'s filter', function () { - expect(chart.rangeChart().filter()).toEqual(chart.filter()); + expect(dc.utils.arraysEqual(chart.rangeChart().filter(), chart.filter())).toEqual(true); }); it('should trigger redraw on its range chart', function () { diff --git a/src/composite-chart.js b/src/composite-chart.js index 041e6d33b..b4c3f0d40 100644 --- a/src/composite-chart.js +++ b/src/composite-chart.js @@ -60,6 +60,7 @@ dc.compositeChart = function (parent, chartGroup) { child.svg(_chart.svg()); child.xUnits(_chart.xUnits()); child.transitionDuration(_chart.transitionDuration(), _chart.transitionDelay()); + // TODO: Determine if we can switch off brush for children and only keep it on for parent child.brushOn(_chart.brushOn()); child.renderTitle(_chart.renderTitle()); child.elasticX(_chart.elasticX()); @@ -68,34 +69,12 @@ dc.compositeChart = function (parent, chartGroup) { return g; }); - _chart._brushing = function () { - // Avoids infinite recursion (mutual recursion between range and focus operations) - // Source Event will be null when brush.move is called programmatically (see below as well). - if (!d3.event.sourceEvent) { return; } - - // Ignore event if recursive event - i.e. not directly generated by user action (like mouse/touch etc.) - // In this case we are more worried about this handler causing brush move programmatically which will - // cause this handler to be invoked again with a new d3.event (and current event set as sourceEvent) - // This check avoids recursive calls - if (d3.event.sourceEvent.type && ['start', 'brush', 'end'].indexOf(d3.event.sourceEvent.type) !== -1) { - return; - } - - var selection = d3.event.selection; - if (selection) { - selection = selection.map(_chart.x().invert); - } - selection = _chart.extendBrush(selection); - - _chart.redrawBrush(selection, false); - - var brushIsEmpty = _chart.brushIsEmpty(selection); - - _chart.replaceFilter(brushIsEmpty ? null : selection); - + _chart.applyBrushSelection = function (rangedFilter) { + _chart.replaceFilter(rangedFilter); for (var i = 0; i < _children.length; ++i) { - _children[i].replaceFilter(brushIsEmpty ? null : selection); + _children[i].replaceFilter(rangedFilter); } + _chart.redrawGroup(); }; _chart._prepareYAxis = function () { @@ -287,7 +266,6 @@ dc.compositeChart = function (parent, chartGroup) { _chart.fadeDeselectedArea = function (selection) { for (var i = 0; i < _children.length; ++i) { var child = _children[i]; - child.brush(_chart.brush()); child.fadeDeselectedArea(selection); } }; diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 0e3d4a5f5..1cb0bf716 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -1065,19 +1065,17 @@ dc.coordinateGridMixin = function (_chart) { _chart.redrawBrush(selection, false); - if (_chart.brushIsEmpty(selection)) { - dc.events.trigger(function () { - _chart.filter(null); - _chart.redrawGroup(); - }, dc.constants.EVENT_DELAY); - } else { - var rangedFilter = dc.filters.RangedFilter(selection[0], selection[1]); + var rangedFilter = _chart.brushIsEmpty(selection) ? null : dc.filters.RangedFilter(selection[0], selection[1]); - dc.events.trigger(function () { - _chart.replaceFilter(rangedFilter); - _chart.redrawGroup(); - }, dc.constants.EVENT_DELAY); - } + dc.events.trigger(function () { + _chart.applyBrushSelection(rangedFilter); + }, dc.constants.EVENT_DELAY); + }; + + // This can be overridden in a derived chart. For example Composite chart overrides it + _chart.applyBrushSelection = function (rangedFilter) { + _chart.replaceFilter(rangedFilter); + _chart.redrawGroup(); }; _chart.setBrushExtents = function (doTransition) { From 1e4ff9467b19a34f17c79ce815dea5490784f43d Mon Sep 17 00:00:00 2001 From: deepak Date: Fri, 13 Apr 2018 23:19:56 +0530 Subject: [PATCH 10/13] Brushing is applied only to the container chart not to the children --- src/composite-chart.js | 3 +-- src/series-chart.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/composite-chart.js b/src/composite-chart.js index b4c3f0d40..8fc8300fb 100644 --- a/src/composite-chart.js +++ b/src/composite-chart.js @@ -60,8 +60,7 @@ dc.compositeChart = function (parent, chartGroup) { child.svg(_chart.svg()); child.xUnits(_chart.xUnits()); child.transitionDuration(_chart.transitionDuration(), _chart.transitionDelay()); - // TODO: Determine if we can switch off brush for children and only keep it on for parent - child.brushOn(_chart.brushOn()); + child.brushOn(false); child.renderTitle(_chart.renderTitle()); child.elasticX(_chart.elasticX()); } diff --git a/src/series-chart.js b/src/series-chart.js index 283f13533..1278b7dd6 100644 --- a/src/series-chart.js +++ b/src/series-chart.js @@ -60,7 +60,7 @@ dc.seriesChart = function (parent, chartGroup) { .group({all: d3.functor(sub.values)}, sub.key) .keyAccessor(_chart.keyAccessor()) .valueAccessor(_chart.valueAccessor()) - .brushOn(_chart.brushOn()); + .brushOn(false); }); // this works around the fact compositeChart doesn't really // have a removal interface From dd640d8b4ac45452df83bd8bcf2ec0ad7802d097 Mon Sep 17 00:00:00 2001 From: deepak Date: Fri, 13 Apr 2018 23:20:43 +0530 Subject: [PATCH 11/13] Brushing support transition during resizing --- src/scatter-plot.js | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/scatter-plot.js b/src/scatter-plot.js index 91ad56898..bea0c75d2 100644 --- a/src/scatter-plot.js +++ b/src/scatter-plot.js @@ -434,21 +434,12 @@ dc.scatterPlot = function (parent, chartGroup) { _chart.redrawBrush(selection, false); - if (brushIsEmpty) { - dc.events.trigger(function () { - _chart.filter(null); - _chart.redrawGroup(); - }); - - } else { - var ranged2DFilter = dc.filters.RangedTwoDimensionalFilter(selection); - dc.events.trigger(function () { - _chart.filter(null); - _chart.filter(ranged2DFilter); - _chart.redrawGroup(); - }, dc.constants.EVENT_DELAY); + var ranged2DFilter = brushIsEmpty ? null : dc.filters.RangedTwoDimensionalFilter(selection); - } + dc.events.trigger(function () { + _chart.replaceFilter(ranged2DFilter); + _chart.redrawGroup(); + }, dc.constants.EVENT_DELAY); }; _chart.redrawBrush = function (selection, doTransition) { @@ -456,17 +447,30 @@ dc.scatterPlot = function (parent, chartGroup) { var _brush = _chart.brush(); var _gBrush = _chart.gBrush(); - if (_brush && _gBrush) { - if (selection) { + if (_chart.brushOn() && _gBrush) { + if (_chart.resizing()) { + _chart.setBrushExtents(doTransition); + } + + if (!selection) { + _gBrush + .call(_brush.move, selection); + + } else { selection = selection.map(function (point) { return point.map(function (coord, i) { var scale = i === 0 ? _chart.x() : _chart.y(); return scale(coord); }); }); - } - _brush.move(_gBrush, selection); + var gBrush = + dc.optionalTransition(doTransition, _chart.transitionDuration(), _chart.transitionDelay())(_gBrush); + + gBrush + .call(_brush.move, selection); + + } } _chart.fadeDeselectedArea(selection); From 763ca3b9ce5bc647dfdee04d1c592919a450b5c0 Mon Sep 17 00:00:00 2001 From: deepak Date: Fri, 13 Apr 2018 23:21:14 +0530 Subject: [PATCH 12/13] Resizing scatter brushing example --- web/resizing/index.html | 1 + web/resizing/resizing-scatter-brushing.html | 94 +++++++++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 web/resizing/resizing-scatter-brushing.html diff --git a/web/resizing/index.html b/web/resizing/index.html index 4dad02376..822dd0355 100644 --- a/web/resizing/index.html +++ b/web/resizing/index.html @@ -15,6 +15,7 @@

Eyeball tests for resizing dc.js charts

resizing row + resizing scatter brushing resizing series diff --git a/web/resizing/resizing-scatter-brushing.html b/web/resizing/resizing-scatter-brushing.html new file mode 100644 index 000000000..efe3f9d41 --- /dev/null +++ b/web/resizing/resizing-scatter-brushing.html @@ -0,0 +1,94 @@ + + + + dc.js - Scatter Plot Brushing Example + + + + +
+
+
+ + + + + + +
+ + From 03e21f81134c15722aa440d0681d64cef69b80ce Mon Sep 17 00:00:00 2001 From: deepak Date: Sat, 14 Apr 2018 21:08:37 +0530 Subject: [PATCH 13/13] Concept of `parentBrushOn`, fixes #1404. --- src/composite-chart.js | 1 + src/coordinate-grid-mixin.js | 19 +++++++++++++++++++ src/line-chart.js | 2 +- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/composite-chart.js b/src/composite-chart.js index 8fc8300fb..5c9a884a7 100644 --- a/src/composite-chart.js +++ b/src/composite-chart.js @@ -60,6 +60,7 @@ dc.compositeChart = function (parent, chartGroup) { child.svg(_chart.svg()); child.xUnits(_chart.xUnits()); child.transitionDuration(_chart.transitionDuration(), _chart.transitionDelay()); + child.parentBrushOn(_chart.brushOn()); child.brushOn(false); child.renderTitle(_chart.renderTitle()); child.elasticX(_chart.elasticX()); diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 1cb0bf716..731c994b4 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -49,6 +49,7 @@ dc.coordinateGridMixin = function (_chart) { var _brush = d3.brushX(); var _gBrush; var _brushOn = true; + var _parentBrushOn = false; var _round; var _renderHorizontalGridLine = false; @@ -1439,6 +1440,24 @@ dc.coordinateGridMixin = function (_chart) { return _chart; }; + /** + * This will be internally used by composite chart onto children. Please go not invoke directly. + * + * @method parentBrushOn + * @memberof dc.coordinateGridMixin + * @protected + * @instance + * @param {Boolean} [brushOn=false] + * @returns {Boolean|dc.coordinateGridMixin} + */ + _chart.parentBrushOn = function (brushOn) { + if (!arguments.length) { + return _parentBrushOn; + } + _parentBrushOn = brushOn; + return _chart; + }; + // Get the SVG rendered brush _chart.gBrush = function () { return _gBrush; diff --git a/src/line-chart.js b/src/line-chart.js index aea026f66..6c38d65a2 100644 --- a/src/line-chart.js +++ b/src/line-chart.js @@ -353,7 +353,7 @@ dc.lineChart = function (parent, chartGroup) { } function drawDots (chartBody, layers) { - if (_chart.xyTipsOn() === 'always' || (!_chart.brushOn() && _chart.xyTipsOn())) { + if (_chart.xyTipsOn() === 'always' || (!(_chart.brushOn() || _chart.parentBrushOn()) && _chart.xyTipsOn())) { var tooltipListClass = TOOLTIP_G_CLASS + '-list'; var tooltips = chartBody.select('g.' + tooltipListClass);