maptiler / maptiler-geocoding-control

The Javascript & TypeScript Map Control component for MapTiler Geocoding service! Easy to be integrated into any JavaScript mapping application.
https://maptiler.com/cloud/geocoding
BSD 3-Clause "New" or "Revised" License
31 stars 4 forks source link

[BUG] geocoding control component freezes after first search #16

Closed Robinlovelace closed 11 months ago

Robinlovelace commented 11 months ago

Describe the bug

I have followed the tutorial at and the geocoding control component works great, but only on the first go.

To Reproduce

See reproducible example + video associated with a simple codebase showing this behaviour here https://github.com/cruse-bike/cruse/issues/8

Expected behavior

I was expecting to be able to clear the search results and fly to another place after the first 'flyto'.

Screenshots

https://github.com/maptiler/maptiler-geocoding-control/assets/1825120/735aa8ab-932f-477b-902d-4dbe80cd3374

Additional context

I'm experimenting with MapLibre and Svelte to add quality of life features (like geocoding) and UX improvements to this web application funded by the Irish Government for sustainable transport planning: https://cruse.bike/

Robinlovelace commented 11 months ago

I should add: I'm not 100% sure this is a bug and I'm a Svelte novice so possible that I've made a silly mistake, apologies if so, but thought it may be helpful to report this unexpected behaviour.

zdila commented 11 months ago

Hi @Robinlovelace .

Than you for your report!

Would you be so kind and try to provide us a minimal reproducible example? You can attach a tarball or modify https://codesandbox.io/p/sandbox/svelte-geocoding-control-4dzp2r (provide your API key for it to work).

Robinlovelace commented 11 months ago

Minimal example, with fix in there:

<script>
    import { base } from '$app/paths';
    import {
        MapLibre,
        VectorTileSource,
        LineLayer,
        Popup,
        GeolocateControl,
        NavigationControl,
        FullscreenControl
    } from 'svelte-maplibre';
    import GeocodingControl from '@maptiler/geocoding-control/svelte/GeocodingControl.svelte';
    const apiKey = 'key';
</script>

<!-- <h1>CRUSE test map</h1> -->

<MapLibre
    {apiKey}
    center={[-8.63, 52.66]}
    zoom={11}
    class="map"
    controlPosition="top-right"
    style="https://basemaps.cartocdn.com/gl/positron-gl-style/style.json"
    let:map
>
    <GeocodingControl
        {apiKey}
        on:select={(e) => {
+           if (!e.detail?.center) return;
            map.flyTo({
                center: e.detail.center,
                zoom: 10
            });
        }}
        class="geoCodeControl"
    />

    <!-- <Control position="top-right"/> -->
    <NavigationControl position="top-right" />
    <GeolocateControl position="top-right" />
    <FullscreenControl position="top-right" />

    <VectorTileSource url={'pmtiles://rnet_limerick.pmtiles'}>
        <LineLayer
            paint={{
                'line-opacity': 0.6,
                // Style based on value of 'Bicycle (Go Dutch)' field
                // With breaks at 0, 10, 100, 1000:
                'line-color': [
                    'step',
                    ['get', 'Bicycle (Go Dutch)'],
                    '#f2f12d',
                    10,
                    '#e6b71e',
                    100,
                    '#d84e11',
                    1000,
                    '#bf1d00'
                ],
                'line-width': 2
            }}
            sourceLayer="rnet_limerick"
            hoverCursor="pointer"
        >
            <Popup openOn="click" offset={[0, -10]} let:features>
                {@const props = features?.[0]?.properties}
                {#each Object.entries(props) as [key, val]}
                    <p>
                        <span class="popUpKey">{key}</span> : <span class="popUpVal">{val}</span>
                    </p>
                {/each}
            </Popup>
        </LineLayer>
    </VectorTileSource>
</MapLibre>

<a href="{base}/about">About</a>

<style>
    :global(.map) {
        height: 500px;
    }

    :global(.geoCodeControl) {
        margin: 10px 15px;
    }
    .popUpKey {
        color: #444
    }
    .popUpVal {
        font-weight: 600;
    }
</style>
Robinlovelace commented 11 months ago

You could also do

gh repo clone cruse-bike/cruse
cd cruse
yarn dev
zdila commented 11 months ago

I have tried it with your project but I can't reproduce it. Using latest Vivaldi on Linux.

Do you see any errors in JS console?

Robinlovelace commented 11 months ago

I have tried it with your project but I can't reproduce it. Using latest Vivaldi on Linux.

Do you see any errors in JS console?

Thanks for testing. Just to check did you remove this line before trying to reproduce it?

As far as I recall this magic line is not in the docs:

+           if (!e.detail?.center) return;
zdila commented 11 months ago

If I remove that line then I can reproduce the issue but it can be expected as detail of the select event can be undefined. Please see https://docs.maptiler.com/sdk-js/modules/geocoding/api/api-reference/#event:select.

Robinlovelace commented 11 months ago

OK glad to you can reproduce the issue. I've taken a look at the API reference and it's not immediately clear that

+           if (!e.detail?.center) return;

needs to be added to the geocoding control to prevent the issue. Maybe worth flagging in the documentation?

I'm not a JavaScript developer so it's possible that patch is obvious to others, may be worth getting feedback from @mattlehrer who provided the patch in https://github.com/cruse-bike/cruse/pull/9, was this expected behaviour or could this be better documented/handled?

zdila commented 11 months ago

Actually it is documented (see the link) and also denoted with TypeScript types which should produce error in your case (if you switch to TypeScript). Value undefined enables developers to detect that the item has been deselected.

Robinlovelace commented 11 months ago

I was referring to this documentation: https://docs.maptiler.com/svelte/maplibre-gl-js/geocoding-control/

Is the equivalent bit this?

  {#if mapController}

I'll defer to others on whether this is an issue with the documentation/implementation or whether I just need to improve my JS knowhow which is entirely possible.

Robinlovelace commented 11 months ago

One additional question: what's the best way to debug this kind of error? I found it a confusing error message but again that's likely because I'm JS novice.

Slightly off topic and happy for this issue to be closed if it works as intended.

zdila commented 11 months ago

I was referring to this documentation: https://docs.maptiler.com/svelte/maplibre-gl-js/geocoding-control/

That sample doesn't use on:select or other advanced features.

what's the best way to debug this kind of error? I found it a confusing error message but again that's likely because I'm JS novice

Your browser's DevTools should show such errors in the Console:

2023-11-30_21-21

zdila commented 11 months ago

It may seem cryptic. It contains stacktrace from the place where error occured (first line of the stacktrace) to toplevel funtion. Look for familiar filenames of your project written in parentheses of the stacktrace lines.

Robinlovelace commented 11 months ago

Sounds like working as intended then, nothing to change? Happy for this to be closed and thanks for all the responses.