search

d3 / d3-transition

Animated transitions for D3 selections.
https://d3js.org/d3-transition
ISC License
224 stars 65 forks source link

strike interpolateNumber from transition docs #79

Closed gordonwoodhull closed 5 years ago

gordonwoodhull commented 6 years ago

per #67, it looks like interpolateNumber is never invoked, because all values are coerced to string first.

The proposal is to bring the documentation in sync with the implementation.

If this proposal is correct, then the first case could probably be dropped from src/transition/interpolate.js

Background: I want to understand why the behavior of some transitions changed when porting dc.js from d3v3 to d3v4. Previously if an attribute was not set on enter but was set in the transition, it took the final value with no animation. Now it starts from zero.

It's our bug and we need to fix it. We were relying on undefined behavior and it's better to be explicit about where you transition from. But, I still want to understand what changed.

I found this SO question: https://stackoverflow.com/questions/47339762/what-attributes-does-d3-transition-change

Which referenced this comment: https://github.com/d3/d3-transition/pull/67#issuecomment-345290841

I'm not sure if this change in implementation explains the change in observed behavior, but as noted, it's not a real problem.

(edited for clarity)

mbostock commented 5 years ago

As it turns out, #67 introduced an inconsistency. If you call transition.attr or transition.style with a constant value, it is coerced to a string, but not if you pass a function value. So for example, this uses interpolateString because the 0 is coerced to "0":

d3.select("body").transition().style("opacity", 0);

But this uses interpolate number because no coercion happens:

d3.select("body").transition().style("opacity", () => 0);

The short-circuit on same-value comparison in transitions uses strict equality, and so we definitely want to short-circuit transitions where the target value is specified as a number that matches the current value. Meaning, #67 only fixed the constant value case, not the function value case.

But, I think it’s also desirable that if the target value is specified as a number, we use interpolateNumber. Mainly this is for performance and simplicity: interpolateString has to do a lot more work, and if we know that the target value is a number, then interpolateNumber should be equivalent (in the cases that matter) and faster. Moreover, that’s the documented behavior, at least prior to this PR. 😉

Therefore closing in favor for #87 #88.

(See #49 for a related issue where transition.style is passed a function.)

gordonwoodhull commented 5 years ago

Just saw this. Thanks @mbostock, this makes much more sense.