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

Feat project details geolocation integration #1370

Conversation

NSUWAL123
Copy link
Collaborator

@NSUWAL123 NSUWAL123 commented Mar 20, 2024

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Related Issue

Describe this PR

This PR integrates the current working geolocation to the project details page and making geolocation function reusable.

Screenshots

1710936304510_720

Alternative Approaches Considered

Edit from @spwoodcock :

  • First we used capacitor, but the implementation uses the web api for mobile anyway, plus it didn't seem to work well.
  • Next we tried a few other libraries that worked ok, but were not very confovurable.
  • The final solution we landed on was returning to first principals: calculating the rotation direction using the quatarnion returned via native web apis.

Review Guide

Notes for the reviewer. How to test this change?

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

@NSUWAL123
Copy link
Collaborator Author

I have made a function in utilsFunction to for geolocation and dumped the previous capacitor geolocation and added current implementation of the geolocation on projectListsMap.
However, I have not removed the code on projectListsMap as this implementation also needs to be tested.

@varun2948 varun2948 merged commit e9ecfdd into hotosm:development Mar 20, 2024
3 checks passed
@spwoodcock
Copy link
Member

spwoodcock commented Mar 20, 2024

Neat! Very cool integrating the quaternion code 😁

I'll merge and we can test on dev πŸ‘
(@varun2948 beat me to it πŸ˜†)

@varun2948
Copy link
Collaborator

hahaha @spwoodcock i was looking over to this PR thoroughly so did it the moment i finished reviewing :D

@varun2948
Copy link
Collaborator

@NSUWAL123 btw have you checked, if the GPS is turned off it causes issue.

@spwoodcock
Copy link
Member

Yep same in my browser, Firefox with GPS disabled:

Home screen:
image

@spwoodcock
Copy link
Member

Screenshot_20240320-214805.png

And then when I decline location on my phone.

Both with the permissions I think.

We probably need a check:

if ('geolocation' in navigator) {}

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

Successfully merging this pull request may close these issues.

None yet

3 participants