Graticule integration - #52
Conversation
|
Hi, Mridul - when it's running, can you push a copy to your gh-pages branch too so we can try it out? Thanks! |
|
This is looking really good. We need to make a couple follow up issues:
Thank you, Mridul!!! This is great progress. |
|
I don't find it on npm I tried earlier as well that's why we need it in source, also we will be altering the code for graticulate, thus we need it anyways. |
|
So you can link to a URL in package.json --
https://stackoverflow.com/questions/17509669/how-to-install-an-npm-package-from-github-directly
And if we need to make changes, we should fork it and submit PRs over there
-- what changes do you need to make? I can help think through what should
be in our library and what in theirs.
If we fork, we can point our package.json towards our fork until they
accept our PR ;-)
…On Mon, Jul 3, 2017 at 5:19 PM, Mridul Nagpal ***@***.***> wrote:
I don't find it on npm I tried earlier as well that's why we need it in
source, also we will be altering the code for graticulate, thus we need it
anyways.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#52 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ7ewji-r-c8Gwt_4uCxBdMy6CELRks5sKVrFgaJpZM4OMqlJ>
.
|
|
I think this is only appllicable to npm packages that are registered and this one is not. I am getting several errors. I tried all of the solutions on that page |
|
Can you post what you tried? I'm sure this works without npm registration; see how the PublicLab.Editor is doing this: https://github.com/publiclab/PublicLab.Editor/blob/master/package.json#L28 I had to fork and adjust that library, so i linked directly to the URL of my modded fork, specifying a branch with |
|
I did |
|
Try adding this to "leaflet-graticule": "git://github.com/Leaflet/Leaflet.Graticule.git#master"then run |
|
I actually tried that earlier, same issue appears to occur again |
|
Ack! it's because it's But also it needs a package.json -- so i forked, opened a new branch & PR, and you can now: until they accept my PR: Leaflet/Leaflet.Graticule#1 |
|
My bad, corrected this |
| // | ||
| // expect(blurredLocation.gridSystem.getCellSize().rows).toBe(72.8); | ||
| // expect(blurredLocation.gridSystem.getCellSize().cols).toBe(118.3); | ||
| // |
There was a problem hiding this comment.
Is there a way to write a test for this? Or shall we just depend on Graticule? I'm not sure, does Graticule have tests?
There was a problem hiding this comment.
I think we will have to depend on graticule for that
| }; | ||
|
|
||
| options.zoom = options.zoom || 6; | ||
| options.map = options.map || new L.Map('map',{zoomControl:false}).setView([options.location.lat, options.location.lon], options.zoom); |
There was a problem hiding this comment.
Can you break this up onto multiple lines? For readability.
| lat: 41.011234567, | ||
| lon: -85.66123456789 | ||
| }; | ||
| var stamenTerrain = L.tileLayer("https://a.tiles.mapbox.com/v3/jywarren.map-lmrwb2em/{z}/{x}/{y}.png").addTo(options.map); |
There was a problem hiding this comment.
Hmm, why this name? Maybe something more generic, like tileLayer?
There was a problem hiding this comment.
It was just a default, will change to tileLayer
| function addGrid() { | ||
| layer = L.latlngGraticule({ | ||
| showLabel: true, | ||
| zoomInterval: [ |
There was a problem hiding this comment.
Is this repeated? Maybe use an options parameter?
|
Done Please have a look. |
|
Great!!! |
2 participants
No description provided.