Map marker on selected result

[scottsfarley93]

This PR handles #213 by adding a mapboxgl#Marker to the map after the user has selected a result. it exposes two additional constructor options:

• addToMap: a boolean field indicating whether the marker should be added. Optional, default true.
• markerOptions: an optional object specify the marker options to be passed to the marker constructor.

Note: this requires a more recent version of mapbox-gl-js (v0.53.0) so there may be compatibility issues. Also, the existing tests need some massaging to get to work with the newer version of mapbox-gl.

• briefly describe the changes in this PR
• write tests for all new functionality
• update CHANGELOG.md with changes under master heading before merging
• Make sure tests work with more recent mapbox-gl-js version

\cc @katydecorah @yuletide

[scottsfarley93]

This bumps the bundle size up a nontrivial amount:

5194 mapbox-gl-geocoder.css
711473 mapbox-gl-geocoder.min.js

We should make sure we're comfortable with such an increase before merging.

\cc @andrewharvey -- what are your thoughts on about the tradeoff here?

[andrewharvey]

This bumps the bundle size up a nontrivial amount:

Is that because it's now including the whole mapbox-gl in the geocoder bundle? If that's the case, I think we should try to find a way avoid that.

[samanpwbb]

To get around an explicit mapboxgl dependency, you could set mapbox gl as a peer dependency in package.json, and make the geocoder take mapboxgl as an argument on initialization.

[andrewharvey]

I haven't tested, so not what the current behaviour is, but if someone clears the geocoder, should the marker be removed? I'm think, yes it should be removed, but what do you think?

[andrewharvey]

I wonder should this work like the new flyTo option, where you can pass true, false, or an options object? Simplifying the two options into a single marker option which can be true, false or an options object?

[scottsfarley93]

Thanks for the feedback @andrewharvey!

if someone clears the geocoder, should the marker be removed?

Good catch. I agree, and added this behavior in f454c4d.

Simplifying the two options into a single marker option which can be true, false or an options object

I think this simplifies the API and, as you point out, makes the behavior more like the existing options. I added this behavior in e730470

[katydecorah]

This works beautifully! I left you two minor comments

[katydecorah]

To confirm, do we want mapbox-gl as a devDep and peerDep? I'm not well versed on how peerDep work so just want to gut check in case this was in error.

[scottsfarley93]

I believe we want it to be both. During regular use, we want it as a peer dependency because we expect the plugin to be used in conjunction with a mapbox-gl map. However, during development we want it as a dev dependency so we can use it in tests.

[andrewharvey]

Is v0.53.0 the minimum requirement? I think if possible we should try to set this to the minimum version required for everything to work in gl-geocoder.

Not essential, just a nice to have.

[andrewharvey]

Just one more thing I noticed, any Marker automatically added here should also be removed from the map inside the onRemove method.

[andrewharvey]

Since this feature was only added to mapbox-gl-js in v0.45.0, I think we should add a note to the changelog about the new minimum gl-js version requirement for mapbox-gl-geocoder v4, like was done for v2 https://github.com/mapbox/mapbox-gl-geocoder/blob/master/CHANGELOG.md#v200

[katydecorah]

So smooth!

[andrewharvey]

👍

[jayenashar]

this is working pretty well for me, but how do i get it to move an existing marker, that i'm using as a default before the user enters a location in the geocoder? can i pass the marker option an existing marker?