SCODEMeetup / community-services-locator-ui

Building a UI for community locator services
MIT License
1 stars 7 forks source link

Center map to user's location and show a map marker #49

Closed mycarrysun closed 4 years ago

mycarrysun commented 5 years ago

Changes

Quick question...do we need to delay the marker by 3 seconds? Why do we need the setTimeout when the map mounts?

vijayyadav06 commented 4 years ago

Changes

  • Try to get the user's location when component is mounted
  • Use a new marker icon to indicate the user's current location
  • Center the map to the user's current location

Quick question...do we need to delay the marker by 3 seconds? Why do we need the setTimeout when the map mounts?

I guess @mihiramin89 may be adding the animation when the pins are dropping to add a delay https://developers-dot-devsite-v2-prod.appspot.com/maps/documentation/javascript/examples/marker-animations-iteration Although it seems like it just delay placing any pins by 3 seconds.

vijayyadav06 commented 4 years ago

Code change looks good, calling out two observations, (no code change needed) -

  1. prompt for current location
  2. In chrome when "allowed" location to be tracked. It zoomed into into my location and being in lewis center I didnt see any pins until i zoom out. FF doesn't zoom in like Chrome.

I think this is acceptable behavior.

mihiramin89 commented 4 years ago

I do not remember the reason I added a 3 second delay for placing the markers. I say lets get rid of it. Tested it locally and I do not see a need for the delay.

mihiramin89 commented 4 years ago

We have tried this implementation before (maybe not this specifically with the arrow functions) but I remember it working locally but not when we deployed and tested on a mobile device. Can we confirm this will work when deployed?

mycarrysun commented 4 years ago

We have tried this implementation before (maybe not this specifically with the arrow functions) but I remember it working locally but not when we deployed and tested on a mobile device. Can we confirm this will work when deployed?

What error were you getting on mobile devices? I can do a remote debugging session on my phone to see if it works as expected

Edit: There is a fallback if the user's location cannot be retrieved then it will stay at the center of Columbus like it was before