Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Brush animation #1402

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions spec/utils-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});
2 changes: 1 addition & 1 deletion src/bubble-chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
2 changes: 1 addition & 1 deletion src/composite-chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
31 changes: 17 additions & 14 deletions src/coordinate-grid-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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;
Expand Down Expand Up @@ -978,7 +973,7 @@ dc.coordinateGridMixin = function (_chart) {

_chart._filter(_);

_chart.redrawBrush(_);
_chart.redrawBrush(_, false);

return _chart;
});
Expand Down Expand Up @@ -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);
}
};
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/scatter-plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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();
Expand Down
21 changes: 21 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
};
6 changes: 4 additions & 2 deletions web/examples/range-series.html
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered defaulting this the other way? I'm not clear which circumstances lead to infinite recursion, or which way is more backward-compatible

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we set it to false, this will invoke the zoom handler in the focus chart, which in turn will invoke brush handler in the range chart. Whenever we are using focus/range chart pairs we need it true.

Examples where that is not the case like http://localhost:8888/web/examples/multi-focus.html, it can be false.

I chose the default to false as that will be the case if someone did not pass the parameter at all.

While at this example - what stops a series chart to be used as a range chart? The reason I am asking is that I have cleaned up some code in composite chart over iterations and it might have resolved the issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will actually love to name the 2nd parameter in _chart.focus to raiseEvents instead of noRaiseEvents. To use default value false, I had to settle for that name.

});
}
});
Expand All @@ -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();

Expand Down
21 changes: 21 additions & 0 deletions web/js/dc.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.