[fix] Set default value for maxZoom and limit maxZoom option #188 #248
Conversation
The issue of empty responses from the tile server when zooming in beyond the supported zoom level has been addressed by setting the default maxZoom level to 15. This change ensures that tiles are displayed up to the supported zoom level. If a different default maxZoom level is desired, it can be modified by adjusting the value of . Closes #188
|
@nemesifier Please review this pull request. If any improvements can be made, please provide comments. |
|
I think due to the dependency update the end Dot is vanished. i followed this implementation to solve this but it did not completely work: |
There was a problem hiding this comment.
Good progress @d1vyanshu-kumar, this seems to work.
We'll need to mention this in the README, we also need at least 1 test for this.
| @@ -76,9 +76,11 @@ function createLeafletCoordSystem(echarts, L) { | |||
| LeafletCoordSys.prototype.dimensions = ["lng", "lat"]; | |||
|
|
|||
| LeafletCoordSys.prototype.setZoom = function (zoom) { | |||
| if (zoom > 15) { | |||
There was a problem hiding this comment.
what if the maxZoom is higher? Why hardcoding this value here? Can we get the maxZoom here?
There was a problem hiding this comment.
@nemesifier, I think a maximum zoom level of 12-18 is appropriate. If we zoom beyond that, we encounter empty responses from the tile server.
https://wiki.openstreetmap.org/wiki/Zoom_levels
but now I think if it is not an ideal approach could you please give me some Hints/approaches on this?
There was a problem hiding this comment.
We should pass the default config here and get the maxZoom value from it instead of duplicating it here.
|
@nemesifier I apologize for the failed test cases. I am currently investigating the issue and will resolve it promptly. Regarding the choice of a maximum zoom level of 15, it serves as a placeholder value commonly used with OpenStreetMap (OSM) tiles. OSM tiles generally offer detailed coverage up to zoom level 15 or 19. As mentioned in the issue, the current behaviour allows zooming beyond the supported levels (up to 30m zoom) by default. This results in empty responses from the tile server, leaving the map without any tiles. Therefore, I believe a maximum zoom level of 15 is appropriate. If we zoom beyond that, we encounter empty responses from the tile server. |
| @@ -44,6 +56,15 @@ export default function extendLeafletModel(echarts) { | |||
| }, | |||
| ], | |||
| layerControl: {}, | |||
| maxZoom: 15 | |||
| @@ -76,9 +76,11 @@ function createLeafletCoordSystem(echarts, L) { | |||
| LeafletCoordSys.prototype.dimensions = ["lng", "lat"]; | |||
|
|
|||
| LeafletCoordSys.prototype.setZoom = function (zoom) { | |||
| if (zoom > 15) { | |||
There was a problem hiding this comment.
We should pass the default config here and get the maxZoom value from it instead of duplicating it here.
|
In the meantime I reverted the echarts version: |
The issue of exceeding the supported zoom level with different tile providers. Previously, zoom requests beyond the provider's limit caused empty responses and missing tiles.
The implemented changes include:
Limiting the zoom level in the setCenterAndZoom method by comparing the requested zoom with the maximum supported zoom (maxZoom).
Optionally adjusting the maxZoom option of the zoom control if it exists, ensuring alignment with the actual limitations.
By implementing these changes, we ensure compatibility with various tile providers and prevent unintended behavior due to excessive zooming.
zoom.setup.5.mp4