Generic location autocomplete #893
Conversation
kpuputti
changed the title
kpuputti
force-pushed
the
generalised-geocoding
branch
from
4e318f6
to
107328c
Compare
| // `false` when using some other Geocoding API. Autocomplete | ||
| // predictions dropdown bottom padding might need adjusting as well to | ||
| // hide the space left for the logo. | ||
| const SHOW_POWERED_BY_GOOGLE = true; |
There was a problem hiding this comment.
Would it be safer to use config.googleMapsAPIKey check somehow here? Especially if we end up making mapbox our default map provider? (Missing to turn this on/off is a clear violation of their terms.)
There was a problem hiding this comment.
Even better would be that the whole div (or just the flag) is exported from Geocoder file.
There was a problem hiding this comment.
Could be better but a config var might make customisers think that it is enough to change the provider.
I think a config still makes sense, then we can already adjust the predictions list paddings etc. based on that. We just have to mention that some imports have to be changes too.
There was a problem hiding this comment.
I moved the attribution element to the geocoder.
However, the predictions list leaves bottom padding for the attribution, and that padding is added in many places. I added a global var that can be used to set the padding when the map provider is changed.
| * [change] Removed the `country` parameter from the search page as it | ||
| was not used anywhere. | ||
| [#893](https://github.com/sharetribe/flex-template-web/pull/893) | ||
|
|
There was a problem hiding this comment.
The changelog could contain even multiple entries for this PR. At least for adding the Mapbox geocoder and maybe also a note about the generalization of the geocoder.
There was a problem hiding this comment.
Is it ok not to advertise it yet? It's not usable without the script imports, and it would be a violation of the service terms to use the Mapbox geocoder without also using the Mapbox map (which isn't finished). We'll have a big major release when all the parts are ready.
There was a problem hiding this comment.
Valid point. Let's add an entry when Mapbox can properly be taken into use.
The padding is set for the "Powered by Google" attribution, but that might not be present e.g. with Mapbox.
| @@ -50,7 +50,7 @@ | |||
| .predictionsRoot { | |||
| position: absolute; | |||
| width: 100%; | |||
| padding-bottom: 98px; | |||
| padding-bottom: var(--locationAutocompleteBottomPadding); | |||
There was a problem hiding this comment.
I think the larger padding here was for the Mobile Safari bottom spacing, but it looked fine with 72px when I tested it.
This PR generalises the
LocationAutocompleteInputso that the geocoding implementation can be swapped.Two geocoder implementation are provided, one for Google Maps Places API (default) and one for Mapbox Geocoding API. The Mapbox geocoder requires
window.mapboxglandwindow.mapboxSdkglobals to be present.