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

Leaflet marker- assets not included in build. #501

Closed
erictheise opened this issue Jul 30, 2021 · 4 comments
Closed

Leaflet marker- assets not included in build. #501

erictheise opened this issue Jul 30, 2021 · 4 comments
Assignees
Labels
Milestone

Comments

@erictheise
Copy link
Member

I'm chasing down some 404 errors as I work with the new unpkg distribution.

Is there a reason that the Gruntfile explicitly grabs only the layers images?

src: ['layers.png','layers-2x.png'],
@erictheise erictheise added the bug label Jul 30, 2021
@erictheise erictheise added this to the v0.9.0 milestone Jul 30, 2021
@Malvoz
Copy link
Member

Malvoz commented Jul 30, 2021

The reasoning is that Leaflet markers aren't used anymore, see #334. Preferably they shouldn't be fetched in that case...

@erictheise
Copy link
Member Author

erictheise commented Jul 30, 2021

leaflet.css still tries to fetch marker-icon.png though.

/* Default icon URLs */
.leaflet-default-icon-path {
	background-image: url(images/marker-icon.png);
	}

Leaving that one, actually all three, in the build seems a smaller price to pay than patching leaflet.css and leaflet-src.js. What do you think?

@Malvoz
Copy link
Member

Malvoz commented Jul 30, 2021

Well, there are issues with using Leaflet as is. For example:

  • We're inheriting quite a few accessibility issues. And I don't expect them (for reasons) to be accepting PRs to fix them any time soon.
  • I'd like to lower the specificity of selectors in leaflet.css (and mapml.css) in general (it'd give this component a "native feel" to it, but also allow authors to write styles without !important statements).

But forking Leaflet hasn't been discussed much. Maybe it's time to, @prushforth?

I'm okay with adding back the markers in the grunt file for now though.

@erictheise erictheise self-assigned this Jul 30, 2021
@prushforth
Copy link
Member

Yeah, let's put those back in for now.

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

No branches or pull requests

3 participants