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

Bar transition oddities #4251

Closed
nicolaskruchten opened this issue Oct 3, 2019 · 12 comments · Fixed by #4262
Closed

Bar transition oddities #4251

nicolaskruchten opened this issue Oct 3, 2019 · 12 comments · Fixed by #4262
Assignees

Comments

@nicolaskruchten
Copy link
Member

In this pen which is in "layout first" transition ordering, there seems to be an extra bar-height jump: https://codepen.io/nicolaskruchten/pen/WNNeZgz?editors=0010

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

Peek 2019-10-07 14-27

Hmm. Interesting, scatter seems to behave the same: https://codepen.io/etpinard/pen/dyyPvvZ?editors=0010

Peek 2019-10-07 14-28

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

... and by the way, transition.ordering: 'layout first' is the default:

ordering: {
valType: 'enumerated',
values: ['layout first', 'traces first'],
dflt: 'layout first',
role: 'info',
editType: 'none',
description: [
'Determines whether the figure\'s layout or traces smoothly transitions',
'during updates that make both traces and layout change.'
].join(' ')
}

@nicolaskruchten
Copy link
Member Author

Yep, I know, I was just calling attention to it and making it easier to change to the non-default options in the pen ;)

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

After some investigations, it doesn't look like bar/plot.js (cc @antoinerg ) is doing anything that results in any discrepancies between bar and scatter transitions.

Layout + traces transitions don't look very good at the moment, but they don't look any worse than what was proposed in #3217

So, this ticket is essentially a duplicate of #1687

@nicolaskruchten
Copy link
Member Author

nicolaskruchten commented Oct 7, 2019

I agree in principle, although solving the double-jump issue seems easier than solving #1687 altogether... what does that first jump represent? The middle point/bar jumps from 4 to 5, then the yaxis range smoothly transitions along with the point/bar, then the point/bar jumps from 5 to 6. I would expect that under layout-first the first jump wouldn't happen, and the point/bar would jump from 4 to 6... ?

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

although solving the double-jump issue seems easier than solving

Double jump? In the gif above I can only detect one "jump"

what does that first jump represent?

Here's the current logic under transition.ordering: 'layout first':

plotly.js/src/plots/plots.js

Lines 2617 to 2621 in bead43f

axisTransitionOpts = transitionOpts;
transitionedTraces = null;
traceTransitionOpts = Lib.extendFlat({}, transitionOpts, {duration: 0});
transitionAxes();
transitionTraces();

  • We first transition the y-axis range from ~ [3,5] to ~ [3,6] while not updating the trace position
  • Then, we update the trace position instantaneously from y=[3,4,5] to y=[3,6,5] (aka the "jump")

@nicolaskruchten
Copy link
Member Author

You don’t see the immediate jump 4-5, then slide, then jump 5-6?

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

... so w/o the "jump" at the end of the transition we'd get: the correct y-axis range of ~ [3,6] but the trace would have coordinates y=[3,4,5].

@nicolaskruchten
Copy link
Member Author

Yeah the jump at the end is fine. This issue is about the first jump :)

@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

Ah now I see it, there's a race condition in:

plotly.js/src/plots/plots.js

Lines 2617 to 2621 in bead43f

axisTransitionOpts = transitionOpts;
transitionedTraces = null;
traceTransitionOpts = Lib.extendFlat({}, transitionOpts, {duration: 0});
transitionAxes();
transitionTraces();

Sorry for the confusion!

Codepen attempt: https://codepen.io/etpinard/pen/oNNgGem?editors=0010

@etpinard etpinard self-assigned this Oct 7, 2019
@etpinard
Copy link
Contributor

etpinard commented Oct 7, 2019

@nicolaskruchten
Copy link
Member Author

phew! Those pens look like what I would expect, yep!

etpinard added a commit that referenced this issue Oct 8, 2019
etpinard added a commit that referenced this issue Oct 8, 2019
... to remove "jump" caused by transitionTraces()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants