Search code, repositories, users, issues, pull requests...

Copy link

Contributor

@camdecoster camdecoster commented Feb 18, 2025 •

edited

Loading

Description

Add minscale and maxscale attributes to geo plots. These set the min and max scale extents for a plot (how far a user can zoom out/in). This PR supersedes #5192 which will be closed after this is merged.

Changes

Adds minscale and maxscale attributes to geo plots. Defaults are 0 and -1, respectively.
Enforces scale extents on zoom mouse events, mode bar button clicks
Manually creates zoom event to enforce scale extents on first render (treating the middle of the plot as the zoom point)

Demo Video or Screenshot(s):

Before:

Screencast.From.2025年02月22日.11-25-20.mp4

After:

Screencast.From.2025年02月22日.11-29-21.mp4

Testing

Start the dev dashboard with npm start
Search for a geo mock and load it (I used geo_legendonly)
Zoom in and out and note how you can go as far as you want in either direction
Edit the mock file and add the following attributes under the "geo" key
- "minscale": 2
- "maxscale": 4
Reload the mock in the dashboard
Note that the mock appears zoomed in upon first load
Zoom in and out and note how you're now limited in how far you can go

mojoaxel and others added 8 commits

February 18, 2025 07:13


 feat(geo): support min/max scale limits

57307ee

fixes #5191


 refactor: rename min/max scale limits

d870252


 chore: use dflt: -1 instad of null

2d9e5f2


 feat: handle maxscale default in scaleExtent

dd635ee


 chore: only rerender on zoom if scale changes

8a2c0c5


 chore: fix typo in min/maxscale attributes

f882191


 test: adopt test/plot-schema.json to new plot-schema

8ed949f


 Trigger initial zoom event to set minscale

29b4316

@camdecoster camdecoster force-pushed the cam/5192/add-min-max-scale-limits branch from c7b5ec3 to 29b4316 Compare

February 18, 2025 14:15

@gvwilson gvwilson requested review from alexcjohnson, emilykl and marthacryan

February 18, 2025 14:47

@gvwilson gvwilson assigned camdecoster

Feb 18, 2025

@gvwilson gvwilson added P1 fix labels

Feb 18, 2025

camdecoster added 4 commits

February 21, 2025 06:52


 Switch to ES5 syntax

048f178


 Merge remote-tracking branch 'origin/master' into cam/5192/add-min-ma...

c6007a8

...x-scale-limits


 Switch to getBBox and fix zoom.event call

84cd037


 Add draft log

d9fca8b

Copy link

Contributor Author

camdecoster commented Feb 22, 2025

I believe that some tests are failing because there's now an extra zoom event on every call to updateFx (for dragMode === 'pan'). I think I need to restrict that call to only happen upon first load. I'll keep looking into it.

add support for min/max scale limits #5192


 Merge remote-tracking branch 'origin/master' into cam/5192/add-min-ma...

5f0083f

...x-scale-limits

@archmoj archmoj marked this pull request as ready for review

July 21, 2025 15:13

This was referenced Jul 21, 2025

Closed

feature: Add projection.minscale and projection.maxscale to geo subplots #7482

Open

camdecoster and others added 5 commits

July 23, 2025 11:21


 Merge remote-tracking branch 'origin/master' into cam/5192/add-min-ma...

0a1f133

...x-scale-limits


 Add event check before triggering zoom transition

ee8597f


 Don't delete default scales for fitbounds

9b04737

@archmoj @camdecoster


 add tests for various cases

32feb89


 Update tests to check scale against scaleExtent

65665fe

Copy link

Contributor Author

camdecoster commented Jul 25, 2025

@archmoj this PR should now be good to review. We don't need both this and #7482. I'd like to argue that we should use the approach of using scaleExtent (the native D3 mechanism) rather than limiting the scale on our side.

archmoj

archmoj reviewed

Jul 25, 2025

View reviewed changes

src/plots/geo/layout_attributes.js

maxscale: {

valType: 'number',

min: 0,

dflt: -1,

Copy link

Contributor

@archmoj archmoj Jul 25, 2025

Choose a reason for hiding this comment

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

Let's try setting dflt: 0 in the API and handle the case of zero instead of -1.

Copy link

Contributor Author

@camdecoster camdecoster Jul 25, 2025

Choose a reason for hiding this comment

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

We represent Infinity with -1 since Infinity isn't valid in JSON. If we made this change, how would we handle when a user wants to use Infinity instead of 0 (which is also a valid value).

Copy link

Contributor

archmoj commented Jul 25, 2025

Opening geo_canadian-cities mock in the dashboard gives me the following error:

plotly.js:9976 Uncaught (in promise) RangeError: Maximum call stack size exceeded
 at String.replace (<anonymous>)
 at stringInputToObject (plotly.js:9976:27)
 at inputToRGB (plotly.js:9412:22)
 at new tinycolor (plotly.js:9165:22)
 at tinycolor (plotly.js:9163:20)
 at color2.fill (plotly.js:11073:18)
 at d3_selectionPrototype.call (plotly.js:1352:20)
 at SVGGElement.<anonymous> (plotly.js:145654:18)
 at plotly.js:1339:22
 at d3_selection_each (plotly.js:1345:36)

@camdecoster do you get the same?

Copy link

Contributor Author

camdecoster commented Jul 25, 2025

@archmoj that should be fixed by some recent commits. Are you on the latest version of the branch? I'm not seeing that error.

Copy link

Contributor

archmoj commented Jul 25, 2025

geo_canadian-cities

Thanks @camdecoster. My local test branch was not up-to-date. I confirm now it renders with no error.

Copy link

Contributor

archmoj commented Jul 25, 2025

If you add

"projection": { "minscale": 0.5, "maxscale": 2 },

to geo_canadian-cities under "geo" object, the map renders smaller with gd._fullLayout.geo.projection.scale: 0.3013880580354019.

@camdecoster Could you please investigate that?