Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add layers in include to map by default #352

Merged

Conversation

crisner
Copy link
Contributor

@crisner crisner commented Jan 16, 2020

Fixes #349 (<=== 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
Copy link
Contributor Author

crisner commented Jan 16, 2020

Work done:

  • Added Unearthing layer to demo
  • Fixed a bug where the points from the Unearthing layer weren't removed from the map after unchecking the layer on the layers control
  • Fixed code to add all layers to map upon initialization if layers are passed through the include param
    • Now all layers would be visible on the map upon initialization
    • The status of the layers cannot be tracked for embed purposes. All the layers will always will be loaded on the map even if the embed code does not have the layers in its hash

Screenshot of unearthing-pvd.html upon initialization:

127 0 0 1_5500_example_unearthing-pvd html (1)

@crisner
Copy link
Contributor Author

crisner commented Jan 16, 2020

@jywarren, It looks like all layers are passed into the include param for the map page, publiclab.org/map here:
https://github.com/publiclab/plots2/blob/46af8030c82fe8346702edbaf917bfeb9e229163/app/views/map/map.html.erb#L104

We should either remove that line or add an extra option param to indicate whether we need the included layers to be displayed upon initialization.

@crisner
Copy link
Contributor Author

crisner commented Jan 16, 2020

Also, it looks like all layers are not included in the map page here: https://publiclab.org/map
Was then an intended change @jywarren, @sagarpreet-chadha, @nstjean ?

@nstjean
Copy link
Contributor

nstjean commented Jan 16, 2020

I didn't do anything with the layers on that page, so I think it wasn't complete yet and they need to be added.

@jywarren
Copy link
Member

Great work!!! I'll bump the version number and publish this!

@jywarren
Copy link
Member

Er, sorry - is this ready to be merged? Given that it's causing an issue on the live site, I'd like to prioritize the narrower fix now, and if you'd like to expand on it in a follow-up, that's great!

Thanks for responding so quickly to this!

Also, it looks like all layers are not included in the map page here: https://publiclab.org/map

I'm not sure, but that page is still new - i think it should have the new menu and all available layers!

Thank you!

@crisner
Copy link
Contributor Author

crisner commented Jan 16, 2020

Well, as a narrower fix it is ready to merge. This may probably need a follow up when I implement the new menu. Thanks!

@jywarren jywarren merged commit 38c1c65 into publiclab:master Jan 16, 2020
@jywarren
Copy link
Member

Thank you!!!

@crisner
Copy link
Contributor Author

crisner commented Jan 17, 2020

@jywarren, I see this has been published! 🎉
I bumped LEL in my forked copy of the plots2 repo to check if the changes have been reflected correctly and it indeed has! Awesome! 🎉

  • checked embed code in /map
  • checked if layers are displayed by default
  • checked for errors from LEL in the console

@crisner
Copy link
Contributor Author

crisner commented Jan 17, 2020

Also, I checked the issue with the fontawesome icon not displaying on the map. Installing this package: https://www.npmjs.com/package/@fortawesome/fontawesome-free, and adding the stylesheet to the application.html.erb does the trick. If you are okay with this I could perhaps open an FTO issue on plots2.

@@ -133,6 +137,14 @@ L.LayerGroup.environmentalLayers = L.LayerGroup.extend(
// Update map state from hash
hash.update(this.options.currentHash);
}

for (let layer of this.options.layers.include) {
if (!this.options.addLayersToMap) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did this logic got removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jywarren
Copy link
Member

jywarren commented Jan 17, 2020 via email

sagarpreet-chadha added a commit that referenced this pull request Mar 25, 2020
* Remove use of hardcoded extents for displaying layers (#332)

* Fix embed code with duplicated text (#340)

* Fix duplication of text in embed code

* Update build file

* Update README.md (#335)

* Update README.md

* Update README.md

* Add check that layers group is not undefined when creating layers-sub-list (#339)

Fixes #333

* Display new items mark in layers menu (#341)

* Create function to check for available layers

* Display badge to show new items

* Save map instance in a variable

* Update readme

* Add contribute button to open infra map

* Add media queries for size 235x300 (#346)

* Add default basemap when undefined (#351)

* Add default basemap when undefined

* Update version number indicating bug-fix

* [WIP] Add layers in include to map by default (#352)

* Add unearthing layer to demo

* Fix bug with removing points from the map on removeLayer

* Add all layers in include param upon map initialization

* bump to v2.1.9

* Implement new menu

* Add new menu dependancies

* Fix styles from overriding default leaflet classes

* Refactor code

* Fix styles for layer group name and layer description

* Add option to disable layer visibility when param.include is true

* Add option to use default layer control

* Fix errors when trying to display layer when out of bounds

* Add missing viewport meta tag

* Move styles to LeafletEnvironmentalLayers.css

* Update documentation

* Add missing dependencies and update build

* Fix error from unearthing layer on page reload

* Set addLayersToMap options from param

* Update documentation

* Update AllLayers.js (#366)

Fixed: Prevent use of the same default tile layer for multiple maps

* Add PLpeopleLayer (#361)

* add PLpeopleLayer to maps

* fix function name to PLpeopleLayer

* fix capital in function name

* make naming consistent for both map examples

* change layer name to PLpeople

* version 2.2.0 and bump LBL to 1.7.0 (#368)

* Feature/minimal mode (#356)

* Create toggle mode control

* Add marker for minimal mode

* Change markers on mode change

* Add condition and marker styles for eonet fires layer

* Add minimal mode content to layers in layercode

* Add minimal mode content to aqicn layer

* Add minimal mode option

* Refactor minimalMode control

* Add mode control to one liner code

* Add documentation to readme

* Fix duplication of layers control, set simple layer control for demo

* Add basic tests

* Update build

* 2.2.0

* Change button names and title

* Update tests for mode control

* Update build

* Add demo with multiple maps in a single page

* Add a check for jQuery and update build

* Bug/fix missing layers in layer menu on refresh (#375)

* Fix 'moveend' event check to affect only when removing layers from map

* Add a check when adding minimal markers to confirm this._map exists

* Update build

* fix conflict

* add parameter display to turn on layers

* check if layer exists before turning it on

* Remove conditions and code responsible for jquery duplication

* Update link to jquery 3.3.1 and remove references to older versions

* Move info.json outside iife and update build

* Install cypress

* published

* Bug/fix layers not removed from map when hash false (#380)

* Add error handling on removing unearthing from map

* Remove redundant code

* Comment out unused baselayer instantiation and update build

* Uncomment baselayers

* Reformat init defaults (#396)

* rewrite init defaults

* remove console.log

* use L.Util.setOptions to set defaults

* Add Cypress

* Add tests to check leaflet left controls

* Add tests for embed prompt and layers menu

* Add child window to prevent iframe conflict on hash change

* Add test to check hash update on map movement

* Move hash test to a seperate file and increase delay

* Set up http server for localhost

* Ignore cypress screenshot and video folders

* Set up cypress in travis

* Oneliner/basic implementation tests (#397)

* testing all the specs

* using babel

* add layer name id to broswer menu

* leaflet not fully loading

* recent tests

* testing error in multiple maps

* rewrite init defaults

* basic one liner tests work

* test shorter transpile files

* emove jquery from gruntfile browserify dist

* use L.Util.setOptions to set defaults

* turn on basic tests again

* disable babel-plugin-transform-object-rest-spread

* clean up

* rebase

* add wisconsin layer back in and fix error

* remove text-table files

* add css file back in

* fix merge errors

* Edit and update documentation for more clarity (#367)

* Add dependencies to documentation

* Add updated documentation to readme

* Add more information

* Edit usage example for layer menu

* Added links to more information

* Update minimal mode documentation

* Add files via upload

* added a layerData folder containing the json file for sea level rise data

Co-authored-by: Renisha Christie. A <29401459+crisner@users.noreply.github.com>
Co-authored-by: Suyash Choudhary <57896905+sssash18@users.noreply.github.com>
Co-authored-by: llabake <lemboyelabake@gmail.com>
Co-authored-by: crisner <christie.reni@gmail.com>
Co-authored-by: Sagarpreet Chadha <chadha.sagarpreet97@gmail.com>
Co-authored-by: jywarren <jeff@unterbahn.com>
Co-authored-by: z-arnott <50159450+z-arnott@users.noreply.github.com>
Co-authored-by: Natalie St Jean <49460529+nstjean@users.noreply.github.com>
Co-authored-by: Natalie <natalie.stjean@gmail.com>
Co-authored-by: Cess <cessmbuguar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unearthing demo not visible in publiclab.org
4 participants