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

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

Closed
chrisfrancis27 opened this issue Jun 16, 2016 · 10 comments

Comments

@chrisfrancis27
Copy link

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
Copy link
Author

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

@bclinkinbeard
Copy link

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
Copy link
Member

mbostock commented Oct 8, 2016

@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
Copy link

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
Copy link
Member

mbostock commented Oct 8, 2016

@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
Copy link

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

@mbostock
Copy link
Member

mbostock commented Oct 9, 2016

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
Copy link

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
Copy link
Member

mbostock commented Oct 9, 2016

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
Copy link

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

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