Skip to content

Commit 2a0e2a1

Browse files
mbostockmbostock
committed
Sort null nodes at the end.
Previously, null nodes were passed to the comparator and were indistinguishable from non-null nodes with no bound data. For consistency with other selection operators that skip null nodes, it seems preferrable to put null nodes at the end of the selection rather than passing them to the comparator. Fixes d3#881.
1 parent 183060d commit 2a0e2a1

4 files changed

Lines changed: 30 additions & 9 deletions

File tree

‎d3.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1805,7 +1805,7 @@
18051805
function d3_selection_sortComparator(comparator) {
18061806
if (!arguments.length) comparator = d3.ascending;
18071807
return function(a, b) {
1808-
return comparator(a && a.__data__, b && b.__data__);
1808+
return !a - !b || comparator(a.__data__, b.__data__);
18091809
};
18101810
}
18111811
d3_selectionPrototype.on = function(type, listener, capture) {

‎d3.min.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/core/selection-sort.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ d3_selectionPrototype.sort = function(comparator) {
77
function d3_selection_sortComparator(comparator) {
88
if (!arguments.length) comparator = d3.ascending;
99
return function(a, b) {
10-
return comparator(a && a.__data__, b && b.__data__);
10+
return (!a - !b) || comparator(a.__data__, b.__data__);
1111
};
1212
}

‎test/core/selection-sort-test.js‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,34 @@ suite.addBatch({
3636
assert.deepEqual(span[2].map(data), [1, 10, 12, 30]);
3737
assert.deepEqual(span[3].map(data), [1, 10, 22, 40]);
3838
},
39-
"treats null nodes as null data": function(span) {
40-
var some = d3.selectAll("div:first-child span"), nulls = false;
41-
some[0][0] = null;
42-
some.sort(function(a, b) { if ((a === null) || (b === null)) ++nulls; return a - b; });
43-
assert.isTrue(nulls > 0);
39+
"sorts null nodes at the end of the selection": function() {
40+
var span = d3.selectAll("div").selectAll("span"), nulls = 0;
41+
d3.select(span[0][0]).remove();
42+
span[0][0] = null;
43+
span.sort(function(a, b) { if ((a === null) || (b === null)) ++nulls; return a - b; });
44+
assert.equal(nulls, 0);
45+
46+
assert.isNull(span[0][3]);
4447
assert.domNull(span[0][0].previousSibling);
45-
assert.deepEqual(some[0].slice(1).map(data), [3, 10, 21]);
48+
assert.domEqual(span[0][1], span[0][0].nextSibling);
49+
assert.domEqual(span[0][0], span[0][1].previousSibling);
50+
assert.domEqual(span[0][2], span[0][1].nextSibling);
51+
assert.domEqual(span[0][1], span[0][2].previousSibling);
52+
assert.domNull(span[0][2].nextSibling);
53+
assert.deepEqual(span[0].slice(0, -1).map(data), [3, 10, 21]);
54+
55+
for (var i = 1; i < 4; ++i) {
56+
var d = span[i].parentNode.__data__;
57+
assert.domNull(span[i][0].previousSibling);
58+
assert.domEqual(span[i][1], span[i][0].nextSibling);
59+
assert.domEqual(span[i][0], span[i][1].previousSibling);
60+
assert.domEqual(span[i][2], span[i][1].nextSibling);
61+
assert.domEqual(span[i][1], span[i][2].previousSibling);
62+
assert.domEqual(span[i][3], span[i][2].nextSibling);
63+
assert.domEqual(span[i][2], span[i][3].previousSibling);
64+
assert.domNull(span[i][3].nextSibling);
65+
assert.deepEqual(span[i].map(data), [1, 2 + d, 10, 20 + d].sort(d3.ascending));
66+
}
4667
},
4768
"returns the current selection": function(span) {
4869
span = d3.select("body"); // https://github.com/tmpvar/jsdom/issues/277

0 commit comments

Comments
 (0)