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

Fixed OSM layer loading and checkbox problem #202

Merged
merged 2 commits into from
Jun 12, 2019

Conversation

ananyaarun
Copy link
Member

@ananyaarun ananyaarun commented Jun 10, 2019

Fixes #139

  • 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

osm

Thanks!

@ananyaarun
Copy link
Member Author

ananyaarun commented Jun 10, 2019

@sagarpreet-chadha

  • I have removed the requirement of minZoom = 7 for the OSM layer
  • The layer now loads (checkbox works) for all zoom values. At the same time it takes way to long to load for a full map but on zoom loads quickly as shown in the gif.
  • I have also added a (zoom in ) like pressure contour layer to tell users that this layers works better on zoom

@ananyaarun
Copy link
Member Author

I have resolved the conflicts , do take a look @sagarpreet-chadha . Thanks !! :)

@jainaman224
Copy link

@sagarpreet-chadha I think we can add dist/LeafletEnvironmentalLayers.js to .gitignore.

@sagarpreet-chadha
Copy link
Collaborator

Hey Aman , great idea 😄 . We though about this and we decided to maintain dist file, like in plots2 or mapknitter we donot want to use any command to build dist file . Agreed we can make a script which can automatically make dist file after npm install LEL is executed . We can open a separate issue for this . Thanks :)

Copy link
Collaborator

@sagarpreet-chadha sagarpreet-chadha left a comment

Choose a reason for hiding this comment

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

Hey @ananyaarun , i am merging this for now . But we have to tackle the lagging of map in full screen due to large parsing of data . Let'a open a new issue for this and we can take help from @jywarren over there . Thank you! Great 😄

@sagarpreet-chadha sagarpreet-chadha merged commit 6c157db into publiclab:master Jun 12, 2019
@ananyaarun
Copy link
Member Author

Sure, i'll open an issue for that. Thanks!!

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.

OSM Landfills, Mines and Quarries checkbox not working.
3 participants