Skip to content
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

Default Leaflet Basemap Tiles #2034

Closed
jtbaker opened this issue Sep 24, 2020 · 6 comments · Fixed by #2472
Closed

Default Leaflet Basemap Tiles #2034

jtbaker opened this issue Sep 24, 2020 · 6 comments · Fixed by #2472
Assignees

Comments

@jtbaker
Copy link
Contributor

jtbaker commented Sep 24, 2020

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
Copy link
Contributor

LvanWissen commented May 7, 2021

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
Copy link
Contributor Author

jtbaker commented May 21, 2021

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

@jtbaker jtbaker closed this as completed May 21, 2021
@TheoLechemia
Copy link

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
Copy link
Contributor

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?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 9, 2024
@samuelhwilliams samuelhwilliams self-assigned this Jul 23, 2024
@samuelhwilliams
Copy link
Contributor

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
Copy link
Contributor

@TheoLechemia I've raised #2472 that covers this in the last commit; feel free to share your thoughts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants