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

Fix bugs caused by layer id conflict #443

Merged

Conversation

crisner
Copy link
Contributor

@crisner crisner commented Mar 6, 2020

Fixes #442 (<=== Add issue number here) &&
Fixes #441

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 Mar 6, 2020

The bugs when using multiple maps in a single page was because there was more than one id with the same name. Refactored all id names to include the id of the map container. This solved the issue #442 and #441.

Screenshot after the fix:
oimBug

@crisner
Copy link
Contributor Author

crisner commented Mar 6, 2020

I caught another weird bug where the OIM layers from one map were interfering with the OIM layers in the other map and removed the error icons from other layers in the first map when one of the OIM layers were clicked. This has also been fixed.

@@ -61,6 +37,31 @@ L.LayerGroup.environmentalLayers = L.LayerGroup.extend(

this.options.layers = param;

this._OpenInfraMap_Power = L.tileLayer('https://tiles-{s}.openinframap.org/power/{z}/{x}/{y}.png', {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am a bit worried about moving these layers here but this seems to fix the issue of cross-wiring between layers when we have multiple maps on the page as mentioned here #443 (comment)

It's hard to test how these layers behave when added to the map because they don't work anymore. We could also comment them out till it is fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok -- and noting #387 as the bug to fix for that. Thanks!

@crisner
Copy link
Contributor Author

crisner commented Mar 6, 2020

There are a couple of failing tests. I'll look into them fixed later tonight or tomorrow.

@crisner crisner force-pushed the fix-bugs-caused-by-layer-id-conflict branch from a1e7dc3 to d2d4e3e Compare March 6, 2020 14:03
@crisner crisner changed the title Fix bugs caused by layer id conflict [WIP]Fix bugs caused by layer id conflict Mar 8, 2020
@crisner crisner force-pushed the fix-bugs-caused-by-layer-id-conflict branch from eee9ce2 to 27f5e73 Compare March 8, 2020 16:10
@crisner
Copy link
Contributor Author

crisner commented Mar 8, 2020

image
image
example_multipleMaps html(Galaxy S5)

@crisner crisner changed the title [WIP]Fix bugs caused by layer id conflict Fix bugs caused by layer id conflict Mar 8, 2020
@crisner
Copy link
Contributor Author

crisner commented Mar 8, 2020

@jywarren, @sagarpreet-chadha, this PR fixes bugs from recent changes, fixes tests that were causing errors in the master branch and is ready for review and merge. Thanks!

@crisner crisner force-pushed the fix-bugs-caused-by-layer-id-conflict branch from 27f5e73 to 1d39d9c Compare March 9, 2020 14:43
@jywarren jywarren merged commit cd4d82f into publiclab:master Mar 9, 2020
@jywarren
Copy link
Member

jywarren commented Mar 9, 2020

Great work!!

@jywarren
Copy link
Member

jywarren commented Mar 9, 2020

Thank you!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants