Fixed bug where heatmap filtering would rely on array equality

dc-js · Nov 9, 2013 · 9aaf971 · 9aaf971
1 parent 45cc6e3
commit 9aaf971
Show file tree

Hide file tree

Showing 10 changed files with 132 additions and 37 deletions.
diff --git a/Gruntfile.js b/Gruntfile.js
@@ -234,5 +234,6 @@ module.exports.jsFiles = [
     "src/heatmap.js",
     "src/d3.box.js",
     "src/box-plot.js",
-    "src/footer.js"
+    "src/footer.js",
+    "src/filters.js"
 ];
diff --git a/spec/filters-spec.js b/spec/filters-spec.js
@@ -0,0 +1,59 @@
+describe('dc.filters', function () {
+    describe('RangedFilter', function () {
+        var filter;
+        beforeEach(function () {
+            filter = dc.filters.RangedFilter(0, 10);
+        });
+
+        it('should act like an array', function () {
+            expect([filter[0], filter[1]]).toEqual([0, 10]);
+        });
+
+        describe("isFiltered", function () {
+            it('should return false when the number is out of range', function () {
+                expect(filter.isFiltered(1234)).toBeFalsy();
+            });
+
+            it('should return true when the number is in range', function () {
+                expect(filter.isFiltered(8.1)).toBeTruthy();
+            });
+
+            it('should include the left bounds', function () {
+                expect(filter.isFiltered(0)).toBeTruthy();
+            });
+
+            it('should exclude the right bounds', function () {
+                expect(filter.isFiltered(10)).toBeFalsy();
+            });
+        });
+    });
+
+    describe('TwoDimensionalFilter', function () {
+        var filter;
+        beforeEach(function () {
+            filter = dc.filters.TwoDimensionalFilter([1,2]);
+        });
+
+        describe('isFiltered', function () {
+            it('should return true if both dimensions are equal', function () {
+                expect(filter.isFiltered([1,2])).toBeTruthy();
+            });
+
+            it('should return false if either dimension is not equal to the filter', function () {
+                expect(filter.isFiltered([1,5])).toBeFalsy();
+            });
+
+            it('should return false if the dimensionality is less', function () {
+                expect(filter.isFiltered([1])).toBeFalsy();
+            });
+
+            it('should return false if the dimensionality is more', function () {
+                expect(filter.isFiltered([1,2,3])).toBeFalsy();
+            });
+
+            it('should return false if the value is not an array', function () {
+                expect(filter.isFiltered(1)).toBeFalsy();
+            });
+        });
+    });
+});
diff --git a/spec/heatmap-spec.js b/spec/heatmap-spec.js
@@ -1,8 +1,8 @@
 describe("dc.heatmap", function() {
-    var id, chart, chartHeight, chartWidth;
+    var id, data, chart, chartHeight, chartWidth;
 
     beforeEach(function() {
-        var data = crossfilter(loadColorFixture());
+        data = crossfilter(loadColorFixture());
         var dimension = data.dimension(function (d) { return [+d.colData, +d.rowData]; });
         var group = dimension.group().reduceSum(function (d) { return +d.colorData; });
 
@@ -112,17 +112,44 @@ describe("dc.heatmap", function() {
     });
 
     describe('filters', function() {
+        var filterX, filterY;
         beforeEach( function() {
-            var filterValue = Math.ceil(Math.random() * 2);
-            chart.filter(filterValue).render();
+            filterX = Math.ceil(Math.random() * 2);
+            filterY = Math.ceil(Math.random() * 2);
+            chart.render();
         });
+
+        function clickCellOnChart(chart, x, y) {
+            var oneCell = chart.selectAll(".box-group").filter(function (d) {
+                return d.key[0] == x && d.key[1] == y;
+            });
+            oneCell.select("rect").on("click")(oneCell.datum());
+            return oneCell;
+        }
+
         it('cells should have the appropriate class', function() {
+            clickCellOnChart(chart, filterX, filterY);
             chart.selectAll(".box-group").each( function(d) {
                 var cell = d3.select(this);
-                expect(cell.classed("selected")).toEqual(chart.hasFilter(d.key));
-                expect(cell.classed("deselected")).not.toEqual(chart.hasFilter(d.key));
+                if (d.key[0] == filterX && d.key[1] == filterY) {
+                    expect(cell.classed("selected")).toBeTruthy();
+                    expect(chart.hasFilter(d.key)).toBeTruthy();
+                } else {
+                    expect(cell.classed("deselected")).toBeTruthy();
+                    expect(chart.hasFilter(d.key)).toBeFalsy();
+                }
             });
         });
+
+        it('should keep all data points for that cell', function () {
+            var otherDimension = data.dimension(function (d) { return +d.colData; });
+            var otherGroup = otherDimension.group().reduceSum(function (d) { return +d.colorData; });
+            var otherChart = dc.baseChart({}).dimension(otherDimension).group(otherGroup);
+
+            otherChart.render();
+            var clickedCell = clickCellOnChart(chart, filterX, filterY);
+            expect(otherChart.data()[filterX - 1].value).toEqual(clickedCell.datum().value);
+        });
     });
 
     describe('click events', function() {

diff --git a/spec/helpers/fixtures.js b/spec/helpers/fixtures.js
@@ -20,8 +20,12 @@ function loadDateFixture() {
 function loadColorFixture() {
     return JSON.parse("[" +
         "{\"colData\":\"1\", \"rowData\": \"1\", \"colorData\": \"1\"}," +
+        "{\"colData\":\"1\", \"rowData\": \"1\", \"colorData\": \"1\"}," +
+        "{\"colData\":\"1\", \"rowData\": \"2\", \"colorData\": \"2\"}," +
         "{\"colData\":\"1\", \"rowData\": \"2\", \"colorData\": \"2\"}," +
         "{\"colData\":\"2\", \"rowData\": \"1\", \"colorData\": \"3\"}," +
+        "{\"colData\":\"2\", \"rowData\": \"1\", \"colorData\": \"3\"}," +
+        "{\"colData\":\"2\", \"rowData\": \"2\", \"colorData\": \"4\"}," +
         "{\"colData\":\"2\", \"rowData\": \"2\", \"colorData\": \"4\"}" +
         "]");
 }

diff --git a/src/base-chart.js b/src/base-chart.js
@@ -74,7 +74,7 @@ dc.baseChart = function (_chart) {
             dimension.filterFunction(function (d) {
                 for(var i = 0; i < filters.length; i++) {
                     var filter = filters[i];
-                    if (filter.inRange && filter.inRange(d)) {
+                    if (filter.isFiltered && filter.isFiltered(d)) {
                         return true;
                     } else if (filter == d) {
                         return true;
@@ -86,6 +86,8 @@ dc.baseChart = function (_chart) {
         return filters;
     };
 
+
+
     var _data = function (group) {
         return group.all();
     };
@@ -525,7 +527,6 @@ dc.baseChart = function (_chart) {
     **/
     _chart.filter = function (_) {
         if (!arguments.length) return _filters.length > 0 ? _filters[0] : null;
-
         if (_ instanceof Array && _[0] instanceof Array) {
             _[0].forEach(function(d){
                 if (_chart.hasFilter(d)) {

diff --git a/src/coordinate-grid-chart.js b/src/coordinate-grid-chart.js
@@ -689,7 +689,7 @@ dc.coordinateGridChart = function (_chart) {
                 dc.redrawAll(_chart.chartGroup());
             });
         } else {
-            var rangedFilter = dc.utils.RangedFilter(extent[0], extent[1]);
+            var rangedFilter = dc.filters.RangedFilter(extent[0], extent[1]);
 
             dc.events.trigger(function () {
                 _chart.filter(null);
@@ -812,7 +812,7 @@ dc.coordinateGridChart = function (_chart) {
                 _rangeChart.focus(refDom);
             }
             _rangeChart.filter(null);
-            var refDomFilter = dc.utils.RangedFilter(refDom[0], refDom[1]);
+            var refDomFilter = dc.filters.RangedFilter(refDom[0], refDom[1]);
             _rangeChart.filter(refDomFilter);
 
             dc.events.trigger(function () {

diff --git a/src/filters.js b/src/filters.js
@@ -0,0 +1,20 @@
+dc.filters = {};
+
+dc.filters.RangedFilter = function(low, high) {
+    var range = Array(low, high);
+    range.isFiltered = function(value) {
+        return value >= this[0] && value < this[1];
+    };
+
+    return range;
+};
+
+dc.filters.TwoDimensionalFilter = function(array) {
+    var filter = array;
+    filter.isFiltered = function(value) {
+        return value.length && value.length == filter.length &&
+               value[0] == filter[0] && value[1] == filter[1];
+    };
+
+    return filter;
+};
diff --git a/src/heatmap.js b/src/heatmap.js
@@ -62,6 +62,15 @@ dc.heatMap = function (parent, chartGroup) {
         });
     }
 
+    dc.override(_chart, "filter", function(filter) {
+        if(filter) {
+            return _chart._filter(dc.filters.TwoDimensionalFilter(filter));
+        } else {
+            return _chart._filter();
+        }
+
+    });
+
     _chart.xAxisOnClick = function (d) { filterAxis(0, d); };
     _chart.yAxisOnClick = function (d) { filterAxis(1, d); };
 

diff --git a/src/utils.js b/src/utils.js
@@ -273,12 +273,3 @@ dc.utils.createLegendable = function (chart, group, accessor, color) {
 };
 
 dc.utils.safeNumber = function(n){return dc.utils.isNumber(+n)?+n:0;};
-
-dc.utils.RangedFilter = function(low, high) {
-    var range = Array(low, high);
-    range.inRange = function(value) {
-        return value >= this[0] && value < this[1];
-    };
-
-    return range;
-};
diff --git a/test/utils-test.js b/test/utils-test.js
@@ -118,23 +118,6 @@ suite.addBatch({
     }
 });
 
-suite.addBatch({
-    'dc.utils.RangedFilter': {
-        topic: function() {
-            return dc.utils.RangedFilter(0,10);
-        },
-        'should act like an array': function (rangedFilter) {
-            assert.deepEqual([rangedFilter[0], rangedFilter[1]], [0,10]);
-        },
-        'should validate numbers out of range': function (rangedFilter) {
-            assert.isFalse(rangedFilter.inRange(13245));
-        },
-        'should validate numbers that are in range': function (rangedFilter) {
-            assert.isTrue(rangedFilter.inRange(4.1));
-        }
-    }
-});
-
 suite.export(module);