Implement changes to fractracker mobile layer #330

crisner · 2020-01-07T16:20:59Z

Fixes #87 (<=== Add issue number here)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

crisner · 2020-01-07T16:23:18Z

Work done:

Implemented GeoJson layer that fetches data on map's moveend event

Thinks to do:

Check and refactor popup code

crisner · 2020-01-07T16:32:39Z

@jywarren, I have set the maximum number of results per page which is 250. I was thinking users would zoom in to get detailed information which would trigger a request resulting in loading markers they may have missed before. But some views may still have more than 250 results. How do you think we should implement this? Should I loop through the pages to load all the markers for the map view the user is on? The current layer setup won't allow the user to access the layer for zoom levels that are less than 5.

sagarpreet-chadha · 2020-01-07T17:09:58Z

src/fracTrackerMobileLayer.js

+        var polygon = left + ' ' + top + ',' + right + ' ' + top + ',' + right + ' ' + bottom + ',' + left + ' ' + bottom + ',' + left + ' ' + top;
+
+        var $ = window.jQuery;
+        var fractrackerMobile_url = 'https://cors-anywhere.herokuapp.com/https://api.fractracker.org/v1/data/report?page=1&results_per_page=250&q={"filters":[{"name":"geometry","op":"intersects","val":"SRID=4326;POLYGON((' + polygon +'))"}],"order_by":[{"field":"report_date","direction":"desc"},{"field":"id","direction":"desc"}]}';


Hey,
What is this url cors-anywhere.herokuapp.com ?

This is to remove the cors policy issue. Jeff had suggested using it in the issue?

I know it doesn't fetch the results if this is not prepended. But I am planning to look into it more tomorrow.

This allows us to fetch it despite CORS restrictions. We've confirmed this is OK with the FracTracker team. Thanks!

sagarpreet-chadha · 2020-01-07T17:13:37Z

Hey @crisner ,

In the URL you have used: page=1&results_per_page=250
So this will fetch same 250 results everytime, right?

crisner · 2020-01-07T17:30:57Z

In the URL you have used: page=1&results_per_page=250
So this will fetch same 250 results everytime, right?

It depends on your map view. But it would fetch the same results for the same map view. To put it simply it depends on the map's current bounds.

jywarren

I think we can just assume people will keep zooming in to find more. 250 is a lot! It sounds good for now! Thanks!!!

jywarren · 2020-01-08T00:06:00Z

I think this looks good to go! Shall we merge it?

crisner · 2020-01-08T01:13:41Z

I need to go over the popup information to make sure I haven't missed anything. It will be ready today (Jan 8). Thanks! :)

crisner · 2020-01-08T01:41:35Z

Also, noting it here to remind myself to update the leafletEnvironmentalLayers.js file with the change.

crisner · 2020-01-08T07:08:29Z

Changes done:

Refactored popup code to handle undefined and null from data properties
Changed layer name to match with the one listed in README.md
Updated AllLayers.js file to reflect the changes
Added promise to ajax request

crisner · 2020-01-08T09:19:14Z

I've also added a simple test to check if the layer gets added to the map. I tried to check for markers but because of the delay in response, the data doesn't become available when the test runs. There should be a better way to check this. I'll work on this later this week or when we are adding missing tests. For now, this PR is ready. Thanks!

jywarren · 2020-01-08T14:43:23Z

This is awesome! Thanks so much. Shall we bump the version number in package.json and also get the latest gh-pages branch pushed up so the main demo is live?

crisner · 2020-01-08T14:59:15Z

Sure. But if I'm not wrong, we had already bumped it once from 2.0.6 to 2.1.6 and hadn't published it. Do we bump it again?

jywarren · 2020-01-08T15:00:45Z

well, in theory only the commits up until the version change in package.json should be included, i think... i guess it's kind of an ambiguity though. I think I can just publish this as 2.1.6 since we ought to have published it as 2.1.0 before anyways. Sound good?

crisner · 2020-01-08T15:01:40Z

Yes, sounds good. :)

jywarren · 2020-01-08T15:07:28Z

publishing!!

jywarren · 2020-01-08T15:09:04Z

Sorry, do we need to do any grunt build on this before publishing? if so, i can just push out a 2.1.7?

crisner · 2020-01-08T15:11:05Z

Yes, I guess. We have git ignored the build file. So I think we have to run grunt build before publishing.

crisner · 2020-01-08T15:12:28Z

if so, i can just push out a 2.1.7

Sounds good.

jywarren · 2020-01-08T15:12:36Z

ok, no prob - 2.1.7 coming up. I can release it fast enough that it'll supercede 2.1.6

jywarren · 2020-01-08T15:14:55Z

done!

jywarren · 2020-01-08T15:15:02Z

now doing gh-pages

jywarren · 2020-01-08T15:21:22Z

It's live! Let's test it out, then let's watch at plots2 for dependabot to bump the LEL version in a PR there... i can give dependabot a nudge from its website console too...

jywarren · 2020-01-08T15:24:29Z

In the demo, how should I test this layer out? AHa! https://publiclab.github.io/leaflet-environmental-layers/example/#lat=41.531&lon=-96.542&zoom=6&layers=Standard,fracTrackerMobile,Power

jywarren · 2020-01-08T15:36:53Z

This is so great. I'm finding some things I'd like to make follow-ups for; is there a place to put these for now?

More layers (like FTM) in menu? - so before we publish, the current demo shows only a few of the layers... this wouldn't get copied into plots2 yet without more integration, right? I just don't want to publish on plots2 before all the existing layers are included. Thanks!
Permalinks for popups? Like, if you open a popup maybe we can have a # button that links you back to this place AND it opens that same popup... this could be tough but useful... i don't see how we have a unique id here, but could we do it based on the layer and... timestamp? lat/lon? This might need some brainstorming.
Less urgent: ways to show when a layer /will be active/ for the given bounding box - do we highlight it, do we have a tooltip to explain, do we show a bounding box when people hover on the layer?

This looks great!

https://publiclab.github.io/leaflet-environmental-layers/example/#lat=40.0333&lon=-75.1557&zoom=10&layers=Standard,fracTrackerMobile,skytruth,toxicReleaseLayer,mapknitter,Power

jywarren · 2020-01-08T15:43:51Z

So nice!

crisner · 2020-01-08T15:47:49Z

I am getting a Uncaught ReferenceError: Spinner is not defined when I try to access the above links.

jywarren · 2020-01-08T15:48:33Z

https://publiclab.github.io/leaflet-environmental-layers/example/#lat=40.1050&lon=-75.6771&zoom=14&layers=Standard,fracTrackerMobile,skytruth,toxicReleaseLayer,mapknitter,Power,Petroleum,Water

Some really nice side-by-side of reports and SkyTruth alerts for what's probably the same site here!

jywarren · 2020-01-08T15:48:59Z

Hmm. Would spinner be an npm req? I included all new npm modules, i think!

crisner · 2020-01-08T15:51:35Z

Yes. Looks like it is not included in the node modules.

GET https://publiclab.github.io/leaflet-environmental-layers/node_modules/leaflet-spin/example/spin/dist/spin.min.js net::ERR_ABORTED 404

jywarren · 2020-01-08T15:52:10Z

I'll hold on bumping dependabot until we're sure here, so give me a heads up! Thanks for everything, this is so exciting :-)

Oh, one more idea! (sorry to dump them here, let's find a place to put them, and we don't have to act on all of these, they're just inspirations while I'm using it) -
- So: We could have a url hash flag to have the menu expanded?

OK i'll try to include it in gh-pages and republish that.

crisner · 2020-01-08T15:53:19Z

Looks like blurred location and blurredlocationDisplay are also missing.

jywarren · 2020-01-08T15:57:09Z

Hm. That spin file is not present here: https://github.com/publiclab/leaflet-environmental-layers/tree/gh-pages/node_modules/leaflet-spin/example/spin

is it supposed to be part of the npm package as pulled in? Is it being gitignored?

crisner · 2020-01-08T15:57:17Z

Oh, one more idea! (sorry to dump them here, let's find a place to put them, and we don't have to act on all of these, they're just inspirations while I'm using it)

I have a few ideas myself. 😅 I was planning to note them all in a planning issue to discuss it first thing tomorrow morning. Maybe you could open a planning issue and put them there? and then I could add on to it?

We could have a url hash flag to have the menu expanded?

Awesome idea.

crisner · 2020-01-08T15:59:13Z

