Skip to content

Choropleth maps don't reset accurately when layout.geo.scope is changed #3831

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

Closed
mbkupfer opened this issue May 3, 2019 · 6 comments
Closed
Assignees
Labels
bug something broken

Comments

@mbkupfer
Copy link

mbkupfer commented May 3, 2019

First and foremost, I thought I would mention that this issue is only occurring with Plotly.react as I was able to implement a correct alternative by using Plotly.newPlot but unfortunately means that I won't be able to use react-plotly for this component.

With that out of the way, the bug occurs when the layout scope is changed and either 1. a double click event is triggered or 2. reset is pressed in the mode bar. When the plot is changed through the dropdown though, there is no issue. All examples below can be reproduced in this code pen.

In cases where scope is not 'usa', the reset changed the lon/lat values to whatever the initial plot's geo scope was set at. This is very pronounced when switching from 'world' to 'north america', thus I chose that for the example, but this occurs in any situation. If the initial plot is set to 'north america', then it would be the reverse.

In cases where the plot is changed to 'usa' though, the js actually throws an error! The error is:
TypeError: centerPx is null.

So this is actually two errors, but they both seem to have the same root cause.

Let me know if I can provide any more clarity.

@mbkupfer
Copy link
Author

mbkupfer commented May 3, 2019

Here is a gif that illustrates what I'm talking about.
Plotly choropleth bug

@etpinard
Copy link
Contributor

etpinard commented May 3, 2019

Thanks very much for the report!

Something must be up with

proto.saveViewInitial = function(geoLayout) {
var center = geoLayout.center || {};
var projLayout = geoLayout.projection;
var rotation = projLayout.rotation || {};
if(geoLayout._isScoped) {
this.viewInitial = {
'center.lon': center.lon,
'center.lat': center.lat,
'projection.scale': projLayout.scale
};
} else if(geoLayout._isClipped) {
this.viewInitial = {
'projection.scale': projLayout.scale,
'projection.rotation.lon': rotation.lon,
'projection.rotation.lat': rotation.lat
};
} else {
this.viewInitial = {
'center.lon': center.lon,
'center.lat': center.lat,
'projection.scale': projLayout.scale,
'projection.rotation.lon': rotation.lon
};
}
};

where we should probably clear the thing on update call involving geo.scope

@etpinard etpinard added the bug something broken label May 3, 2019
@mbkupfer
Copy link
Author

mbkupfer commented May 7, 2019

@etpinard, I'm not sure how far along you are in this, but I spent some time inspecting geo.js and it seems like we can mitigate by resetting viewInitial in the conditional below.

I've also been trying to level up my js, so I wouldn't mind submitting a PR if this hasn't made it down your pipeline yet. Let me know.

if(_this.topojson === null || topojsonNameNew !== _this.topojsonName) {

@etpinard
Copy link
Contributor

etpinard commented May 9, 2019

@mbkupfer We'll have a fix for this bug merged before we release 1.48.0

Here's your codepen demo using a "fixed" bundle: https://codepen.io/etpinard/pen/XwXqrN?editors=1010

Thanks again for using (and potentially contributing to 😏 ) plotly.js!

@mbkupfer
Copy link
Author

Awesome, thanks for fixing this so quickly!

Thanks for making plotly such a great library.

@etpinard
Copy link
Contributor

No problem! Thanks for the very clear bug report!

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

No branches or pull requests

2 participants