-
Notifications
You must be signed in to change notification settings - Fork 90
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
Fix: Restrict Map Zoom Levels to Supported Range #331
base: master
Are you sure you want to change the base?
Conversation
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.
The issue description asks to allow to make this configurable, while providing a sensible default. I don't think having this is a sensible default.
Okay, working on it |
Hello @nemesifier |
mapOptions: { | ||
center: [46.86764405052012, 19.675998687744144], | ||
zoom: 5, | ||
maxZoom: 18, | ||
zoomControl: true, |
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.
is this necessary?
@@ -33,6 +33,8 @@ const NetJSONGraphDefaultConfig = { | |||
svgRender: false, | |||
switchMode: false, | |||
maxPointsFetched: 10000, | |||
maxZoom : 18, | |||
minZoom : 3, |
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.
These options are defined 3 times.
Are you sure these are needed here? It seems to me they're not.
@@ -161,6 +163,7 @@ const NetJSONGraphDefaultConfig = { | |||
mapOptions: { | |||
roam: true, | |||
zoomAnimation: false, | |||
maxZoom: 18, |
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.
Maybe we can move the minZoom
option here.
self.leaflet = self.echarts._api.getCoordinateSystems()[0].getLeaflet(); | ||
self.leaflet._zoomAnimated = false; | ||
|
||
self.leaflet.options.maxZoom = self.config.maxZoomLevel; |
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.
where on hearth is maxZoomLevel
coming from??
Checklist
Reference to Existing Issue
Closes #188
Description of Changes
earlier the max zoom was 32 by default , now it's reduced to 18 for better viewing experience