From fa94c9f3df50b7b3928be40821d17d770bdfb6f3 Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 24 Jan 2019 13:11:50 -0800 Subject: [PATCH] String coercion and comparison fixes. Fixes #88. Fixes #87. --- src/transition/attr.js | 54 ++++++++++++----------- src/transition/style.js | 45 ++++++++++--------- test/transition/attr-test.js | 82 +++++++++++++++++++++++++++++++++++ test/transition/style-test.js | 52 ++++++++++++++++++++++ 4 files changed, 187 insertions(+), 46 deletions(-) diff --git a/src/transition/attr.js b/src/transition/attr.js index 15c990e..98f42d9 100644 --- a/src/transition/attr.js +++ b/src/transition/attr.js @@ -16,52 +16,56 @@ function attrRemoveNS(fullname) { } function attrConstant(name, interpolate, value1) { - var value00, + var string00, + string1 = value1 + "", interpolate0; return function() { - var value0 = this.getAttribute(name); - return value0 === value1 ? null - : value0 === value00 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value1); + var string0 = this.getAttribute(name); + return string0 === string1 ? null + : string0 === string00 ? interpolate0 + : interpolate0 = interpolate(string00 = string0, value1); }; } function attrConstantNS(fullname, interpolate, value1) { - var value00, + var string00, + string1 = value1 + "", interpolate0; return function() { - var value0 = this.getAttributeNS(fullname.space, fullname.local); - return value0 === value1 ? null - : value0 === value00 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value1); + var string0 = this.getAttributeNS(fullname.space, fullname.local); + return string0 === string1 ? null + : string0 === string00 ? interpolate0 + : interpolate0 = interpolate(string00 = string0, value1); }; } function attrFunction(name, interpolate, value) { - var value00, - value10, + var string00, + string10, interpolate0; return function() { - var value0, value1 = value(this); + var string0, value1 = value(this), string1; if (value1 == null) return void this.removeAttribute(name); - value0 = this.getAttribute(name); - return value0 === value1 ? null - : value0 === value00 && value1 === value10 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value10 = value1); + string0 = this.getAttribute(name); + string1 = value1 + ""; + return string0 === string1 ? null + : string0 === string00 && string1 === string10 ? interpolate0 + : (string10 = string1, interpolate0 = interpolate(string00 = string0, value1)); }; } function attrFunctionNS(fullname, interpolate, value) { - var value00, - value10, + var string00, + string10, interpolate0; return function() { - var value0, value1 = value(this); + var string0, value1 = value(this), string1; if (value1 == null) return void this.removeAttributeNS(fullname.space, fullname.local); - value0 = this.getAttributeNS(fullname.space, fullname.local); - return value0 === value1 ? null - : value0 === value00 && value1 === value10 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value10 = value1); + string0 = this.getAttributeNS(fullname.space, fullname.local); + string1 = value1 + ""; + return string0 === string1 ? null + : string0 === string00 && string1 === string10 ? interpolate0 + : (string10 = string1, interpolate0 = interpolate(string00 = string0, value1)); }; } @@ -70,5 +74,5 @@ export default function(name, value) { return this.attrTween(name, typeof value === "function" ? (fullname.local ? attrFunctionNS : attrFunction)(fullname, i, tweenValue(this, "attr." + name, value)) : value == null ? (fullname.local ? attrRemoveNS : attrRemove)(fullname) - : (fullname.local ? attrConstantNS : attrConstant)(fullname, i, value + "")); + : (fullname.local ? attrConstantNS : attrConstant)(fullname, i, value)); } diff --git a/src/transition/style.js b/src/transition/style.js index 9c87f92..94ea5c0 100644 --- a/src/transition/style.js +++ b/src/transition/style.js @@ -4,15 +4,15 @@ import {tweenValue} from "./tween"; import interpolate from "./interpolate"; function styleRemove(name, interpolate) { - var value00, - value10, + var string00, + string10, interpolate0; return function() { - var value0 = style(this, name), - value1 = (this.style.removeProperty(name), style(this, name)); - return value0 === value1 ? null - : value0 === value00 && value1 === value10 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value10 = value1); + var string0 = style(this, name), + string1 = (this.style.removeProperty(name), style(this, name)); + return string0 === string1 ? null + : string0 === string00 && string1 === string10 ? interpolate0 + : interpolate0 = interpolate(string00 = string0, string10 = string1); }; } @@ -23,27 +23,30 @@ function styleRemoveEnd(name) { } function styleConstant(name, interpolate, value1) { - var value00, + var string00, + string1 = value1 + "", interpolate0; return function() { - var value0 = style(this, name); - return value0 === value1 ? null - : value0 === value00 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value1); + var string0 = style(this, name); + return string0 === string1 ? null + : string0 === string00 ? interpolate0 + : interpolate0 = interpolate(string00 = string0, value1); }; } function styleFunction(name, interpolate, value) { - var value00, - value10, + var string00, + string10, interpolate0; return function() { - var value0 = style(this, name), - value1 = value(this); - if (value1 == null) value1 = (this.style.removeProperty(name), style(this, name)); - return value0 === value1 ? null - : value0 === value00 && value1 === value10 ? interpolate0 - : interpolate0 = interpolate(value00 = value0, value10 = value1); + var string0 = style(this, name), + value1 = value(this), + string1 = value1 + ""; + console.log({value1}); + if (value1 == null) string1 = value1 = (this.style.removeProperty(name), style(this, name)); + return string0 === string1 ? null + : string0 === string00 && string1 === string10 ? interpolate0 + : (string10 = string1, interpolate0 = interpolate(string00 = string0, value1)); }; } @@ -54,5 +57,5 @@ export default function(name, value, priority) { .on("end.style." + name, styleRemoveEnd(name)) : this.styleTween(name, typeof value === "function" ? styleFunction(name, i, tweenValue(this, "style." + name, value)) - : styleConstant(name, i, value + ""), priority); + : styleConstant(name, i, value), priority); } diff --git a/test/transition/attr-test.js b/test/transition/attr-test.js index e1ff141..fecd5f9 100644 --- a/test/transition/attr-test.js +++ b/test/transition/attr-test.js @@ -63,6 +63,88 @@ tape("transition.attr(name, value) creates a namespaced tween to the value retur }, 125); }); +tape("transition.attr(name, constant) is a noop if the string-coerced value matches the current value on tween initialization", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).attr("foo", 1), + transition = selection.transition().attr("foo", 1); + + d3_timer.timeout(function(elapsed) { + root.setAttribute("foo", 2); + }, 125); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.getAttribute("foo"), "2"); + test.end(); + }, 250); +}); + +tape("transition.attr(ns:name, constant) is a noop if the string-coerced value matches the current value on tween initialization", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).attr("svg:foo", 1), + transition = selection.transition().attr("svg:foo", 1); + + d3_timer.timeout(function(elapsed) { + root.setAttributeNS("http://www.w3.org/2000/svg", "foo", 2); + }, 125); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.getAttributeNS("http://www.w3.org/2000/svg", "foo"), "2"); + test.end(); + }, 250); +}); + +tape("transition.attr(name, function) is a noop if the string-coerced value matches the current value on tween initialization", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).attr("foo", 1), + transition = selection.transition().attr("foo", function() { return 1; }); + + d3_timer.timeout(function(elapsed) { + root.setAttribute("foo", 2); + }, 125); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.getAttribute("foo"), "2"); + test.end(); + }, 250); +}); + +tape("transition.attr(ns:name, function) is a noop if the string-coerced value matches the current value on tween initialization", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).attr("svg:foo", 1), + transition = selection.transition().attr("svg:foo", function() { return 1; }); + + d3_timer.timeout(function(elapsed) { + root.setAttributeNS("http://www.w3.org/2000/svg", "foo", 2); + }, 125); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.getAttributeNS("http://www.w3.org/2000/svg", "foo"), "2"); + test.end(); + }, 250); +}); + +tape("transition.attr(name, constant) uses interpolateNumber if value is a number", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).attr("foo", "15px"), + transition = selection.transition().attr("foo", 10); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.getAttribute("foo"), "NaN"); + test.end(); + }, 125); +}); + +tape("transition.attr(name, function) uses interpolateNumber if value is a number", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).attr("foo", "15px"), + transition = selection.transition().attr("foo", () => 10); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.getAttribute("foo"), "NaN"); + test.end(); + }, 125); +}); + tape("transition.attr(name, value) immediately evaluates the specified function with the expected context and arguments", function(test) { var document = jsdom("

