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 #1374

Conversation

NSUWAL123
Copy link
Collaborator

@NSUWAL123 NSUWAL123 commented Mar 21, 2024

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

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

Related Issue

Also solves #1343

Describe this PR

This PR covers the following things:

  1. Remove geolocation from projectListMap component(HomePage)
  2. Generate Mbtiles btn shift to bottom for mobile view
  3. Only ask permissions if the geolocation status is turned on

A brief description of how this solves the issue.

Screenshots

1711010921207_720

Alternative Approaches Considered

Did you attempt any other approaches that are not documented in code?

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

@spwoodcock , @varun2948 . It seems that some of the browser doesn't support AbsoluteOrientationSensor.
That's why Sam's laptop was showing page crash error. I think we need another workaround to get phone's quaternion.
image

Error on firefox:
image

Absolutely working fine on chrome:
image

@varun2948
Copy link
Collaborator

We mostly used this in mobile and most of the android is based on chromium and chrome so i think it will not be that much of a problem. What's your say @spwoodcock ?

@spwoodcock
Copy link
Member

Damn, didn't realise that!

I thought the error on Firefox was because of my privacy settings / disabled geolocation.

At the very least we shouldn't have an error if not available. It should be caught gracefully and displayed to the user that no geolocation is available.

This will be perfect for the demo, but we probably also need to support Safari in the future too (I'll merge this and make an issue for another PR)

@spwoodcock spwoodcock merged commit aec9fb3 into hotosm:development Mar 22, 2024
3 checks passed
@NSUWAL123
Copy link
Collaborator Author

@spwoodcock, fixed the page crash error on another PR

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