Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix bug with unexpected transition inheritance. #682

Merged
merged 1 commit into from

1 participant

@mbostock
Owner

Prior to this change, transitions used transition.each internally, which had the
unexpected side-effect of enabling transitions on d3.transition(selection) when
called from within a tween function. This would only occur on the first
invocation of the tween function when the elapsed time between the transition
creation and the transition start was greater than the transition delay;
however, this is fairly common as the default delay for transitions is zero.

This bug caused unexpected behavior if you tried to redraw an axis within a
custom tween function: in some cases, the synchronous redraw of the axis would
compete with a concurrent transition, causing unexpected behavior. By avoiding
the use of transition.each internally, the user now controls when automatic
transitions are enabled.

@mbostock Fix bug with unexpected transition inheritance.
Prior to this change, transitions used transition.each internally, which had the
unexpected side-effect of enabling transitions on d3.transition(selection) when
called from within a tween function. This would only occur on the first
invocation of the tween function when the elapsed time between the transition
creation and the transition start was greater than the transition delay;
however, this is fairly common as the default delay for transitions is zero.

This bug caused unexpected behavior if you tried to redraw an axis within a
custom tween function: in some cases, the synchronous redraw of the axis would
compete with a concurrent transition, causing unexpected behavior. By avoiding
the use of transition.each internally, the user now controls when automatic
transitions are enabled.
cc359a9
@mbostock mbostock merged commit cc359a9 into master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 24, 2012
  1. Fix bug with unexpected transition inheritance.

    authored
    Prior to this change, transitions used transition.each internally, which had the
    unexpected side-effect of enabling transitions on d3.transition(selection) when
    called from within a tween function. This would only occur on the first
    invocation of the tween function when the elapsed time between the transition
    creation and the transition start was greater than the transition delay;
    however, this is fairly common as the default delay for transitions is zero.
    
    This bug caused unexpected behavior if you tried to redraw an axis within a
    custom tween function: in some cases, the synchronous redraw of the axis would
    compete with a concurrent transition, causing unexpected behavior. By avoiding
    the use of transition.each internally, the user now controls when automatic
    transitions are enabled.
