Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strike interpolateNumber from transition docs #79

Conversation

gordonwoodhull
Copy link

@gordonwoodhull gordonwoodhull commented Apr 10, 2018

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: #67 (comment)

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)

@gordonwoodhull gordonwoodhull force-pushed the strike-interpolateNumber-from-docs branch from be2d428 to 47d3b03 Compare April 10, 2018 07:55
@mbostock
Copy link
Member

mbostock commented Jan 21, 2019

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.)

@mbostock mbostock closed this Jan 21, 2019
@gordonwoodhull gordonwoodhull deleted the strike-interpolateNumber-from-docs branch June 5, 2019 16:13
@gordonwoodhull
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants