wp-plugin-leaflet-map
wp-plugin-leaflet-map copied to clipboard
max_zoom and default tiles
Hi Bozdoz,
Due to a bug in my plugin, I took one more look at max_zoom. I would like to change https://github.com/bozdoz/wp-plugin-leaflet-map/blob/b6bafe8f4fd1373776201768f5a179d55fcdc845/shortcodes/class.map-shortcode.php#L81-L82 to
$atts['max_zoom'] = empty($max_zoom) ?
$settings->get('default_max_zoom') :
($max_zoom <= $settings->get('default_max_zoom') ? $max_zoom : $settings->get('default_max_zoom'));
Default max zoom should be the max zoom of default tiles or smaller.
I make a pull request.
I don't understand. If the user passes max-zoom, why would we override that with the default max zoom?
Some tiles have a max zoom and the max zoom from map must not be greater as the max zoom from tiles. If the max zoom from map is greater as max zoom from tiles you get grey areas.
Just seems like a difficult problem to debug if someone wants to override the default but can't
Or we need to have two options max_zoom, one for map and one for tiles. And the same for min_zoom. https://leafletjs.com/reference.html#map-minzoom https://leafletjs.com/reference.html#map-maxzoom
New approach:
https://github.com/bozdoz/wp-plugin-leaflet-map/blob/b6bafe8f4fd1373776201768f5a179d55fcdc845/shortcodes/class.map-shortcode.php#L193-L201
We need some more options. There is a repository leaflet-providers and here are the options for many tileservers. Is there a convenient way to consider many different options without defining a setting for each possible one? For now you have defined some options for MapQuest.
To avoid grey areas there is an option maxNativeZoom.
I suppose min and max zoom overlap too much with tilelayer min and max; why don't we just add tileMaxZoom?
This I have done now. I prepared a pull request. Sorry about the removed spaces, this makes my editor automatically.
class.leaflet-map.php:
- leafletjs version 1.9.4
class.plugin-settings.php
- changed default max_zoom to 18, because https://leafletjs.com/reference.html#map-maxzoom and https://leafletjs.com/reference.html#tilelayer-maxzoom
- removed subdomains from default map_tile_url
- new option map_tile_maxzoom
readme.txt
- WordPress 6.3
shortcodes/class.map-shortcode.php
- changed getting map maxZoom, if empty
- tile maxZoom
templates/settings.php:
- added _e(' - Please change JavaScript-URL and CSS-URL.', 'leaflet-map');
What do you mean?
Thanks! I'll take a look next week.
Why remove subdomains?
I read this in german OSM forum. There is a link to an issue. And in leaflet-providers.js are the tiles without subdomain.
Great find. Makes sense
Could you make a pull request of that branch?
max_zoom and map_tile_maxzoom do not work yet together, and it takes time to test. If I write
[leaflet-map zoom=18 max_zoom=23 map_tile_maxzoom="20" tileurl=https://....]
I get grey tiles if I zoom to a zoomlevel greater than map_tile_maxzoom. It should not be possible to zoom to that zoomlevel.
Right but what would you expect given a shortcode like that? Are you suggesting that map max zoom should be restricted to map tile max zoom no matter what?
I made a commit. Now it works as I expect. But it does not work, if detect-retina is true and it is a retina display. Then the maxZoom should be one step lower.
I think, I found the solution, it works with Retina also now.
The differences to your reposiitory are:
- changed default max_zoom to 18, because https://leafletjs.com/reference.html#map-maxzoom and https://leafletjs.com/reference.html#tilelayer-maxzoom
- removed subdomains from default map_tile_url
- new option map_tile_maxzoom
- new option map_tile_minzoom
- no grey tiles anymore
Maybe we need to change "Info: your leaflet version may be out-of-sync with the latest default version: 1.9.4". Many don't know what to do with it. But you do it, your english is better than mine.
What do you mean?
Yeah I looked into changing it. Maybe we could output the js and css urls we expect for latest.