This page is out of date. Refresh to see the latest.
View
57 d3.v2.js
@@ -2011,14 +2011,19 @@ d3_selectionPrototype.on = function(type, listener, capture) {
});
};
d3_selectionPrototype.each = function(callback) {
- for (var j = -1, m = this.length; ++j < m;) {
- for (var group = this[j], i = -1, n = group.length; ++i < n;) {
- var node = group[i];
- if (node) callback.call(node, node.__data__, i, j);
+ return d3_selection_each(this, function(node, i, j) {
+ callback.call(node, node.__data__, i, j);
+ });
+};
+
+function d3_selection_each(groups, callback) {
+ for (var j = 0, m = groups.length; j < m; j++) {
+ for (var group = groups[j], i = 0, n = group.length, node; i < n; i++) {
+ if (node = group[i]) callback(node, i, j);
}
}
- return this;
-};
+ return groups;
+}
//
// Note: assigning to the arguments array simultaneously changes the value of
// the corresponding argument!
@@ -2145,12 +2150,12 @@ function d3_transition(groups, id, time) {
};
d3.timer(function(elapsed) {
- groups.each(function(d, i, j) {
+ return d3_selection_each(groups, function(node, i, j) {
var tweened = [],
- node = this,
- delay = groups[j][i].delay,
- duration = groups[j][i].duration,
- lock = node.__transition__ || (node.__transition__ = {active: 0, count: 0});
+ delay = node.delay,
+ duration = node.duration,
+ lock = (node = node.node).__transition__ || (node.__transition__ = {active: 0, count: 0}),
+ d = node.__data__;
++lock.count;
@@ -2196,7 +2201,6 @@ function d3_transition(groups, id, time) {
return 1;
}
});
- return 1;
}, 0, time);
return groups;
@@ -2341,16 +2345,14 @@ d3_transitionPrototype.remove = function() {
});
};
d3_transitionPrototype.delay = function(value) {
- var groups = this;
- return groups.each(typeof value === "function"
- ? function(d, i, j) { groups[j][i].delay = value.apply(this, arguments) | 0; }
- : (value = value | 0, function(d, i, j) { groups[j][i].delay = value; }));
+ return d3_selection_each(this, typeof value === "function"
+ ? function(node, i, j) { node.delay = value.call(node = node.node, node.__data__, i, j) | 0; }
+ : (value = value | 0, function(node) { node.delay = value; }));
};
d3_transitionPrototype.duration = function(value) {
- var groups = this;
- return groups.each(typeof value === "function"
- ? function(d, i, j) { groups[j][i].duration = Math.max(1, value.apply(this, arguments) | 0); }
- : (value = Math.max(1, value | 0), function(d, i, j) { groups[j][i].duration = value; }));
+ return d3_selection_each(this, typeof value === "function"
+ ? function(node, i, j) { node.duration = Math.max(1, value.call(node = node.node, node.__data__, i, j) | 0); }
+ : (value = Math.max(1, value | 0), function(node) { node.duration = value; }));
};
function d3_transition_each(callback) {
var id = d3_transitionId,
@@ -2360,16 +2362,11 @@ function d3_transition_each(callback) {
d3_transitionId = this.id;
d3_transitionEase = this.ease();
- for (var j = 0, m = this.length; j < m; j++) {
- for (var group = this[j], i = 0, n = group.length; i < n; i++) {
- var node = group[i];
- if (node) {
- d3_transitionDelay = this[j][i].delay;
- d3_transitionDuration = this[j][i].duration;
- callback.call(node = node.node, node.__data__, i, j);
- }
- }
- }
+ d3_selection_each(this, function(node, i, j) {
+ d3_transitionDelay = node.delay;
+ d3_transitionDuration = node.duration;
+ callback.call(node = node.node, node.__data__, i, j);
+ });
d3_transitionId = id;
d3_transitionEase = ease;
View
8 d3.v2.min.js
4 additions, 4 deletions not shown
View
17 src/core/selection-each.js
@@ -1,9 +1,14 @@
d3_selectionPrototype.each = function(callback) {
- for (var j = -1, m = this.length; ++j < m;) {
- for (var group = this[j], i = -1, n = group.length; ++i < n;) {
- var node = group[i];
- if (node) callback.call(node, node.__data__, i, j);
+ return d3_selection_each(this, function(node, i, j) {
+ callback.call(node, node.__data__, i, j);
+ });
+};
+
+function d3_selection_each(groups, callback) {
+ for (var j = 0, m = groups.length; j < m; j++) {
+ for (var group = groups[j], i = 0, n = group.length, node; i < n; i++) {
+ if (node = group[i]) callback(node, i, j);
}
}
- return this;
-};
+ return groups;
+}
View
7 src/core/transition-delay.js
@@ -1,6 +1,5 @@
d3_transitionPrototype.delay = function(value) {
- var groups = this;
- return groups.each(typeof value === "function"
- ? function(d, i, j) { groups[j][i].delay = value.apply(this, arguments) | 0; }
- : (value = value | 0, function(d, i, j) { groups[j][i].delay = value; }));
+ return d3_selection_each(this, typeof value === "function"
+ ? function(node, i, j) { node.delay = value.call(node = node.node, node.__data__, i, j) | 0; }
+ : (value = value | 0, function(node) { node.delay = value; }));
};
View
7 src/core/transition-duration.js
@@ -1,6 +1,5 @@
d3_transitionPrototype.duration = function(value) {
- var groups = this;
- return groups.each(typeof value === "function"
- ? function(d, i, j) { groups[j][i].duration = Math.max(1, value.apply(this, arguments) | 0); }
- : (value = Math.max(1, value | 0), function(d, i, j) { groups[j][i].duration = value; }));
+ return d3_selection_each(this, typeof value === "function"
+ ? function(node, i, j) { node.duration = Math.max(1, value.call(node = node.node, node.__data__, i, j) | 0); }
+ : (value = Math.max(1, value | 0), function(node) { node.duration = value; }));
};
View
15 src/core/transition-each.js
@@ -6,16 +6,11 @@ function d3_transition_each(callback) {
d3_transitionId = this.id;
d3_transitionEase = this.ease();
- for (var j = 0, m = this.length; j < m; j++) {
- for (var group = this[j], i = 0, n = group.length; i < n; i++) {
- var node = group[i];
- if (node) {
- d3_transitionDelay = this[j][i].delay;
- d3_transitionDuration = this[j][i].duration;
- callback.call(node = node.node, node.__data__, i, j);
- }
- }
- }
+ d3_selection_each(this, function(node, i, j) {
+ d3_transitionDelay = node.delay;
+ d3_transitionDuration = node.duration;
+ callback.call(node = node.node, node.__data__, i, j);
+ });
d3_transitionId = id;
d3_transitionEase = ease;
View
11 src/core/transition.js
@@ -29,12 +29,12 @@ function d3_transition(groups, id, time) {
};
d3.timer(function(elapsed) {
- groups.each(function(d, i, j) {
+ return d3_selection_each(groups, function(node, i, j) {
var tweened = [],
- node = this,
- delay = groups[j][i].delay,
- duration = groups[j][i].duration,
- lock = node.__transition__ || (node.__transition__ = {active: 0, count: 0});
+ delay = node.delay,
+ duration = node.duration,
+ lock = (node = node.node).__transition__ || (node.__transition__ = {active: 0, count: 0}),
+ d = node.__data__;
++lock.count;
@@ -80,7 +80,6 @@ function d3_transition(groups, id, time) {
return 1;
}
});
- return 1;
}, 0, time);
return groups;
Something went wrong with that request. Please try again.