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

fix uirevision autorange bug #6046

Merged
merged 2 commits into from Dec 2, 2021
Merged

fix uirevision autorange bug #6046

merged 2 commits into from Dec 2, 2021

Conversation

alexcjohnson
Copy link
Contributor

@archmoj as promised, here's my fix for the uirevision autorange issue. Unfortunately this didn't allow me to remove any of the other special handling we already had in the uirevision code for other autorange cases, so it's getting complicated. But I think the tests are fairly comprehensive.

@alexcjohnson alexcjohnson added this to the v2.7.0 milestone Nov 29, 2021
@alexcjohnson
Copy link
Contributor Author

@archmoj weird flaky CI failure, have you seen this before? https://app.circleci.com/pipelines/github/plotly/plotly.js/6026/workflows/967ed402-5fa4-4798-a886-4fdc0b2ab842/jobs/141674

  An error was thrown in afterAll
  Uncaught Error: Cannot find module '/home/circleci/plotly.js/test/jasmine/tests/box_test.js'

(and a bunch more of the same with other files)

It fixed itself with the second commit so not blocking, just strange and I wanted to flag it in case this has become a common occurrence.

var preAuto = layoutPreGUI[head + '.autorange'];
if(preAuto || (preAuto === null && pre0 === null && pre1 === null)) {
// Only read the input layout once and stash the result,
// so we get it before we start modifying it
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one took me a while to figure out... thorough testing pays dividends: prior to this stashing, everything worked except react to STAY implicit, which I would have thought would be an obvious noop.

@archmoj
Copy link
Contributor

archmoj commented Dec 1, 2021

@archmoj weird flaky CI failure, have you seen this before? https://app.circleci.com/pipelines/github/plotly/plotly.js/6026/workflows/967ed402-5fa4-4798-a886-4fdc0b2ab842/jobs/141674

  An error was thrown in afterAll
  Uncaught Error: Cannot find module '/home/circleci/plotly.js/test/jasmine/tests/box_test.js'

(and a bunch more of the same with other files)

It fixed itself with the second commit so not blocking, just strange and I wanted to flag it in case this has become a common occurrence.

I think this could be related to jasmine containers available memory.

@archmoj
Copy link
Contributor

archmoj commented Dec 1, 2021

Beautiful
💃

@alexcjohnson alexcjohnson merged commit 7d3b93e into master Dec 2, 2021
@alexcjohnson alexcjohnson deleted the uirevision-autorange-fix branch December 2, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants