mapbox / mapbox-gl-geocoder

Geocoder control for mapbox-gl-js using Mapbox Geocoding API
https://mapbox.com/mapbox-gl-js/example/mapbox-gl-geocoder/
ISC License
362 stars 181 forks source link

Fix setPlaceholder to update this.options.placeholder #502

Closed marexandre closed 1 year ago

marexandre commented 1 year ago

Fix setPlaceholder so that when setPlaceholder is called afterwards it return updated placeholder string

Before

Example of the bug and what this PR tries to fix:

const geocoder = new MapboxGeocoder({
    accessToken: mapboxgl.accessToken,
    mapboxgl: mapboxgl,
    placeholder: 'Test'
});

geocoder.getPlaceholder();
// returns Test"

geocoder.setPlaceholder('New string');

geocoder.getPlaceholder();
// returns  "Test"  and not the updated placeholder text

After

const geocoder = new MapboxGeocoder({
    accessToken: mapboxgl.accessToken,
    mapboxgl: mapboxgl,
    placeholder: 'Test'
});

geocoder.getPlaceholder();
// returns  "Test"

geocoder.setPlaceholder('New string');

geocoder.getPlaceholder();
// returns  "New string" 
marexandre commented 1 year ago

If I wanted to have this change in v4 of this library, what would be the recommendations to add a similar fix?

marexandre commented 1 year ago

CI is failing for the following reason - which does not seem to be related to changes in this PR...

Screen Shot 2023-07-05 at 12 03 23 PM
marexandre commented 1 year ago

@stepankuzmin thank you for taking a look at this changes - rant the tests locally and all passed 🎉 How should we proceed with merging this PR? Anything else I should do or missed?