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

Update all maps to MapboxGL #749

Open
russbiggs opened this issue Oct 31, 2019 · 3 comments
Open

Update all maps to MapboxGL #749

russbiggs opened this issue Oct 31, 2019 · 3 comments

Comments

@russbiggs
Copy link
Contributor

It looks like the maps on / and /create still use leaflet. campaign/{uuid} uses mapboxGL. Is it worth updating all the maps for consistency?

@russbiggs
Copy link
Contributor Author

@localjo localjo mentioned this issue Nov 1, 2019
11 tasks
@localjo
Copy link
Collaborator

localjo commented Nov 4, 2019

It looks like mapbox-gl is used in map.html, and the contents of map.html are injected into campaign_detail.html.

I think as we touch each page in the app and update it to use the new design, we could switch it to use mapbox-gl too. For now, I'll comment out the leaflet scripts in the new base template, and see if I can easily switch the user overview page to mapbox-gl while working on #752 . If we run into some leaflet code that is difficult to switch, we can uncomment the leaflet scripts.

@JorgeMartinezG
Copy link
Collaborator

@localjo yes! In fact, I'm working on that map.html rendering from the backend, since it is needed for #724 .

You can check the changes here: e2c2579. I'll finish this and send the PR this week.

I think this is a starting point for removing the lambda rendering functions. Since map.html needs only the campaign geometry and the first type, we can send this from context dictionary and let Jinja render it in each request.

Let me know your opinion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants