d3 / d3-transition

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

selection.transition(transitionInstance) doesn't inherit timing properties #45

Closed chrisfrancis27 closed 8 years ago

chrisfrancis27 commented 8 years ago

From the selection.transition([name]) docs:

If the name is a transition instance, the returned transition has the same id and name as the specified transition. If a transition with the same id already exists on a selected element, the existing transition is returned for that element. Otherwise, the timing of the returned transition is inherited from the existing transition of the same id on the nearest ancestor of each selected element.

And the example given:

var t = d3.transition()
    .duration(750)
    .ease(d3.easeLinear);

d3.selectAll(".apple").transition(t)
    .style("fill", "red");

d3.selectAll(".orange").transition(t)
    .style("fill", "orange");

My understanding is that the transitions applied to ".orange" and ".apple" elements should inherit the 750ms duration and linear easing from transition t. However, they appear use the d3 default transition properties instead.

See this Fiddle for example.

Am I misunderstanding how to use transition inheritance, or are the docs incorrect?

chrisfrancis27 commented 8 years ago

Actually, as soon as I posted this I went on the hunt for source issue and it seems like this was addressed in https://github.com/d3/d3-transition/issues/40 but those changes are in v4-alpha! I'm still on 3.5.17... Sorry for the noise - closing this.

bclinkinbeard commented 7 years ago

This still seems to be an issue in 4.2.3, and I can't understand the behavior I'm seeing. Basically, it seems like the duration is only respected if the transition instance is created and used in the same frame or something. I created a JSBin of the code from the docs referenced above, and a similar example JSBin here. In both examples, the duration is ignored if the transition is created outside the function in which it is used.

mbostock commented 7 years ago

@bclinkinbeard Transitions are destroyed after they end, so you can’t inherit a transition that’s ended. Thus if you call changed within the first five seconds of page load, it will inherit the five-second duration, but it won’t if you call it after—it’ll fallback to the default duration because the specified transition isn’t found on an ancestor element.

Arguably the bug in that if the specified transition-to-inherit-from is not found, it should throw an Error rather than falling back to the default parameters. That behavior isn’t documented, so it might be reasonable to change it, though I expect some people might be annoyed that there could which worked (albeit incorrectly) is now throwing an error…

bclinkinbeard commented 7 years ago

Ah, OK, I at least understand it now. I read the docs several times and couldn't really wrap my head around the ancestor/document element stuff.

I guess there is no way to create a reusable transition instance then, because there is no way to create an inert transition? That is what I initially thought transition(t) was doing. Is a factory the best (only?) way to create reusable transitions?

mbostock commented 7 years ago

@bclinkinbeard The primary unit of reusability is the function. You can use transition.call to call a function that sets the desired properties on the transition:

function configure(transition) {
  return transition
      .duration(5000)
      .ease(d3.easeLinear);
}

d3.select(document.body)
  .transition()
    .call(configure)
    .style("background-color", "red");

Or perhaps:

function slowTransition(selection) {
  return selection.transition()
      .duration(5000)
      .ease(d3.easeLinear);
}

slowTransition(d3.select(document.body))
    .style("background-color", "red");
bclinkinbeard commented 7 years ago

Why doesn't .call() seem to work?

mbostock commented 7 years ago

Please be more specific and include a link to an example that demonstrates the issue, preferably on bl.ocks.org. I tested the above examples and they worked fine, and there are unit tests in this repository for transition.call (and likewise selection.call in d3-selection). So just saying something doesn’t work without being specific is unproductive.

bclinkinbeard commented 7 years ago

Sorry, typing on my phone. Your first example is what I meant by factory so that's 👍. Just curious why call wouldn't work since I thought it can do anything with the selection passed in.

mbostock commented 7 years ago

Not sure what you mean; transition.call does work, as shown in my example. Using selection.call would also work, but it always returns the current selection (rather than the return value of the called function), so it can’t easily be used to instantiate a transition and return it for further customization. I’d change the behavior of selection.call to return the return value of the called function but it’d have to wait for D3 5.0.

bclinkinbeard commented 7 years ago

OK, all clear now. Thanks and sorry again for being vague/short. I should know better than trying to edit and grok JSBins on my phone while out and about. :)