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

Don't call getGeocode from getRoute #5083

Conversation

AntonKhorev
Copy link
Collaborator

Part of #5064.

getRoute shouldn't know anything about geocoding, doesn't need to wait for every kind of geocoding (namely for reverse one), therefore shouldn't command the endpoints to do it. All necessary geocoding is triggered by endpoint.setValue and input event listeners.

hasGeocode didn't actually indicated if an endpoint has a value acquired from a geocoder. It was set to true when at least one call to a geocoder finished successfully, including getting an empty result. getRoute doesn't need to know what calls were made, it only needs coordinates. It also doesn't get to decide when to fetch coordinates, this is done in drag/change callbacks.
@tomhughes
Copy link
Member

tomhughes commented Aug 16, 2024

Is there some reason for pulling apart #5064 into fragments? I know I skipped reviewing that one - that was because it was a bit larger so I was going to leave it to the weekend...

@AntonKhorev
Copy link
Collaborator Author

AntonKhorev commented Aug 16, 2024

If you apply Remove latlng parameter from endpoint.setValue, you'll have to apply the entire #5064, otherwise reverse geocoding will stop working. But then you may run into precision problems if you don't do also #5084.

If you just merge this pull request, reverse geocoding behavior won't change.

@tomhughes
Copy link
Member

What is Remove latlng parameter from endpoint.setValue though? I don't see any PR called that?

@AntonKhorev
Copy link
Collaborator Author

First commit in #5064: b9f8057

@tomhughes
Copy link
Member

Looks good to me, thanks.

@tomhughes tomhughes merged commit 41b81bd into openstreetmap:master Aug 18, 2024
24 checks passed
@AntonKhorev AntonKhorev deleted the directions-endpoint-private-get-geocode branch August 19, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants