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

Fix location not clearing #231

Closed steveone closed 6 years ago

steveone commented 6 years ago

Issue Number: 226

Issue Description: If you click on multiple points of the map, each location will get a pin without you hitting "save" on the pins. The expected behavior is that one pin will be created, and if you don't hit "save" the pin will be removed when you click elsewhere on the map.

Summary of solution:

  1. Added "unSavedMarker" to components local state to track unsaved markers
  2. When a new marker is created, the old marker is removed

Can this issue be closed?

Additional code needed to clear "unSavedMarker" in the state when a marker is successfully saved, that feature is throwing an error (in Chrome Canary) so testing was not possible

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

Saving a marker throws an error Uncaught (in promise) TypeError: Failed to fetch xhr.js....

screen shot 2018-03-12 at 9 02 35 pm

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!

steveone commented 6 years ago

I only updated one file "client/src/components/LeafletOsmMap/index.js" so I'm not sure why it shows 20 commits. If I did something wrong, if someone could please let me know I would be happy to correct it.

motosharpley commented 6 years ago

@steveone it looks like you may have pulled from the master branch that is why you ended up with all the dist/ files and related commits. We are using development as the default branch so you should pull from there to bring your local repo up to date. The master branch is being used to serve an automated deployment and that is why it has all the dist/build assets included.

steveone commented 6 years ago

@motosharpley I will do an update tonight and try to fix the issue. Thank you for your guidance.