[Maps] Fixed double click issue when deleting a shape

[maksimkovalev]

Closes: #117369

The user will now see a waiting icon after deleting the shape, but multiple clicks will not be processed until the async operation is complete.

cc @nreese

[nreese]

Thanks for opening this. Small user feedback like this PR introduces makes such a big difference in application polish.

What do you think about disabling the draw buttons on the left when edit mode is WAIT? Users can exit WAIT state by clicking on these buttons.

A good way to see this is to open your browsers debugging tools and go to the network tab. In chrome, there is a "no throttling" select. I find that changing this to "slow 3g" gives you time to really see the interaction with a slow network and mess around with controls when in "wait" state.

[elasticmachine]

Pinging @elastic/kibana-gis (Team:Geo)

[nreese]

@elasticmachine merge upstream

[nreese]

Can you remove waitingState from DrawFeatureControl. Since actions addNewFeatureToIndex and deleteFeatureFromIndex modify the edit mode at the end of the action, I think it makes the most sense to also modify the edit mode in the action at the beginning of the action so that all edit mode changes are in the same location in the code base

[nreese]

@elasticmachine merge upstream

[maksimkovalev]

@elasticmachine merge upstream

[nreese]

@elasticmachine merge upstream

[nreese]

This is looking much better, I like how this PR is cleaning how editShape is set. It's much easier to follow now that it is set in an action vs having it set via a callback triggered by mb_draw.

I am still able to trigger the original bug 1 out of 20 tries, so its rare but still possible. Adding a debounce to DrawControl.onClick with a small timeout of something like 100 will eliminate the possibility of double clicking a single feature before redux has time to update.

[maksimkovalev]

@nreese I'm not quite sure that adding a debounce to DrawControl.onClick will solve the original issue.
We need to return the drawing state to DELETE when the shape is competely removed from the map, but this removal process is controlled by the MbMap plugin.
Returning to the DELETE state is going faster, but showing/removing a shape on the map by the MbMap plugin takes some short or even fixed time. That's why I added setTimeout a few commits ago.

[nreese]

@elasticmachine merge upstream

[nreese]

I'm not quite sure that adding a debounce to DrawControl.onClick will solve the original issue.

You are right, the debounce solves one race condition where a second feature click is allowed before redux updates state to WAIT but there is another race condition where a deleted feature can be clicked after state is set to DELETE but before the layer has re-synced with MapLibre and re-rendered.

Returning to the DELETE state is going faster, but showing/removing a shape on the map by the MbMap plugin takes some short or even fixed time. That's why I added setTimeout a few commits ago.

A timeout of arbitrary length does not completely fix the race-condition, it just makes it more unlikely.

Chatted with @thomasneirynck offline about a solution to avoid the second race condition and we came up with the following

• add a new piece of state to redux in ui called deletedFeatureIds, which is a string array
• In deleteFeatureFromIndex, return if featureId is contained in deletedFeatureIds.
• In deleteFeatureFromIndex, after successful call to await (layer as IVectorLayer).deleteFeature(featureId);, dispatch action to add featureId to deletedFeatureIds.
• Update updateEditShape action to clear deletedFeatureIds if shapeToDraw is not DELETE.

[nreese]

@elasticmachine merge upstream

[nreese]

LGTM
code review, tested in chrome

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: ca6b6fa
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
maps	759	758	-1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
maps	2.5MB	2.5MB	+112.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
maps	66.0KB	66.0KB	+14.0B

History

• 💚 Build #27944 succeeded 4b4ffc2
• 💚 Build #27799 succeeded 1d1f08c
• 💚 Build #27708 succeeded 25679b1
• 💚 Build #27085 succeeded 341e3bf
• 💚 Build #26751 succeeded 3f64ce0
• 💚 Build #26526 succeeded 3e03912

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream