feat(map-and-label): remove individual features

[jessicamcinchak]

Rebased version of #3595

My changes (builds on 3595 description):

• Remove feature from map
• Shift labels on "remove" so they stay incremental
• Ensure previously working handleSubmit & "back"/"change" navigation still works
  • This is honestly still a bit wonky but I think less vital for UR than that actual "remove" functionality !!
  • See feat: enable "back" & "edit" navigation for List map fields and MapAndLabel using @opensystemslab/map v1.0.0-alpha.2 #3612 for original implementation, which isn't ideal in combo with new GeoJSONChange hook here

Open questions:

• When removing a feature, I'm forcing a re-render of the web component by changing a key prop on its' parent container which does cause a visible flash - is this acceptable? Other less hacky feeling ideas?
  • Tried many other variations but was having a hard time getting the web component to re-render based on changing features alone - I think this is most likely due to Lit's "shallow change detection"
  • Spent some time reading up on Lit Events, but not totally clear to me if an event listener would be another way to solve this?

[github-actions]

Removed vultr server and associated DNS entries

[jessicamcinchak]

There's some ugly active tab index management going on! Very much need to revisit this / open to suggestions.

In many instances, I want the active tab index to be the last in the list. Otherwise there can be some strange behavior, for example on staging:

• Add trees 1, 2, 3 on the map
• Navigate to Tab 1 (it's active)
• Add another tree (fix(migrations): point volume to directory expected by Hasura #4) to the map - but now tab 2 is active when we really want 4 to be active

[DafyddLlyr]

Yeah this is an issue in the List component which is avoided by only allowing a single active item at a time (it's not possible to add when there's an active item). That solution won't work here due to the map driven nature of the component.

It's going to be a little tricky as setFeatures() is async, but we can take the length from geojson["EPSG:3857"].features reliably I believe.

Something like this might work?

const newFeatures = geojson["EPSG:3857"].features;
setFeatures(newFeatures);
setActiveIndex(newFeatures.length - 1)

I've not tried this so I might be off the mark a little here and treading over old ground you've already worked through.

An alternative might a useEffect() to catch changes to features.length and tie the update to setActiveIndex to that?

[DafyddLlyr]

Working as expected and code is clear and straightforward 👍

A few small issues below which may already be on your radar from last week -

Sorting
10 is coming before 2 -

Map re-render
Updating the key works, but I agree is a bit hacky.

I think what we want to do is add a custom change detection handler to drawGeojsonData that checks the feature length instead of shallow object equality (docs). This looks like it should work? Not a blocker for UR as it's a relatively small UX issue and the map is often out of view when hitting the remove button.

Active tab handling
A few unexpected things here - it's not a simple one. It feels odd to remove feature 2 of 10, and then be jumped onto editing feature 10 (the last in the list). I'd expect to just "stay in place" on the list as I removed items. However, this might be a little odd as due the the label updates you'd still be on "Feature 2" (the previous feature 3).

Editing
When an item is edited via the map, it looks like the active index is not being set correctly. I guess I'd expect either nothing to happen with tabs or to make the edited item active. Right now it's removing the schema fields. I don't think this is introduced here but wanted to flag!

Screen.Recording.2024-09-09.at.09.16.42.mov

[DafyddLlyr]

Yeah this is an issue in the List component which is avoided by only allowing a single active item at a time (it's not possible to add when there's an active item). That solution won't work here due to the map driven nature of the component.

It's going to be a little tricky as setFeatures() is async, but we can take the length from geojson["EPSG:3857"].features reliably I believe.

Something like this might work?

const newFeatures = geojson["EPSG:3857"].features;
setFeatures(newFeatures);
setActiveIndex(newFeatures.length - 1)

I've not tried this so I might be off the mark a little here and treading over old ground you've already worked through.

An alternative might a useEffect() to catch changes to features.length and tie the update to setActiveIndex to that?

[jessicamcinchak]

Thanks @DafyddLlyr for thorough review here - merging to staging for UR deadline, but will open follow PRs today to address these comments / fix-forward !