pallets-eco / flask-admin

Simple and extensible administrative interface framework for Flask
https://flask-admin.readthedocs.io
BSD 3-Clause "New" or "Revised" License
5.8k stars 1.58k forks source link

Default Leaflet Basemap Tiles #2034

Closed jtbaker closed 3 months ago

jtbaker commented 4 years ago

Would it be possible to move toward a non-vendor lock in from Mapbox for the basemap tile provider? Per #1910 There seem to be some issues with the implementation and the MAPBOX_ID URI reference may be deprecated in their APIs.

There are also several open map tile providers that do not require an API key or other authentication mechanism. I think one of these (probably the default OpenStreetMap tiles) would make more sense for most users as a default configuration since this is a simple admin tool.

I'll write up a PR for such a change, including the documentation if no one objects.

LvanWissen commented 3 years ago

This is possible if you make use of the data attributes you can pass to the form, in particular this setting: https://github.com/flask-admin/flask-admin/blob/c8877df933ed4842e4bb8f2c8e9301c5507a42b4/flask_admin/static/admin/js/form.js#L162

Example:

from flask_admin.contrib.geoa import ModelView

class CityView(ModelView):

    form_widget_args = {
        # field 'coordinates' is of type Geometry in my model
        'coordinates': {
            'data-tile-layer-url': "{s}.tile.openstreetmap.org/{z}/{x}/{y}.png"
        }
    }

Check out all the $el.data('my-setting') configurables that you can set this way in https://github.com/flask-admin/flask-admin/blob/master/flask_admin/static/admin/js/form.js

Edit: It's actually much simpler since the 'tile_layer_url' setting is checked in https://github.com/flask-admin/flask-admin/blob/master/flask_admin/contrib/geoa/typefmt.py.

Simply add tile_layer_url (and tile_layer_attribution) to your view like so:

from flask_admin.contrib.geoa import ModelView

class CityView(ModelView):

    tile_layer_url = '{s}.tile.openstreetmap.org/{z}/{x}/{y}.png'
    tile_layer_attribution = '&copy; <a href="https://www.openstreetmap.org/copyright">OpenStreetMap</a> contributors'
jtbaker commented 3 years ago

Thank you @LvanWissen! I didn't realize it was configurable via subclassing the model.

TheoLechemia commented 1 year ago

Thanks for the tip @LvanWissen However, we still need put the MAPBOX_MAP_ID in the config in we want to have a map. this parameter control the load of the leaflet lib : https://github.com/flask-admin/flask-admin/blob/master/flask_admin/templates/bootstrap4/admin/lib.html#L260 and other stuff if form.js : https://github.com/flask-admin/flask-admin/blob/master/flask_admin/static/admin/js/form.js#L76 Would it not be better to use a parameter like MAP_TILE_LAYER in the config ? I can make a PR if you approve.

LvanWissen commented 1 year ago

Good point, but in terms of features it's only a naming flaw. Go ahead if you want to make a PR, but maybe check with @mrjoes first if this is desirable and how to make sure it's not breaking current implementations?

samuelhwilliams commented 3 months ago

We should tidy up the MAPBOX_MAP_ID conditional so that it's more clear that this is a gate for any map implementation. We should probably also document the overrides tile_layer_url and tile_layer_attribution better.

samuelhwilliams commented 3 months ago

@TheoLechemia I've raised https://github.com/pallets-eco/flask-admin/pull/2472 that covers this in the last commit; feel free to share your thoughts.