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

Fix Leaflet marker image path discovery #586

Closed
wants to merge 1 commit into from
Closed

Conversation

zarino
Copy link
Member

@zarino zarino commented Jul 17, 2024

Including url("/static/leaflet/images/marker-icon.png") in our Sass doesn’t work because django-compressor adds a hash as a query parameter, which breaks Leaflet’s /^(.*)marker-icon\.png$/ regexp.

So this commit removes the ruleset from our Sass pipeline, and just embeds it directly into the HTML page, where django-compressor can’t mess it up.

Including `url("/static/leaflet/images/marker-icon.png")` in our Sass
doesn’t work because django-compressor adds a hash as a query parameter,
which breaks Leaflet’s `/^(.*)marker-icon\.png$/` regexp.

So this commit removes the ruleset from our Sass pipeline, and just
embeds it directly into the HTML page, where django-compressor can’t
mess it up.
@zarino
Copy link
Member Author

zarino commented Jul 18, 2024

FYI @struan I’ve tested this out locally (with DEBUG=False in my docker env) and it works – and have verified that the URL in the HTML gets passed to Leaflet when it’s making its marker. Whether something on the live site (but not on Docker under DEBUG=False) will add a hash to the URL remains to be seen.

Note, though, I really think we should be setting this path with Icon.Default.imagePath = "<whatever>" in explore.esm.js but that doesn’t seem to work. Maybe because we’re importing bits of Leaflet individually (eg: import { Icon } from 'leaflet/src/layer/marker') and the modification doesn’t carry through separate imported objects? Must admit this is the point I get out my depth with ES6 Modules 😬 – see alternate pull request in #588

@zarino zarino marked this pull request as ready for review July 18, 2024 16:54
@zarino zarino requested a review from struan July 18, 2024 16:54
@zarino
Copy link
Member Author

zarino commented Jul 22, 2024

Closing in favour of #588.

@zarino zarino closed this Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants