dimfeld / svelte-maplibre

Svelte bindings for the MapLibre mapping library
https://svelte-maplibre.vercel.app
MIT License
284 stars 34 forks source link

Inconsistent use of manageHoverState #35

Closed dabreegster closed 9 months ago

dabreegster commented 1 year ago

I spent a bit of time puzzling through how popups were triggered when hovering on a line layer, but my opacity wasn't changing:

          <LineLayer
            paint={{
              "line-width": 5,
              "line-color": "red",
              "line-opacity": hoverStateFilter(1.0, 0.5),
            }}
          > 
            <Popup openOn="hover" let:features>
              ...
            </Popup>
          </LineLayer>

I needed to pass manageHoverState. FillLayer and others don't override this, or even let it be set, AFAICT.

https://github.com/dimfeld/svelte-maplibre/blob/ad6bb3eafaccfa53ca13e3e099c8743383da9f7b/src/lib/LineLayer.svelte#L26 https://github.com/dimfeld/svelte-maplibre/blob/ad6bb3eafaccfa53ca13e3e099c8743383da9f7b/src/lib/Layer.svelte#L32

Any reason for line layer to disable it by default? For consistency, what if all the layers did export let manageHoverState = false; to let it be overridable? Happy to send a PR if that's the desired behavior.


And thanks by the way for this library! I've been using MapLibre and Svelte for a few months in different projects, and by far this library is looking extremely promising for simplifying some of them. I'll open a few other issues, and I'm happy to send PRs.

dimfeld commented 1 year ago

IIRC it was to improve the behavior when you have both a LineLayer and a FillLayer for the same GeoJSON polygons, since otherwise they start to conflict as they are both trying to manage the same polygons. But I agree the current implementation feels inconsistent and is definitely not helpful for other uses of LineLayer.

Allowing any layer to override manageHoverState and defaulting to false feels like the right change here.