Hm. That spin file is not present here: https://github.com/publiclab/leaflet-environmental-layers/tree/gh-pages/node_modules/leaflet-spin/example/spin

is it supposed to be part of the npm package as pulled in? Is it being gitignored?

Let me check

jywarren · 2020-01-08T16:01:33Z

OK awesome -- would you mind opening this list issue? That'd be super - and isn't our meeting Friday morning, not tomorrow?

crisner · 2020-01-08T16:03:14Z

Yes. Friday morning, your time.

crisner · 2020-01-08T16:05:45Z

would you mind opening this list issue?

Yes. Shall do it.

Btw, the dist file exists in the node modules in the local repo and node modules are git ignored in other branches anyway.

crisner · 2020-01-08T16:06:18Z

Sorry in the local machine. The file exists in my local copy.

crisner · 2020-01-08T16:10:37Z

And the file exists in the gh-pages branch of my copy https://github.com/crisner/leaflet-environmental-layers/tree/gh-pages/node_modules/leaflet-spin/example/spin/dist

The branch is not updated with the latest commits though. May be running npm install again would fix it?

crisner · 2020-01-08T16:21:53Z

I have opened an issue, #334 as a place to add and discuss new ideas. Shall edit the post with a few of my ideas and description tomorrow.

jywarren · 2020-01-08T19:58:39Z

You're right, I had missed some modules! Pushing them now.

jywarren · 2020-01-08T21:13:47Z

And regarding layers, it now appears to load all layers! Maybe it was the missing modules that stopped it.

crisner · 2020-01-09T06:05:25Z

I guess this resolves this comment? I'd like to note that unearthing layer is not included. Shall we include that too in the demo?

jywarren · 2020-01-09T14:52:57Z

Oh, that would be great. And yes! Thanks!

…

On Thu, Jan 9, 2020, 1:05 AM Renisha Christie. A ***@***.***> wrote: I guess this resolves this comment <#320 (comment)>? I'd like to note that unearthing layer is not included. Shall we include that too in the demo? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#330?email_source=notifications&email_token=AAAF6JZIBSBZTI3VVD7K76LQ425CNA5CNFSM4KD3UZ3KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIPDFXQ#issuecomment-572404446>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J2SSIJZLMNECU6VOPDQ425CNANCNFSM4KD3UZ3A> .

Implement changes to fractracker mobile layer

532972a

crisner mentioned this pull request Jan 7, 2020

Outreachy project planning publiclab/plots2#6887

Closed

sagarpreet-chadha reviewed Jan 7, 2020

View reviewed changes

jywarren approved these changes Jan 8, 2020

View reviewed changes

Update layer in AllLayers.js and refactor code

af5e51f

crisner changed the title ~~[WIP] Implement changes to fractracker mobile layer~~ Implement changes to fractracker mobile layer Jan 8, 2020

crisner added 2 commits January 8, 2020 12:58

Update layer name change in layers.js

a2698f2

Add basic tests

046483e

jywarren merged commit f10a792 into publiclab:master Jan 8, 2020

jywarren mentioned this pull request Jan 8, 2020

Bump leaflet-environmental-layers from 2.0.6 to 2.1.7 publiclab/plots2#7147

Closed

crisner mentioned this pull request Feb 4, 2020

Outreachy Project Recap: summary, planning & discussion publiclab/plots2#7432

Closed

Implement changes to fractracker mobile layer #330

Implement changes to fractracker mobile layer #330

Conversation

crisner commented Jan 7, 2020 • edited Loading

crisner commented Jan 7, 2020

crisner commented Jan 7, 2020

sagarpreet-chadha Jan 7, 2020

Choose a reason for hiding this comment

crisner Jan 7, 2020

Choose a reason for hiding this comment

crisner Jan 7, 2020

Choose a reason for hiding this comment

jywarren Jan 8, 2020

Choose a reason for hiding this comment

sagarpreet-chadha commented Jan 7, 2020

crisner commented Jan 7, 2020

jywarren left a comment

Choose a reason for hiding this comment

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

crisner commented Jan 8, 2020

jywarren commented Jan 8, 2020

jywarren commented Jan 8, 2020

crisner commented Jan 9, 2020

jywarren commented Jan 9, 2020 via email

crisner commented Jan 7, 2020 •

edited

Loading