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

React + uirevision fixes #3394

Merged
merged 4 commits into from
Jan 4, 2019
Merged

React + uirevision fixes #3394

merged 4 commits into from
Jan 4, 2019

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Jan 3, 2019

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 fill gd._fullLayout._preGUI and get uirevision 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

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

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?

Copy link
Contributor Author

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

Copy link
Collaborator

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];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice simplification!

@alexcjohnson
Copy link
Collaborator

Beautiful, thanks for cleaning all this up! 💃

@etpinard etpinard merged commit 364cdec into master Jan 4, 2019
@etpinard etpinard deleted the uirevision-fixes branch January 4, 2019 15:17
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.

uirevision + 3D view changes
2 participants