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

Updates Geocoding to match API #217

Merged
merged 2 commits into from
Nov 30, 2016
Merged

Updates Geocoding to match API #217

merged 2 commits into from
Nov 30, 2016

Conversation

cammace
Copy link

@cammace cammace commented Nov 23, 2016

We are missing a few options/parameters the geocoder gives you. This PR adds limit and exposes country in the auto complete widget. Still need to add landmark.

cc: @zugaldia

@mention-bot
Copy link

@cammace, thanks for your PR! By analyzing the history of the files in this pull request, we identified @zugaldia, @ivovandongen and @sbma44 to be potential reviewers.

@cammace
Copy link
Author

cammace commented Nov 28, 2016

Still need to add landmark.

This is a property and can be acquired by the user using feature.getProperties().get("landmark").getAsBoolean().

@cammace
Copy link
Author

cammace commented Nov 28, 2016

Could I get someone on @mapbox/android for review :)

* country codes, separated by commas.
* @since 2.0.0
*/
public void setCountry(String country) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to offer a varargs method here for convenience so that the comma separated format is abstracted:

public void setCountries(String... countries)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great point @ivovandongen, adding.

@sbma44
Copy link
Member

sbma44 commented Nov 29, 2016

Thanks a ton for tackling this -- we're a bit thin on Android chops on the geocoding team.

This is a property and can be acquired by the user using

landmark is both a property on the response and (part of) a valid value for the types param. e.g. types=place,poi.landmark

@cammace
Copy link
Author

cammace commented Nov 29, 2016

@ivovandongen: I've added a countries option which takes in a String array, array was used here since it was already implemented that way in the libjava countries option.

@sbma44 Happy to help out on the geocoding Android side of things. For types, we allow poi.landmark to be passed in to filter the results.

Copy link
Contributor

@ivovandongen ivovandongen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cammace cammace merged commit 9dd43f8 into master Nov 30, 2016
@cammace cammace deleted the cam-geocoding-widget branch November 30, 2016 13:29
@zugaldia zugaldia mentioned this pull request Feb 10, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Feb 22, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 9, 2017
9 tasks
@zugaldia zugaldia mentioned this pull request Mar 17, 2017
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants