search

Leaflet / Leaflet.Icon.Glyph

Add glyphs from icon fonts to your LeafletJS markers
130 stars 45 forks source link

the "shape version" #3

Closed ckhung closed 8 years ago

ckhung commented 8 years ago

@IvanSanchez: thanks for writing Leaflet/Leaflet.Icon.Glyph I need to create markers of different colors for buses and bikes and found your code a nice starting point to build upon. Wonder if my enhancement is suitable to be merged back to your project. I am a collaboration newbie and I am also not very familiar with the js dev tools, so please let me know if anything should be improved. Or I'll change the name of my fork and go on with my own repo if this enhancement is not directly relevant to your original project.

IvanSanchez commented 8 years ago

Hi @ckhung!

My answer is going to be "no". The goal of L.Icon.Glyph is to provide glyphs, not to provide extra marker images and colours.

I am a collaboration newbie and I am also not very familiar with the js dev tools, so please let me know if anything should be improved.

The code itself looks quite alright! If I were to improve something, it would be taking L.RotatedMarker out, and using variableNamesWithCamelCase instead of variable_names_with_underscores (because that's how all the Leaflet code is written).

Or I'll change the name of my fork and go on with my own repo if this enhancement is not directly relevant to your original project.

Yes. I don't think more marker images are within the scope of L.Icon.Glyph. But if you want to fork this into something like L.Icon.AwesomeGlyph or whatever, go ahead! ;-)

ckhung commented 8 years ago

Thanks for the advice!