-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
React + uirevision fixes #3394
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
React + uirevision fixes #3394
Conversation
... and remove `self.opts` use `self.gd._fullLayout[self.id]` instead, which always points to the correct container even on redraw-less updates
} | ||
|
||
// mocking panning/scrolling is brittle, | ||
// this here is enough to to trigger the relayoutCallback |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this presumably doesn't actually change camera.eye
, right? Would it be possible to do something that changes the eye
, even if it's not robust what it changes it to? Just "not the default", and verify that after another _react
it's again "not the default"? Or is it so brittle we can't even ensure it will register anything at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this presumably doesn't actually change
camera.eye
, right?
Yes, you were correct.
Would it be possible to do something that changes the
eye
, even if it's not robust what it changes it to? Just "not the default", and verify that after another_react
it's again "not the default"?
Good call. Here's my attempt -> b880717
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah clever! I'll take it :)
@@ -32,9 +32,6 @@ function Mapbox(opts) { | |||
// unique id for this Mapbox instance | |||
this.uid = fullLayout._uid + '-' + this.id; | |||
|
|||
// full mapbox options (N.B. needs to be updated on every updates) | |||
this.opts = fullLayout[this.id]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice simplification!
Beautiful, thanks for cleaning all this up! 💃 |
fixes #3378 - that is
uirevision
is now functional with subplots that do not call_guilRelayout
at interactions' end (i.e. gl3d, geo and mapbox).These subplots have to use
_storeDirectGUIEdit
to fillgd._fullLayout._preGUI
and getuirevision
to work correctly. On master,_preGUI
was filled with the "new" values instead of the "old" (i.e. "pre-interaction") values that the restyle/relayout/update/react logic expects. In brief, fixing this bug was mostly just a matter of reordering things.cc @plotly/plotly_js @alexcjohnson