search

TheDevPath / Navi

Open Source Project for Grow with Google Udacity Scholarship Challenge - Navigation app using offline first strategy and google maps api - To get started please refer to the README.md - CONTRIBUTING.md and the project Wiki
https://navi-rocks.herokuapp.com/
MIT License
53 stars 63 forks source link

Home busy message #308

Closed someyoungideas closed 6 years ago

someyoungideas commented 6 years ago

Issue Number:

302

Issue Description:

Creating some visual indication to the user while Navi loads their geolocation, remove message after geolocation is found, and update if errors occur. Also, update Search component to not block searching if geolocation is denied or while loading.

Summary of solution:

  1. Create busyMessage in home route state
  2. Create _setBusyMessage method to update state while attempting to get geolocation
  3. Setting default userPosition with empty lat and lng properties
  4. Checking for lat and lng in google-api-controller.js and if it doesn't exist use default location

Can this issue be closed?

No. Still need to discuss how to handle Search component and the google-api-controller autocomplete endpoint to not error if geolocation is not enabled.

Should any new issues be added as a result of this solution?

No

Have you named your branch in a descriptive way? Remember to name your branch in a unique and descriptive manner in order to properly reflect the issue or feature.

Thanks for contributing!

someyoungideas commented 6 years ago

@motosharpley @jkwening there is a decision I made here that could be up for discussion. I set the default location in the google-api-controller to Lawrence, KS. I did this just because I knew it is was the original center of Google Maps. It is a college town close by and knew of the odd fact haha. This could be handled in the Search component on the front end if you would prefer, or it could stay as is. I can also update the location to somewhere that is more meaningful to the original creator(s) of this repo if you'd like.

motosharpley commented 6 years ago

I think this might be a good point to look at a new feature branch to work out how we want to handle user settings allowing registered users to set a default location and using an ultimate default like this might be a good way to handle some worst case scenarios

someyoungideas commented 6 years ago

@motosharpley would you like the default location to be removed for now, and be a separate issue? Or would you like this current default location to stay in place and create a new issue to start a discussion about a default location in a user settings?

motosharpley commented 6 years ago

Ya I think we should figure out how we want to handle the search feature before we set default location since currently we use the location to influence search suggestion, I think it might be better to refactor search to be more flexible first and then address the default location issue.

someyoungideas commented 6 years ago

@motosharpley alright I removed the change to the google-api-controller.js endpoint. I also updated the description slightly because my original issue was discussing handling errors with the Search component if the user does not have geolocation enabled. This will still require some discussion on how it should be handled. I got my formatting setup to not override the entire file spacing as well, but it took me a couple go arounds unfortunately.

motosharpley commented 6 years ago

@someyoungideas sorry to push that last merge over yours but I wanted to get the bug fix in first, can you just check that the bug fix didn't affect any of your changes. Thanks