"), one = document.querySelector("#one"), diff --git a/test/transition/style-test.js b/test/transition/style-test.js index 1f750ac..ae954b5 100644 --- a/test/transition/style-test.js +++ b/test/transition/style-test.js @@ -103,6 +103,36 @@ tape("transition.style(name, value) creates an tween which removes the specified }); }); +tape("transition.style(name, constant) is a noop if the string-coerced value matches the current value on tween initialization", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).style("opacity", 1), + transition = selection.transition().style("opacity", 1); + + d3_timer.timeout(function(elapsed) { + root.style.opacity = 0.5; + }, 125); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.style.getPropertyValue("opacity"), "0.5"); + test.end(); + }, 250); +}); + +tape("transition.style(name, function) is a noop if the string-coerced value matches the current value on tween initialization", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).style("opacity", 1), + transition = selection.transition().style("opacity", function() { return 1; }); + + d3_timer.timeout(function(elapsed) { + root.style.opacity = 0.5; + }, 125); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.style.getPropertyValue("opacity"), "0.5"); + test.end(); + }, 250); +}); + tape("transition.style(name, value) interpolates numbers", function(test) { var root = jsdom().documentElement, ease = d3_ease.easeCubic, @@ -117,6 +147,28 @@ tape("transition.style(name, value) interpolates numbers", function(test) { }, 125); }); +tape("transition.style(name, constant) uses interpolateNumber if value is a number", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).style("font-size", "15px"), + transition = selection.transition().style("font-size", 10); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.style.getPropertyValue("font-size"), "NaN"); + test.end(); + }, 125); +}); + +tape("transition.style(name, function) uses interpolateNumber if value is a number", function(test) { + var root = jsdom().documentElement, + selection = d3_selection.select(root).style("font-size", "15px"), + transition = selection.transition().style("font-size", () => 10); + + d3_timer.timeout(function(elapsed) { + test.strictEqual(root.style.getPropertyValue("font-size"), "NaN"); + test.end(); + }, 125); +}); + tape("transition.style(name, value) interpolates strings", function(test) { var root = jsdom().documentElement, ease = d3_ease.easeCubic,