Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

feat(autocomplete): add locate event when pin is clicked #1063

Merged
merged 1 commit into from
May 26, 2020

Conversation

MarcL
Copy link
Contributor

@MarcL MarcL commented May 25, 2020

Summary
Allow the pin to emit a locate event when clicked. This will allow integrating code to determine when a user enables geolocation in the browser and act upon it.

This addresses an issue as discussed in #952.

I came across this too as I wanted to use the places module in a Facebook Messenger Webview. The pin icon looks like it should geolocate the user so having this as an additional event would allow me to do this without tying it to the places implementation.

I chose the name locate for the event but I'm happy to change this to a better name if appropriate. I've added a unit test for this but had to refactor the mocking of createAutocompleteDataset as it's set up prior to the places instance being created before every test. I felt this was appropriate but happy to refactor this.

The documentation has been updated.

Result

The locate event is emitted when clicking on the pin icon to allow users to determine when this has happend.

docs(documentation): add locate event for pin
@MarcL MarcL changed the title feat(autocomplete): add locate event when pin is clicked + tests feat(autocomplete): add locate event when pin is clicked May 25, 2020
@JonathanMontane
Copy link
Contributor

JonathanMontane commented May 26, 2020

Wow! Thanks for the PR and great initiative!

The code looks great to me, but before I merge I wanted to make a small note regarding your use case: You plan on using the icon as a means for the user to tell you to geolocate them, which is completely fine, but be careful to call places.configure({ useDeviceLocation: false }) in your on('locate') handler because the current design makes it so that if useDeviceLocation is set to true, the navigator geolocation is always favoured over any aroundLatLng parameter provided, which would make it so that your geolocation data is ignored.

I will probably change that in a future PR, as it makes more sense that user data has higher priority than browser data, but for now please be wary of this "bug".

@JonathanMontane JonathanMontane self-requested a review May 26, 2020 08:03
@JonathanMontane JonathanMontane merged commit 9b9efdc into algolia:master May 26, 2020
@MarcL MarcL deleted the add-pin-event branch May 26, 2020 10:48
@MarcL
Copy link
Contributor Author

MarcL commented May 26, 2020

Thanks for merging @JonathanMontane and I'm happy to raise a PR. 🎉

The reason I raised this was that I implemented it for a client and they thought that clicking on the pin icon would locate them. I don't think its purpose is quite clear enough.

That's a good point about the device location. I'll ensure I add that to my listener.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants