-
Notifications
You must be signed in to change notification settings - Fork 326
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
fix(geocoder): Propogate geocoder error to consumer #487
base: main
Are you sure you want to change the base?
Conversation
closes ubilabs#486 Add a new prop to propogate errors from the Geocoder.geocode API to the consumer application
@yfr hey, can you review this when you get the time? Our Sentry is full of these errors and thought it would be great to have a fix in the upstream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. We had a lot on our plates but i managed to review now.
Just two small changes.
const location = (gmaps?.geometry && | ||
gmaps.geometry.location) as google.maps.LatLng; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const location = (gmaps?.geometry && | |
gmaps.geometry.location) as google.maps.LatLng; | |
const location = gmaps?.geometry?.location as google.maps.LatLng; |
const location = (gmaps?.geometry && | ||
gmaps.geometry.location) as google.maps.LatLng; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const location = (gmaps?.geometry && | |
gmaps.geometry.location) as google.maps.LatLng; | |
const location = gmaps?.geometry?.location as google.maps.LatLng; |
@prateek3255 would you like to update this pull request according to @yfr's comments, so we can get this done? |
Description
closes #486
Checklist