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

Enable limiting location autocomplete to a certain country #1002

Merged
merged 6 commits into from
Jan 18, 2019

Conversation

OtterleyW
Copy link
Contributor

@OtterleyW OtterleyW commented Jan 17, 2019

Limit location autocomplete by adding an optional country parameter to geocoding call in both Mapbox and Google Maps integrations. Variable used to enable this is config.maps.search.limit and it can be found from config.js file. The same variable works for Mapbox and Google Maps integrations.

Also updated Mapbox SDK to version 0.5.0.

src/config.js Outdated
// Limit location autocomplete to a one or more countries
// using ISO 3166 alpha 2 country codes separated by commas.
// If you don't want to limit the autocomplete you can comment out this value.
limit: ['AU'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe changing the name to countryLimit

Copy link
Contributor

Choose a reason for hiding this comment

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

And don't leave this countryLimit active!

*
* @return {Promise<{ search, predictions[] }>} - Promise of an object
* with the original search query and an array of
* `google.maps.places.AutocompletePrediction` objects
*/
export const getPlacePredictions = (search, sessionToken) =>
export const getPlacePredictions = (search, sessionToken, restriction) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

It is probably better to rename this to searchConfigurations (or something like that) since there are more configurations that could be passed in than just componentRestrictions

const limitCountriesMaybe = config.maps.search.limit
? { country: config.maps.search.limit }
: {};
const restriction = { componentRestrictions: { ...limitCountriesMaybe } };
Copy link
Contributor

@Gnito Gnito Jan 17, 2019

Choose a reason for hiding this comment

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

Let's not send a key componentRestrictions if there is no countryLimit set.

 const limitCountriesMaybe = config.maps.search.limit
      ? { 
           componentRestrictions: {
             country: config.maps.search.limit,
           },
         }
      : {};

Then limitCountriesMaybe could be spread directly to the call:

return getPlacePredictions(search, this.getSessionToken(), limitCountriesMaybe).then(results => {

Copy link
Contributor

@Gnito Gnito left a comment

Choose a reason for hiding this comment

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

LGTM

@OtterleyW OtterleyW merged commit a3d8963 into master Jan 18, 2019
@OtterleyW OtterleyW deleted the limit-location-autocomplete branch January 18, 2019 10:56
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.

2 participants