dimfeld / svelte-maplibre

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

Adds a JoinedData Component to allow client side joins #65

Closed stuartlynn closed 9 months ago

stuartlynn commented 9 months ago

This PR adds a JoinedData component intended to be used as a sub-component of the <VectorSource /> or <GeoJSON /> components.

It has three arguments

This allows the data that is passed in to the component to be referenced in styling and popups through the ["feature-state", colName] funciton in maplibre styling

vercel[bot] commented 9 months ago

Someone is attempting to deploy a commit to a Personal Account owned by @dimfeld on Vercel.

@dimfeld first needs to authorize it.

vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
svelte-maplibre ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 29, 2023 6:46pm
dimfeld commented 9 months ago

A couple other things:

  1. If an item is removed from data, then the feature state for the ID of the removed item will still be present on the corresponding feature. This might necessitate keeping a list of which feature IDs were set last time, and then unsetting the state from any features which were not seen in the current run of the loop.

  2. I think this will conflict with the hover state management when that is enabled, but I'm not sure what to do about that.

edit: I have an idea of what to do about the second item with a separate piece of code to merge hover state with other feature state, but it's getting outside the scope of this PR. Don't worry about that one for now.

dimfeld commented 9 months ago

Haven't actually tried it but I think this will work for item 1 in the previous comment.

let lastFeatureIds = new Set();
$: if (data && $map && $source) {

  let seenIds = new Set();
  for (const row of data) {
    const id = row[idCol];

    lastFeatureIds.delete(id);
    seenIds.add(id);

    $map.setFeatureState({ id, source: $source, sourceLayer }, row);
  }

  for (const id of lastFeatureIds){
    $map.setFeatureState({ id, source: $source, sourceLayer }, null);
  }

  lastFeatureIds = seenIds;
}
changeset-bot[bot] commented 9 months ago

🦋 Changeset detected

Latest commit: 6bf06a96a3abfd90e3209633e827956934006802

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package | Name | Type | | --------------- | ----- | | svelte-maplibre | Minor |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

stuartlynn commented 9 months ago

The latest push integrates the JoinData with hover state but doesn't 100% fix the data removal issue. I am not 100% why that isn't working. If anyone can spot what I am doing wrong let me know.

stuartlynn commented 9 months ago

Ok I think this should be good to go now. The updates

Just to know there is a weird issue with MapLibre. It took me a while to figure out why feature removal wasn't working but essentially if you set the featureState on a given feature to:

{
    propertyOne: "one"
}

and then set it to

{
    propertyTwo:"two"
}

what you actually get is

{
   propertyOne:"one",
   propertyTwo:"two"
}

so to reset the state to blank you have to manually set any property that was previously set to null so

{
    propertyOne:null,
    propertyTwo:"two"
}

I am going to look in to it tomorrow and see if this is intentional from there side, a known bug or something to flag up to them.

stuartlynn commented 9 months ago

@dimfeld If your happy with this I think it's good to merge

stuartlynn commented 9 months ago

Ok so apparently it's intentional

When using this method, the state object is merged with any existing key-value pairs in the feature's state.

https://maplibre.org/maplibre-gl-js/docs/API/classes/maplibregl.Map/#setfeaturestate

and I think means we can remove the custom logic I added to handle not overwriting hoverState

stuartlynn commented 9 months ago

Ok now we are 100% good to go.

stuartlynn commented 9 months ago

Ran format and changeset

Also went with your suggestions of using removeFeatureState.

dimfeld commented 9 months ago

Hmm did the removeFeatureState commit maybe not get pushed? I'm not seeing that for some reason.

dimfeld commented 9 months ago

Don't worry about it, I'll just make the change since I have it open now anyway :)