-
-
Notifications
You must be signed in to change notification settings - Fork 2k
chore: Update geo trace fitbounds default to 'locations' #7895
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
base: v4.0
Are you sure you want to change the base?
Changes from all commits
35e356e
2d1a490
03c1e43
a8742c8
9894fdb
82acb12
0f9af80
02b0c92
fe45f65
4f6f122
c2b23ea
93055f0
5aa17cf
195345c
f04bc14
e470048
5feb26a
16254c7
122cdd4
7dd5c74
087b342
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| - **Breaking:** Change `layout.geo.fitbounds` default from `false` to `'locations'` [[#7895](https://github.com/plotly/plotly.js/pull/7895)] | ||
| - `geo` subplots will now auto-fit the initial view to the trace data | ||
| - Set `fitbounds: false` explicitly to opt out |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again not totally clear to me how this centerpoint was chosen. I mean, there's no reason the center has to be the Europe-centric map we're used to, although that does have the advantage of splitting fewer countries in half.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my comment here. |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem correct
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my comment here. |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is due to disallowing |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Green dotted line is cut off the top. Maybe this is an existing limitation with
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's an existing issue and it's on my list to work on in a follow up. |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The top plot doesn't look right, why is the orange line forced to wrap off the right edge?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is happening because the smallest gap that contains the points (not the line) is what's shown. Adding the lines to the fit check is something I'm looking into. |
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does fitbounds choose a center when the geoJSON wraps all the way around the globe?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It creates one big bounding box around all of the geometry and finds the smallest gap that contains that box. We use |
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.
This also seems wrong
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.
I disallowed a few projections because the
fitboundsmath can lead to undesirable results. In this case, the result looks okay, but now it's not usingfitboundsdue to the disallow list. For now, I think we should keep the disallow list and I can follow up with a more nuanced approach after v4.