From 007a8759bf72e2b5a31424cb8fa1183fa3586e8a Mon Sep 17 00:00:00 2001 From: deepak Date: Thu, 12 Apr 2018 23:08:45 +0530 Subject: [PATCH 1/8] 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 ecba31ac2..e3a8b0e18 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 3c351bea2..96e74d8ce 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 43ef1e48a..fc9380859 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 brushSelection = d3.event.selection; - if (brushSelection) { - brushSelection = brushSelection.map(_chart.x().invert); - } - brushSelection = _chart.extendBrush(brushSelection); - - _chart.redrawBrush(brushSelection, false); - - var brushIsEmpty = _chart.brushIsEmpty(brushSelection); - - _chart.replaceFilter(brushIsEmpty ? null : brushSelection); - + _chart.applyBrushSelection = function (rangedFilter) { + _chart.replaceFilter(rangedFilter); for (var i = 0; i < _children.length; ++i) { - _children[i].replaceFilter(brushIsEmpty ? null : brushSelection); + _children[i].replaceFilter(rangedFilter); } + _chart.redrawGroup(); }; _chart._prepareYAxis = function () { @@ -287,7 +266,6 @@ dc.compositeChart = function (parent, chartGroup) { _chart.fadeDeselectedArea = function (brushSelection) { for (var i = 0; i < _children.length; ++i) { var child = _children[i]; - child.brush(_chart.brush()); child.fadeDeselectedArea(brushSelection); } }; diff --git a/src/coordinate-grid-mixin.js b/src/coordinate-grid-mixin.js index 83eeda1fd..b316beab8 100644 --- a/src/coordinate-grid-mixin.js +++ b/src/coordinate-grid-mixin.js @@ -1087,19 +1087,17 @@ dc.coordinateGridMixin = function (_chart) { _chart.redrawBrush(brushSelection, false); - if (_chart.brushIsEmpty(brushSelection)) { - dc.events.trigger(function () { - _chart.filter(null); - _chart.redrawGroup(); - }, dc.constants.EVENT_DELAY); - } else { - var rangedFilter = dc.filters.RangedFilter(brushSelection[0], brushSelection[1]); + var rangedFilter = _chart.brushIsEmpty(brushSelection) ? null : dc.filters.RangedFilter(brushSelection[0], brushSelection[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 2498616ab3bdf2946bfd6a1b01ee3325c778c382 Mon Sep 17 00:00:00 2001 From: deepak Date: Fri, 13 Apr 2018 23:19:56 +0530 Subject: [PATCH 2/8] 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 fc9380859..cdf6dada6 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 609ca1599..1146e7c70 100644 --- a/src/series-chart.js +++ b/src/series-chart.js @@ -62,7 +62,7 @@ dc.seriesChart = function (parent, chartGroup) { }, 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 0dfa3d59d601ed4592891a8621fffb884e6f4ceb Mon Sep 17 00:00:00 2001 From: deepak Date: Sat, 14 Apr 2018 21:08:37 +0530 Subject: [PATCH 3/8] 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 cdf6dada6..883f46e07 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 b316beab8..0e46a9644 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; @@ -1461,6 +1462,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 6146ee854..e7d425e6b 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); From dbe00f4dbe7502fedbeaa4bdb8edc3f1654dfb6a Mon Sep 17 00:00:00 2001 From: deepak Date: Sun, 22 Apr 2018 12:01:21 +0100 Subject: [PATCH 4/8] Rebase onto `dc.js/develop`. Updated for #1422 --- src/bar-chart.js | 2 +- src/box-plot.js | 2 +- src/composite-chart.js | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/bar-chart.js b/src/bar-chart.js index 52e4ab091..03d1581d6 100644 --- a/src/bar-chart.js +++ b/src/bar-chart.js @@ -228,7 +228,7 @@ dc.barChart = function (parent, chartGroup) { bars.classed(dc.constants.SELECTED_CLASS, false); bars.classed(dc.constants.DESELECTED_CLASS, false); } - } else if (_chart.brushOn()) { + } else if (_chart.brushOn() || _chart.parentBrushOn()) { if (!_chart.brushIsEmpty(brushSelection)) { var start = brushSelection[0]; var end = brushSelection[1]; diff --git a/src/box-plot.js b/src/box-plot.js index 434c1cd4d..5c2bb4271 100644 --- a/src/box-plot.js +++ b/src/box-plot.js @@ -192,7 +192,7 @@ dc.boxPlot = function (parent, chartGroup) { } }); } else { - if (!_chart.brushOn()) { + if (!(_chart.brushOn() || _chart.parentBrushOn())) { return; } var start = brushSelection[0]; diff --git a/src/composite-chart.js b/src/composite-chart.js index 883f46e07..702d88ed1 100644 --- a/src/composite-chart.js +++ b/src/composite-chart.js @@ -264,9 +264,11 @@ dc.compositeChart = function (parent, chartGroup) { }; _chart.fadeDeselectedArea = function (brushSelection) { - for (var i = 0; i < _children.length; ++i) { - var child = _children[i]; - child.fadeDeselectedArea(brushSelection); + if (_chart.brushOn()) { + for (var i = 0; i < _children.length; ++i) { + var child = _children[i]; + child.fadeDeselectedArea(brushSelection); + } } }; From cf73244a5ae63dec0017c84e4da59a9689b6add5 Mon Sep 17 00:00:00 2001 From: deepak Date: Sun, 6 May 2018 20:16:42 +0530 Subject: [PATCH 5/8] range-series works without hacks --- web/examples/range-series-legacy.html | 112 ++++++++++++++++++++++++++ web/examples/range-series.html | 42 +--------- 2 files changed, 116 insertions(+), 38 deletions(-) create mode 100644 web/examples/range-series-legacy.html diff --git a/web/examples/range-series-legacy.html b/web/examples/range-series-legacy.html new file mode 100644 index 000000000..ab795d37c --- /dev/null +++ b/web/examples/range-series-legacy.html @@ -0,0 +1,112 @@ + + + + dc.js - Series Example + + + + + + +
+ + +

This page demonstrates a very hacky workaround for issue #479, using a series chart as a range chart. +

+
+ + + + + + +
+ + diff --git a/web/examples/range-series.html b/web/examples/range-series.html index da5dbc4dd..e0fe31390 100644 --- a/web/examples/range-series.html +++ b/web/examples/range-series.html @@ -11,7 +11,8 @@
-

This page demonstrates a very hacky workaround for issue #479, using a series chart as a range chart. +

This page demonstrates a line chart used as a rangeChart after #479 + is fixed by #1408.

@@ -39,6 +40,7 @@ .x(d3.scaleLinear().domain([0,20])) .brushOn(false) .yAxisLabel("Measured Speed km/s") + .yAxisPadding("5%") .xAxisLabel("Run") .elasticY(true) .dimension(runDimension) @@ -52,46 +54,10 @@ focusChart.yAxis().tickFormat(function(d) {return d3.format(',d')(d+299500);}); focusChart.margins().left += 40; - focusChart.elasticX = function() { - return arguments.length ? this : false; - }; - - function rangesEqual (range1, range2) { - if (!range1 && !range2) { - return true; - } else if (!range1 || !range2) { - return false; - } else if (range1.length === 0 && range2.length === 0) { - return true; - } else if (range1[0].valueOf() === range2[0].valueOf() && - range1[1].valueOf() === range2[1].valueOf()) { - return true; - } - return false; - } - overviewChart .width(768) .height(100) - .chart(function(c,_,_,i) { - var chart = dc.lineChart(c).curve(d3.curveCardinal); - if(i===0) - chart.on('filtered', function (chart) { - if (!chart.filter()) { - dc.events.trigger(function () { - overviewChart.focusChart().x().domain(overviewChart.focusChart().xOriginalDomain()); - overviewChart.focusChart().redraw(); - }); - } else if (!rangesEqual(chart.filter(), overviewChart.focusChart().filter())) { - dc.events.trigger(function () { - // 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); - }); - } - }); - return chart; - }) + .chart(function(c) { return dc.lineChart(c).curve(d3.curveCardinal); }) .x(d3.scaleLinear().domain([0,20])) .brushOn(true) .xAxisLabel("Run") From a57cb0b46cab669d6bff03e3e6ab907971d9abaa Mon Sep 17 00:00:00 2001 From: deepak Date: Mon, 7 May 2018 07:29:13 +0530 Subject: [PATCH 6/8] Scatter plot brushing example --- web/examples/scatter-series.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/examples/scatter-series.html b/web/examples/scatter-series.html index 8783a2d25..14ab031a7 100644 --- a/web/examples/scatter-series.html +++ b/web/examples/scatter-series.html @@ -40,7 +40,7 @@ .height(480) .chart(subChart) .x(d3.scaleLinear().domain([0,20])) - .brushOn(false) + .brushOn(true) .yAxisLabel("Measured Speed km/s") .xAxisLabel("Run") .clipPadding(10) From 976e8f68d19411c526d52ae284bd3be7793005a6 Mon Sep 17 00:00:00 2001 From: deepak Date: Mon, 7 May 2018 07:31:43 +0530 Subject: [PATCH 7/8] Using keyAccessor and valueAccessor while checking if a point is filtered --- src/scatter-plot.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scatter-plot.js b/src/scatter-plot.js index dd43188bd..69ecd8701 100644 --- a/src/scatter-plot.js +++ b/src/scatter-plot.js @@ -93,7 +93,7 @@ dc.scatterPlot = function (parent, chartGroup) { symbols.call(renderTitles, _chart.data()); symbols.each(function (d, i) { - _filtered[i] = !_chart.filter() || _chart.filter().isFiltered([d.key[0], d.key[1]]); + _filtered[i] = !_chart.filter() || _chart.filter().isFiltered([_chart.keyAccessor()(d), _chart.valueAccessor()(d)]); }); dc.transition(symbols, _chart.transitionDuration(), _chart.transitionDelay()) From 7a49b38e771546903d687f68c9ab2454670a2a3f Mon Sep 17 00:00:00 2001 From: deepak Date: Mon, 7 May 2018 22:41:26 +0530 Subject: [PATCH 8/8] Brushing on composite chart having sub charts with different dimensions --- web/examples/composite-brushing.html | 102 +++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 web/examples/composite-brushing.html diff --git a/web/examples/composite-brushing.html b/web/examples/composite-brushing.html new file mode 100644 index 000000000..42132d8dd --- /dev/null +++ b/web/examples/composite-brushing.html @@ -0,0 +1,102 @@ + + + + dc.js - Composite Chart Brushing Example + + + + + +
+ + +

Usually sub charts of a composite chart share the dimension of the parent. + However sometimes, especially when scatter plots are composed, the sub charts may + used different dimensions. This example uses two scatter plots both using array dimensions. + Typically scatter plots use two dimensional brushing (see scatter brushing), + however, composite charts only support one dimensional (along x axis) brushing.

+

Try brushing on the chart and see data getting filtered on the right.

+

For the curious, you will notice that unlike other charts brushing removes points outside range of the brush + instead of just fading them. + The three dimensions used by different charts are actually related. + When brush applies filters on all these three dimensions, all data outside the brush range actually gets removed + by crossfilter. There is no easy way to currently avoid that.

+
+
+ + + + + + +
+ +