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

Axis autorange on trailing transition redraws #4252

Closed
nicolaskruchten opened this issue Oct 3, 2019 · 9 comments
Closed

Axis autorange on trailing transition redraws #4252

nicolaskruchten opened this issue Oct 3, 2019 · 9 comments

Comments

@nicolaskruchten
Copy link
Member

nicolaskruchten commented Oct 3, 2019

Here is a pair of CodePens showing the same figure in two versions with strangely-different behaviour:

In the most recent one, the bars slide, which is good, but then the y-axis range also jumps, which it didn't do before. Note also that after the animation is done playing, the y-axis range does not fully contain the bar: double-clicking on the plot expands the y-axis range.

@antoinerg
Copy link
Contributor

For the version on master to behave like 1.49.5 with regards to y-axis range, one can set redraw: false. I am a bit surprised by the change. 🤔

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

We get similar results using type: 'scatter' traces:

1.49.5: https://codepen.io/etpinard/pen/rNNawmR
1.50.0: https://codepen.io/etpinard/pen/qBBEjrE

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

Here's the problem (again, nothing in bar/plot.js mades the behaviour worse - cc @antoinerg ):

plotly.js/src/plots/plots.js

Lines 2728 to 2739 in bead43f

return Promise.resolve().then(function() {
if(opts.redraw) {
return Registry.call('redraw', gd);
}
}).then(function() {
// Set transitioning false again once the redraw has occurred. This is used, for example,
// to prevent the trailing redraw from autoranging:
gd._transitioning = false;
gd._transitioningWithDuration = false;
gd.emit('plotly_transitioned', []);
}).then(callback);

so during the trailing redraw the axis doAutoRange routine is skipped via the early return in:

function doAutoRangeAndConstraints() {
if(gd._transitioning) return;
subroutines.doAutoRangeAndConstraints(gd);
// store initial ranges *after* enforcing constraints, otherwise
// we will never look like we're at the initial ranges
if(graphWasEmpty) Axes.saveRangeInitial(gd);
// this one is different from shapes/annotations calcAutorange
// the others incorporate those components into ax._extremes,
// this one actually sets the ranges in rangesliders.
Registry.getComponentMethod('rangeslider', 'calcAutorange')(gd);
}

Only after the trailing redraw does Boolean(gd._transitioning) // => false so that subsequent redraws (e.g. via double-click) get the new auto-range.

So, unfortunately the current Plotly.animate pipeline doesn't allow animating frames with different auto-range values. At the moment, it's probably best to "always" set the axis ranges when animating.


Now, how to fix this thing? Adding

                gd._transitioning = false;
                gd._transitioningWithDuration = false;

to

plotly.js/src/plots/plots.js

Lines 2729 to 2731 in bead43f

if(opts.redraw) {
return Registry.call('redraw', gd);
}

before the redraw call appears to make things "work" - but the result isn't "great": the trace positions transition smoothly and then the axes "jump" to the new auto-range (this is essentially the same problem as #4251 & #1687).

By adding these lines, no current test is failing, but we'll need to spend the time to dig deeper. I don't believe Ricky wrote:

            // Set transitioning false again once the redraw has occurred. This is used, for example,
            // to prevent the trailing redraw from autoranging:

for nothing.

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

Can I suggest renaming this issue: Axis autorange on trailing transition redraws

@nicolaskruchten
Copy link
Member Author

So what changed between 1.49.5 and 1.50.0? And is that change an improvement?

@nicolaskruchten
Copy link
Member Author

(I definitely think we should rename the issue but I can't tell if the issue is "something changed for the worse" or "something changed halfway for the better and we need to finish the swing" :)

@etpinard
Copy link
Contributor

etpinard commented Oct 8, 2019

So what changed between 1.49.5 and 1.50.0?

The bars now smoothly transition.

And is that change an improvement?

Not sure how to answer that. Making bar smoothly transition is a definite improvement, but w/o setting fixed range, the behaviour is arguably more confusing in 1.50.0. But note that even in 1.49.5 the y-axis range does not properly update at the end of the transition. So, the Axis autorange on trailing transition redraws issue applies to both 1.49.5 and 1.50.0 and has probably been part of plotly.js even since animations got introduced.

By setting the y-axis range, things look pretty good in 1.50.0: https://codepen.io/etpinard/pen/xxxbBzx

@etpinard etpinard changed the title Bar animation oddities Axis autorange on trailing transition redraws Oct 8, 2019
@nicolaskruchten
Copy link
Member Author

oooh I see what you're saying. Your two scatter CodePens show the same behaviour as each other and as the 1.50 bar behaviour. I thought you were saying the same change had occurred in scatter as in bar.

Things do looks great with a fixed axis range, no doubt, I just worry that most people won't do that and just expect some automation.

Re your proposed fix and

but the result isn't "great": the trace positions transition smoothly and then the axes "jump" to the new auto-range (this is essentially the same problem as #4251 & #1687).

I think the current behaviour is pretty troubling, so even if the fix results in "not great" we should try implementing it, unless we think we can sort out #1687 soon (which still seems unlikely?). It seems like you have an independent fix for #4251, which is nice.

@gvwilson
Copy link
Contributor

Hi - we are currently trying to tidy up Plotly's public repositories to help us focus our efforts on things that will help users most. Since this issue has been sitting for several years, I'm going to close it. If it's still a concern, we'd be grateful if you could open a new issue (with a short reproducible example if appropriate) so that we can add it to our backlog. Thanks for your help - @gvwilson

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

No branches or pull requests

4 participants