bozdoz / wp-plugin-leaflet-map

Add leaflet maps to Wordpress with shortcodes
https://wordpress.org/plugins/leaflet-map/
GNU General Public License v2.0
142 stars 72 forks source link

max_zoom and default tiles #232

Open hupe13 opened 1 year ago

hupe13 commented 1 year ago

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.

bozdoz commented 1 year ago

I don't understand. If the user passes max-zoom, why would we override that with the default max zoom?

hupe13 commented 1 year ago

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.

bozdoz commented 1 year ago

Just seems like a difficult problem to debug if someone wants to override the default but can't

hupe13 commented 1 year ago

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

hupe13 commented 1 year ago

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.

bozdoz commented 1 year ago

I suppose min and max zoom overlap too much with tilelayer min and max; why don't we just add tileMaxZoom?

hupe13 commented 1 year ago

This I have done now. I prepared a pull request. Sorry about the removed spaces, this makes my editor automatically.

class.leaflet-map.php:

class.plugin-settings.php

readme.txt

shortcodes/class.map-shortcode.php

templates/settings.php:

What do you mean?

bozdoz commented 1 year ago

Thanks! I'll take a look next week.

bozdoz commented 1 year ago

Why remove subdomains?

hupe13 commented 1 year ago

I read this in german OSM forum. There is a link to an issue. And in leaflet-providers.js are the tiles without subdomain.

bozdoz commented 1 year ago

Great find. Makes sense

bozdoz commented 1 year ago

Could you make a pull request of that branch?

hupe13 commented 1 year ago

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.

bozdoz commented 1 year ago

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?

hupe13 commented 1 year ago

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.

hupe13 commented 1 year ago

I think, I found the solution, it works with Retina also now.

The differences to your reposiitory are:

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?

bozdoz commented 1 year ago

Yeah I looked into changing it. Maybe we could output the js and css urls we expect for latest.