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

Transitions should use computed value for end, too? #47

Closed
wlodzislav opened this issue Jul 14, 2016 · 9 comments
Closed

Transitions should use computed value for end, too? #47

wlodzislav opened this issue Jul 14, 2016 · 9 comments
Assignees

Comments

@wlodzislav
Copy link

wlodzislav commented Jul 14, 2016

When transition 'left' or 'right' style from negative % to positive % sets wrong values. When do the same for 'top' or 'bottom' it sets she same value all the animation frames.

Code to reproduce:

<div style="position: absolute"></div>
var div = d3.select("div");
div
    .style("left", "-50%")
    .transition().duration(1000)
    .style("left", "50%")
    .tween("log", function () {
        return function (t) {
            console.log(div.style("left")); // will be something like -5470.53px to 523px
            console.log(div.node().style.left); // will be something like -522.996% to 50%
        };
    });

I used https://d3js.org/d3.v4.js for testing.

@curran
Copy link
Contributor

curran commented Jul 14, 2016

@wlodzislav
Copy link
Author

Bug reproduces in Chrome 51 and Firefox 47 but not in Safary.

@mbostock
Copy link
Member

mbostock commented Jul 14, 2016

This is the expected behavior because transition.style uses the computed value as the starting value (hence converting percentage to pixels), then followed by d3.interpolateString.

You can avoid this by using transition.styleTween and specifying the start value explicitly by returning an interpolator that both starts and ends with percentages.

I suppose an alternative solution would be for D3 to set the target value temporarily, compute the resulting computed value, and then use that for the interpolator. But that is a fairly significant change and also means that the resulting assigned style property value will be different than what was passed to transition.style when the transition ends. So I am inclined to close this as designed.

@wlodzislav
Copy link
Author

wlodzislav commented Jul 14, 2016

I'm not sure, note that in my case div.style("left") is -4597.61px, but window.getComputedStyle(document.querySelector("div")).left when left is set to -50% is -480px

@mbostock
Copy link
Member

mbostock commented Jul 14, 2016

Yes. That is because div.style("left") is again returning the computed value.

@mbostock mbostock self-assigned this Jul 14, 2016
@mbostock
Copy link
Member

mbostock commented Jul 14, 2016

We already set the target value temporarily in the case of transition.style(name, null) so that we know how to transition when removing a style. I suppose it wouldn’t be too much of a stretch to temporarily set the style in all cases, so that transition.style uses the computed value for both the start and end. (Related bug #49.)

I’m a little worried that this will be detrimental to performance, especially since a naïve implementation would interleave reading and writing to the DOM and waste effort. But perhaps there’s a simple four-pass approach where, on all starting elements, we first read the starting computing values, then temporarily set the style to the target value, then read the ending computed values, and then finally restore to the starting value.

@mbostock mbostock changed the title Wrong interpolation with transition form -50% to 50% for absolute positioned element Transitions should use computed value for end, too? Jul 14, 2016
@wlodzislav
Copy link
Author

I don't think I get it, why not just interpolate value from node.style.left if it's presented to the end value without computing and converting to px? I think it's common situation when inline style is already set or set just before the transition.

@mbostock
Copy link
Member

mbostock commented Jul 15, 2016 via email

@mbostock
Copy link
Member

mbostock commented Mar 8, 2017

I think it would be reasonable for style transitions to use the inline value if present as the starting value, and only fallback to the computed value if the inline value is not present. That way it’s a lot easier to control the starting value of a transition, and by extension the interpolator. Also, it’s more consistent with how attribute transitions work. And it’s faster!

mbostock added a commit to d3/d3-selection that referenced this issue Mar 8, 2017
Rather than always returning the computed style value, selection.style now
favors the inline value, if present, and only falls back to the computed value
if an inline value is not present.

By not computing the style value in the common case where an inline style is
present, this is faster, but more importantly it is more predictable, since a
computed value can be surprisingly different from an inline value, such as when
the inline value is specified as a percentage.

This change is especially useful for transition.style, d3/d3-transition#47, so
that you can control the starting style value of a transition.
mbostock added a commit that referenced this issue Mar 8, 2017
Rather than always starting from the computed style value, transition.style now
favors the inline value, if present, and only falls back to the computed value
if an inline value is not present.

By not computing the style value in the common case where an inline style is
present, this is faster, but more importantly it is more predictable since a
computed value can be surprisingly different from an inline value, such as when
the inline value is specified as a percentage.

Related d3/d3-selection#120. Fixes #47.
mbostock added a commit to d3/d3-selection that referenced this issue May 15, 2017
Rather than always returning the computed style value, selection.style now
favors the inline value, if present, and only falls back to the computed value
if an inline value is not present.

By not computing the style value in the common case where an inline style is
present, this is faster, but more importantly it is more predictable, since a
computed value can be surprisingly different from an inline value, such as when
the inline value is specified as a percentage.

This change is especially useful for transition.style, d3/d3-transition#47, so
that you can control the starting style value of a transition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants