fix: Dynamically compute default zoom/center for map traces

[camdecoster]

Description

Use lat/lon points to dynamically compute the default zoom/center for scattermap and densitymap traces.

Closes #7674.

Changes

• Add check during handleDefaults to dynamically computer map bounds
• Add tests
• Linting/formatting

Screenshots

Before	After

Testing

• Be on master
• Run the following mock through Plotly devtools

  {
    "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 }
  }

• Note that the Trondheim point is outside of the frame
• Switch to this branch
• Run the same mock
• Note that the scatter points are nicely framed
• Pan/zoom around
• Click the reset view button
• Note that the view is still nicely framed

Notes

• This PR is a combination of work done by me, @palmerusaf (Dynamic map defaults #7692), and @DhruvGarg111 (feat: Add symlog scale, scattermap zoom fix, and line dash support #7718)
• This is landing in v4.0, so it will be on by default. Users that don't want auto-framing should specify a center and/or zoom.
• The first commit handles linting/formatting for a couple of files. That commit can be ignored during review.

[emilykl]

Should skip this step if bounds is defined.

Also, I'm not positive, but I think we should test containerOut rather than containerIn, so that if the user supplies an invalid center or zoom value, we ignore it and compute the auto-bounds anyway.

Edit: Never mind, I misunderstood the bounds param. I do think we should test on containerOut though.

Suggested change

	if (containerIn.center === undefined && containerIn.zoom === undefined) {
	if (containerOut.center === undefined && containerOut.zoom === undefined) {

[camdecoster]

If we test containerOut, center/zoom will always be set because coerce supplies default values for those attributes. If containerIn doesn't have center/zoom, that's a signal that we need to provide bounds for the map. If the user supplies junk, this will still work because the call to getMapFitBounds won't run and coerce already took care of the junk values.

[emilykl]

Oh yeah of course we can't use containerOut, that makes sense.

But that does mean auto-bounds won't be applied if the user supplies junk values for center and zoom, right?

(That seems like the wrong behavior to me, but I could be convinced otherwise.)

[camdecoster]

I'm deferring to the user in this case. If they explicitly set values that would otherwise be set by auto-fitting, I opted to skip auto-fitting. If you want to provide garbage values, that's up to you. We could update that behavior in the future, but I think we should trust the user for now.

[emilykl]

That's fine, I guess I'm drawing an analogy to the normal coerce behavior where if a value is invalid we replace it with the default, but I don't think we always do that in the case of dynamic defaults. I think either behavior is defensible.

[emilykl]

This will change the center and zoom values stored in fullLayout.map, right? I'm not totally sure it's OK to modify fullLayout at this point.

[DhruvGarg111]

modifying fullLayout at this point , This is actually the same pattern used a few lines below in the moveend
handler, where user pan/zoom writes back to opts._input.center , opts._input.zoom , etc. We need to do it here because MapLibre resolves the bounding box into a concrete center/zoom internally , if we don't capture those values, subsequent updateLayout calls would reset the view to the schema defaults (center 0,0 / zoom 1) instead of the auto-framed position.

please correct me if i am wrong.

[camdecoster]

It's okay because that's the pattern we're already using in moveend down below. Since we're already mutating these values during pan/zoom, this seemed like an acceptable way to save state.

[emilykl]

It gives me the heebie-jeebies slightly but you're right that's what's happening already in moveend, so I can't come up with a good argument against it!

[emilykl]

I don't love this, since in theory viewInitial is already being set in the plot() function and this feels like an end run around that logic and ripe for bugs. But I'm open to arguments about why this is the best/only way to handle viewInitial in the auto-bounds case.

[DhruvGarg111]

The issue is timing: viewInitial normally gets set in
plot() before the map's load event fires, at which point opts.center and opts.zoom still hold the schema
defaults since MapLibre hasn't resolved the bounds yet. Without this override, double-clicking "reset view" would
snap to center=(0,0), zoom=1 instead of the auto-framed view the user actually saw. Moving the viewInitial
assignment into the load callback for all cases would be cleaner but felt like too big a refactor for this PR -
happy to hear if you see a better approach though!

[camdecoster]

This update is necessary to make the Reset modebar button and map double click callbacks use the correct values. This seemed like a reasonable place to update the data, but let me explore reworking the plot function.

[emilykl]

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 Plotly.restyle(gd, {"lat": [[45.5017]]}, [0]) removes the latter 3 cities from the first trace, but doesn't update the auto-bounds.

Calling Plotly.deleteTraces(gd, 1) deletes the second trace, but doesn't update the auto-bounds.

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 layout.uirevision.

[DhruvGarg111]

Dug into the restyle/deleteTraces behavior. The auto-fit only runs in handleDefaults, and only when center and zoom are both undefined. The problem is that once the map renders, saveViewToLayout writes the computed center/zoom back onto _input (so pan/zoom and reset-view stick around). After that first render, they're no longer undefined, so the next supplyDefaults from restyle/deleteTraces skips the entire fit-bounds block. react works because you're handing it a fresh layout with center/zoom undefined again.

Re: your question about other layout values that key off the data, autorange is the obvious comparison. It's the same idea, but it doesn't get stuck like this because it keeps an explicit autorange flag instead of overwriting the user's input with the computed value. That's basically what's missing here: "auto" and the saved-view state are stored in the same fields, so auto-fit can only fire once.

If we do want restyle/deleteTraces to re-fit, copying the autorange approach seems like the right path: keep a flag for "user never set center/zoom," recompute on data changes, and let uirevision decide whether to preserve a manual pan/zoom or throw it away. The map.fitBounds re-fit path already exists, so it's mostly about fixing the gating.

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 + react) and handle the data-only case in a follow-up? Happy to do either and take the implementation.

I'll add the Jasmine tests for center/zoom across the different calls, plus the uirevision case, once we settle on the expected behavior.

thanks.

[palmerusaf]

Sorry for the late reply. I've been quite busy with work and school. One issue I found with this implementation is that it doesn't handle points around the anti-meridian well.

This seems like a common issue. Even one of our dependencies, turf.js. doesn't handle it well either.
One solution that works is to normalize the lat inputs to 360, sort them, iterate over them to find the largest gap, then take the complement of that. However, this introduces sorting which will be expensive when the input is large.