Add support for Leaflet plugin 'Leaflet.vector-markers' #942
Conversation
| @@ -14,6 +14,7 @@ | |||
| "dist/angular-leaflet-directive.js" | |||
| ], | |||
| "dependencies": { | |||
| "Leaflet.vector-markers": "https://github.com/hiasinho/Leaflet.vector-markers", | |||
There was a problem hiding this comment.
There are no specific releases?
There was a problem hiding this comment.
I'd prefer an sha# at least.
There was a problem hiding this comment.
I could not find it in the Bower repository, so I used the GitHub URL instead.
I'll add a SHA#.
There was a problem hiding this comment.
@molehillrocker Perhaps ask if the maintainer would publish it at Bower and/or create version tags at Github? I often find asking working pretty well. ;-)
There was a problem hiding this comment.
@molehillrocker Although, now that I checked, that package already has release tags (latest 0.0.3) and it's also at Bower:
$ bower search vector-markers
Search results:
leaflet.vector-markers git://github.com/hiasinho/Leaflet.vector-markers.git(ping @hiasinho)
There was a problem hiding this comment.
Oh, I did not try vector-markers solely. I thought packages are registered at Bower with the name that is denoted in the bower.json file.
Just to get it right:
- You prefer Bower packages over raw GitHub links?
- You prefer using Git tags over SHA# and SHA# over no version identifier at all?
|
Looks good besides comments. Anyway you could come up with some specs to add to markers to make sure a vector marker works . This is more important with not having a release tag attached to the bower.json. |
|
I'm some kind of a noob in the JavaScript world, by 'specs' you mean tests? 😆 |
|
yes test/specs mean basically the same |
| @@ -32,6 +32,7 @@ | |||
| "leaflet-tilelayer-geojson": "*", | |||
| "Leaflet.utfgrid": "danzel/Leaflet.utfgrid", | |||
| "Leaflet.awesome-markers": "*", | |||
| "Leaflet.vector-markers": "leaflet.vector-markers#0.0.3", | |||
There was a problem hiding this comment.
why not
"Leaflet.vector-markers": "0.0.3",
or
"Leaflet.vector-markers": "0.0.*",
There was a problem hiding this comment.
Bower couldn't load the library when just using the version number, also this is what it saved into the bower.json when using bower install leaflet.vector-markers --save-dev.
But maybe this is a local problem of mine, when the pure version number is supposed to work also? I'll try.
There was a problem hiding this comment.
Ah, the library is registered at Bower with all letters written in lowercase. Thus, Bower added the repository name in the value part. I'll fix that.
|
Basically LGTM (except bower.json) , but specs would be great as other people will have to maintain this. Basic specs to check for marker creation as a vector is all I am asking for at a minimum. |
|
Okay, I'll try my best. I've never written tests before in JS or CoffeeScript, so it may take some time to dive into this. |
|
@molehillrocker take it as a learning challenge. ;-) Remember that if it doesn't work for you, just give heads ups here and we'll help. |
| @@ -0,0 +1,99 @@ | |||
| 'use strict'; | |||
|
This looks good, but why are so many random commits from other people in this? Should this be rebased? Just trying to make sure we don't lose changes that exist in master. |
|
Yepp, rebase + squash. @molehillrocker let us know if you need help with that. |
|
|
- Leaflet.vector-markers is a fork of Leaflet.awesome-markers, but instead of PNGs it uses SVGs as markers. - Add additional example to the already existing marker examples.
- The former dependency to Leaflet.vector-markers was added manually, since the package was not found using Bower due to a type in its name... -_- - Using the proper package name worked. - Remove the dependency declaration from the dependencies and add it to the devDeps only.
- The uppercase L in the library name caused Bower to fail when downloading the library. Thus, the GitHub repository was required to be denoted in the version part. - Using the correct library name fixes this problem and allows us to just use the version number solely.
|
Mmmh I guess I previously mixed fetch/rebase from origin and upstream. Now I rebased onto |
|
cool |
…rkers Add support for Leaflet plugin 'Leaflet.vector-markers'
of PNGs it uses SVGs as markers.