RTFDB-4842: Improve user input handling in "mapbox-gl-geocoder" - #559
RTFDB-4842: Improve user input handling in "mapbox-gl-geocoder"#559pahuta wants to merge 2 commits into
Conversation
- Add optional `parseExtendedSpatialFormats` option with four independently toggled formats, all disabled by default: `commaSeparatedLngLatZoom` (`lng,lat,zoom`), `slashSeparatedZoomLatLng` (`zoom/lat/lng`), `tile` (`z/x/y`) and `quadkey` - Add `lib/spatial-formats.js`, which turns a matching search input into a synthetic GeoJSON feature and prepends it to the suggestion list on both the successful and the failed request path - Add `isValidTile`, `isValidQuadkey`, `tileToLngLat` and `quadkeyToTile` helpers to `lib/utils.js` - Use the parsed zoom when flying to a selected feature that has no `bbox` - Fix `getSelectedIndex` in `lib/events.js` reporting index 0 for every result feature without an `id` https://mapbox.atlassian.net/browse/RTFDB-4842
| trackProximity: true, | ||
| useBrowserFocus: true, | ||
| enableGeolocation: true, | ||
| parseExtendedSpatialFormats: { |
There was a problem hiding this comment.
Why do we need to expose this as a public API?
There was a problem hiding this comment.
To give our users (including internal ones) the ability to quickly navigate to a desired location. It's practically the same as how coordinate search works now.
There was a problem hiding this comment.
So you expect end users to configure the accepted formats in UI?
There was a problem hiding this comment.
No, not on UI. Let's clarify, in this context "end users" are developers and they will just enable parsing predefined formats. Here is an example how we configure mapbox-gl-geocoder right now:
const geocoder = new MapboxGeocoder({
accessToken,
origin: config.MAPBOX_API_URL,
useBrowserFocus: true,
mapboxgl: mapboxgl,
marker: false,
reverseGeocode: true,
flipCoordinates: true
});These are just a few additional configurations which are fully backwards compatible with a current default behavior. Example how new parseExtendedSpatialFormats will be used:
const geocoder = new MapboxGeocoder({
accessToken,
origin: config.MAPBOX_API_URL,
useBrowserFocus: true,
mapboxgl: mapboxgl,
marker: false,
reverseGeocode: true,
flipCoordinates: true,
// new config below
parseExtendedSpatialFormats: {
commaSeparatedLngLatZoom: true,
slashSeparatedZoomLatLng: true,
tile: true,
quadkey: true
}
});After that users on UI will be able to specify coordinates/tile/quadkey in the search input and navigate to the location.
There was a problem hiding this comment.
I still don't understand why this needs to be configurable. Do you expect developers to explicitly opt out of some formats? Also, why did you choose these formats (i.e., why lnglat and not latlng)? I wonder if we could enable this by default, improve the detection logic, and avoid introducing such a public API at all.
There was a problem hiding this comment.
Changed the implementation as we discussed: indeed localGeocoder covers our needs and there is no sense to introduce the parseExtendedSpatialFormats parameter.
|
|
||
| // Marks a feature synthesized from an extended spatial format, so it can be told | ||
| // apart from Geocoding API results (which carry `_source: 'mapbox'`). | ||
| const SOURCE = 'extended-spatial-format'; |
There was a problem hiding this comment.
Why do we need syntetic features at all?
There was a problem hiding this comment.
The same reason as in previous comment. I agree that it's not directly related to the geocoding but it significantly improve UX. For example in our user facing applications in Mapbox Console user will be able to quickly move the map to the desired location if he/she has coordinates in other format than apps support.
There was a problem hiding this comment.
The feature itself is good, but I wonder if we could simplify the implementation
There was a problem hiding this comment.
I open to the suggestion. In general code of mapbox-gl-geocoder is old and written in "all in one file" style. I tried to keep it maintainable by putting new functionality separately as much as possible and maintaining backward compatibility.
- remove a `parseExtendedSpatialFormats` contractor parameter in favor of using a `localGeocoder` https://mapbox.atlassian.net/browse/RTFDB-4842
2 participants
localGeocoderfunctionisValidTile,isValidQuadkey,tileToLngLatandquadkeyToTilehelpers tolib/utils.jsbboxhttps://mapbox.atlassian.net/browse/RTFDB-4842
npm run docsand commit changes to API.mdmainheading before mergingParsing extended spatial formats when enabled:
parseExtendedSpatialFormats.mp4
Previous vs current behavior when user cut or undo search input value
cut_undo.mp4