fix: Dynamically compute default zoom/center for map traces#7884
fix: Dynamically compute default zoom/center for map traces#7884camdecoster wants to merge 20 commits into
Conversation
|
I think you've addressed all my previous comments. I did find one bug though: Plotly functions which update only the data don't adjust the auto-fit bounds like they (probably?) should. Here's a sample mock with 2 traces: {
"data": [
{
"hovertext": ["Montreal", "New York", "Los Angeles", "San Francisco"],
"lat": [45.5017, 40.7128, 34.0522, 37.7749],
"lon": [-73.5673, -74.006, -118.2437, -122.4194],
"marker": { "color": "#0f0" },
"mode": "markers",
"type": "scattermap"
},
{
"hovertext": ["San Marino", "Cairo", "Istanbul", "Trondheim"],
"lat": [43.9360958, 30.06263, 41.01384, 63.43049],
"lon": [12.4417702, 31.24967, 28.94966, 10.39506],
"marker": { "color": "#f00" },
"mode": "markers",
"type": "scattermap"
}
],
"layout": {
"width": 900,
"height": 600
}
}Calling Calling On the other hand, calling Plotly.react(gd, {
"hovertext": ["Montreal", "New York", "Los Angeles", "San Francisco"],
"lat": [45.5017, 40.7128, 34.0522, 37.7749],
"lon": [-73.5673, -74.006, -118.2437, -122.4194],
"marker": { "color": "#0f0" },
"mode": "markers",
"type": "scattermap"
})works as expected (updates data, and updates auto-bounds, which makes sense because no center/zoom settings are passed in the layout). I guess you could make an argument that this is expected behavior (functions which only update data should not affect the layout) but I'm not sure it's what we want. Do we have any other layout values which determine their defaults based on values in the data? If so, what do we do in those cases? I'll sleep on it. Relatedly, could you add an end-to-end Jasmine test which creates a new scattermap plot and then checks the zoom and center of the resulting map object? And several more which then call the various Plotly functions on that plot, and check that the resulting zoom and center match whatever we decide the correct behavior to be? Finally, I almost hate to bring it up, but... it's probably worth testing whether the auto-bounds behave as expected with respect to |
|
Dug into the Re: your question about other layout values that key off the data, If we do want Since this is going into v4.0, do you want the full re-fit behavior in this PR, or should we ship what we have (auto-fit on initial plot + I'll add the Jasmine tests for thanks. |
|
@palmerusaf , can you please share the data you used here. i need to reproduce this to find the exact error. |
|
@palmerusaf that case should be handled. Have you tested your example with the latest changes? From everything that I've tried with coords that span the antimeridian, the fit works as it should. |
|
@emilykl |
|
@camdecoster Just tested looks good now. I just tested through the test dashboard using the following mock data. {
"data": [
{
"hovertext": [
"P1",
"P2",
"P3",
"P4"
],
"lat": [
43.9360958,
30.06263,
41.01384,
63.43049
],
"legendgroup": "",
"lon": [
170,
180,
-180,
-170
],
"marker": {
"color": "#636efa"
},
"mode": "markers",
"type": "scattermap"
}
]
} |
|
Great. Thanks for checking. |
| handleItemDefaults: handleLayerDefaults | ||
| }); | ||
|
|
||
| // Explicitly assign `_fitBounds` (even when null) so `relinkPrivateKeys` |
There was a problem hiding this comment.
| // Explicitly assign `_fitBounds` (even when null) so `relinkPrivateKeys` | |
| // Explicitly reset `_fitBounds` to null so that `relinkPrivateKeys` |
| handleItemDefaults: handleLayerDefaults | ||
| }); | ||
|
|
||
| // Explicitly assign `_fitBounds` (even when null) so `relinkPrivateKeys` |
There was a problem hiding this comment.
Can you add a comment here explaining the meanings and usage of _fitBounds vs. _fitView ?


Description
Use lat/lon points to dynamically compute the default zoom/center for
scattermapanddensitymaptraces.Closes #7674.
Changes
handleDefaultsto dynamically computer map boundsScreenshots
Testing
{ "data": [ { "hovertext": ["San Marino", "Cairo", "Istanbul", "Trondheim"], "lat": [43.9360958, 30.06263, 41.01384, 63.43049], "lon": [12.4417702, 31.24967, 28.94966, 10.39506], "marker": { "color": "#f00" }, "mode": "markers", "type": "scattermap" } ], "layout": { "width": 900, "height": 600 } }Notes