Graticule integration #52

mridulnagpal · 2017-07-03T20:37:03Z

No description provided.

jywarren · 2017-07-03T20:47:44Z

Hi, Mridul - when it's running, can you push a copy to your gh-pages branch too so we can try it out? Thanks!

mridulnagpal · 2017-07-03T20:51:38Z

Here you go https://mridulnagpal.github.io/leaflet-blurred-location/examples/

jywarren · 2017-07-03T21:14:45Z

This is looking really good. We need to make a couple follow up issues:

let's tweak the grid resolution per zoom level so that multiple grid squares are shown for each zoom level (this may take some manual tweaking) -- and this will be what sets add setZoomByPrecision() method #44's setZoomByPrecision() - even if we don't do an algorithmic approach, we could actually literally do a look up table... worth considering. This can be a separate issue and separate PR.
we need to highlight the center grid square now (I think this'll be the hard part, but if you look in how Graticule is implemented, it shouldn't be too hard). This can be in a separate issue.
Can you require the Graticule plugin via npm using your package.json ?, instead of directly including the source? check if it's available on npmjs.org. This should be done in this PR. Thank you!

Thank you, Mridul!!! This is great progress.

mridulnagpal · 2017-07-03T21:19:01Z

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.

jywarren · 2017-07-03T21:32:37Z

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> .

mridulnagpal · 2017-07-04T02:31:57Z

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

jywarren · 2017-07-05T17:58:28Z

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 #branchname.

mridulnagpal · 2017-07-05T23:06:38Z

I did npm install --save-dev git://github.com/Leaflet/Leaflet.Graticule.git#master and got error

jywarren · 2017-07-06T18:37:08Z

Try adding this to package.json --

"leaflet-graticule": "git://github.com/Leaflet/Leaflet.Graticule.git#master"

then run npm update

mridulnagpal · 2017-07-07T03:38:25Z

I actually tried that earlier, same issue appears to occur again

jywarren · 2017-07-07T22:52:23Z

Ack! it's because it's git://github.com/Leaflet/Leaflet.Graticule.git, not git://github.com/Leaflet/Graticule.git! That really got me.

But also it needs a package.json -- so i forked, opened a new branch & PR, and you can now:

"leaflet-graticule": "git://github.com/jywarren/Leaflet.Graticule.git#patch-1"

until they accept my PR: Leaflet/Leaflet.Graticule#1

mridulnagpal · 2017-07-09T16:12:50Z

My bad, corrected this

jywarren · 2017-07-09T16:54:19Z

spec/javascripts/test_spec.js

+  //
+  //   expect(blurredLocation.gridSystem.getCellSize().rows).toBe(72.8);
+  //   expect(blurredLocation.gridSystem.getCellSize().cols).toBe(118.3);
+  //


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?

I think we will have to depend on graticule for that

jywarren · 2017-07-09T16:55:15Z

src/blurredLocation.js

+  };
+
+  options.zoom = options.zoom || 6;
+  options.map = options.map || new L.Map('map',{zoomControl:false}).setView([options.location.lat, options.location.lon], options.zoom);


Can you break this up onto multiple lines? For readability.

jywarren · 2017-07-09T16:56:31Z

src/blurredLocation.js

-    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);


Hmm, why this name? Maybe something more generic, like tileLayer?

It was just a default, will change to tileLayer

jywarren · 2017-07-09T16:57:37Z

src/core/gridSystem.js

+  function addGrid() {
+     layer = L.latlngGraticule({
+                    showLabel: true,
+                    zoomInterval: [


Is this repeated? Maybe use an options parameter?

mridulnagpal · 2017-07-09T17:18:08Z

Done Please have a look.

jywarren · 2017-07-09T18:15:08Z

Great!!!

mridulnagpal added 4 commits July 4, 2017 02:06

Graticule integrated

01a31ce

Graticule integrated

9f6381c

Cellsize spec removed

b09debf

Test passing

968de51

mridulnagpal added 2 commits July 4, 2017 02:32

Colored grid

c37dc41

map exposed to public API and Readme section added

ff4ef0c

tweaks

3c6acee

mridulnagpal added 3 commits July 9, 2017 21:37

Library included

22d36e4

tweaks

f94b7b5

tweaks

da783af

jywarren requested changes Jul 9, 2017

View reviewed changes

Tweaks

43177d9

mridulnagpal added 2 commits July 9, 2017 22:54

tweaks

bf3c9dc

Set interval changed

28df77e

jywarren approved these changes Jul 9, 2017

View reviewed changes

jywarren merged commit 99ef13b into publiclab:master Jul 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Graticule integration #52

Graticule integration #52

mridulnagpal commented Jul 3, 2017

jywarren commented Jul 3, 2017

mridulnagpal commented Jul 3, 2017

jywarren commented Jul 3, 2017

mridulnagpal commented Jul 3, 2017

jywarren commented Jul 3, 2017 via email

mridulnagpal commented Jul 4, 2017

jywarren commented Jul 5, 2017

mridulnagpal commented Jul 5, 2017

jywarren commented Jul 6, 2017

mridulnagpal commented Jul 7, 2017

jywarren commented Jul 7, 2017

mridulnagpal commented Jul 9, 2017

jywarren Jul 9, 2017

mridulnagpal Jul 9, 2017

jywarren Jul 9, 2017

mridulnagpal Jul 9, 2017

jywarren Jul 9, 2017

mridulnagpal Jul 9, 2017

jywarren Jul 9, 2017

mridulnagpal Jul 9, 2017

mridulnagpal commented Jul 9, 2017

jywarren commented Jul 9, 2017

Graticule integration #52

Graticule integration #52

Conversation

mridulnagpal commented Jul 3, 2017

jywarren commented Jul 3, 2017

mridulnagpal commented Jul 3, 2017

jywarren commented Jul 3, 2017

mridulnagpal commented Jul 3, 2017

jywarren commented Jul 3, 2017 via email

mridulnagpal commented Jul 4, 2017

jywarren commented Jul 5, 2017

mridulnagpal commented Jul 5, 2017

jywarren commented Jul 6, 2017

mridulnagpal commented Jul 7, 2017

jywarren commented Jul 7, 2017

mridulnagpal commented Jul 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mridulnagpal commented Jul 9, 2017

jywarren commented Jul 9, 2017