Reset AoIs

[nerik]

This adds a reset button as suggested here

Unfortunately, I couldn't get this to work. The control needs access to the map ref, which is stored in the MapsContext (reachable via the useMaps hook), but useThemedControls creates a separated React root, hence without access to any context.

I've been looking for hours for a clean solution but I'm stuck, sorry :(

[netlify]

✅ Deploy Preview for veda-ui ready!

Name	Link
🔨 Latest commit	733f10c
🔍 Latest deploy log	https://app.netlify.com/sites/veda-ui/deploys/6538edc4b8290c0008ffe1f7
😎 Deploy Preview	https://deploy-preview-707--veda-ui.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[danielfdsilva]

Would it be possible to access the context here and pass it as a prop to ResetAoI?

[nerik]

Great suggestion: d102d80

However instance doesn't exist on this map ref for some reason.

I could create a specific context for this map implementation in order for the draw control to expose its internals for other draw-related controls, but this really feels like a hack.

[danielfdsilva]

Where is instance coming from?

[danielfdsilva]

The previous ref implementation had an instance because we added it since it served our needs.

veda-ui/app/scripts/components/common/mapbox/index.tsx

Lines 168 to 175 in 1bb5c5e

	useImperativeHandle(ref, () => ({
	resize: () => {
	mapRef.current?.resize();
	mapCompareRef.current?.resize();
	},
	instance: mapRef.current,
	compareInstance: mapCompareRef.current
	}));

And then the map stored the draw control so we can act on it, but none of this is default.

veda-ui/app/scripts/components/common/mapbox/aoi/mb-aoi-draw.js

Lines 32 to 40 in 1bb5c5e

	const newMbDraw = new MapboxDraw({
	modes: MapboxDraw.modes,
	displayControlsDefault: false,
	styles: computeDrawStyles(theme)
	});
	mbDrawRef.current = newMbDraw;
	mbMap.addControl(newMbDraw, 'top-left');

[danielfdsilva]

A solution for this implementation could be to add a reference to the control when it is mounted:

@app/scripts/components/common/map/controls/aoi/index.tsx

useControl<MapboxDraw>(
-    () => {
+    ({map}) => {
      control.current = new MapboxDraw(props);
+      map._drawControl = control.current;
      return control.current;
    },

Then you could access it via map._drawControl on the reset button

[danielfdsilva]

However for the clear button I don't think you need to access the trash method. Using this without a selection does nothing.
I'd imagine that deleting all the feature via the atom would do the trick.

[nerik]

I've used deleteAll: d102d80, it works!

I'd imagine that deleting all the feature via the atom would do the trick.
Sadly no, you have to manually keep the atom and the mbdraw state in sync

I have taken the opportunity to clean up that flow with the upload AoI feature: 66a9f32

[danielfdsilva]

I had a brief conversation with Erik at the end of last week and this is at a point where it is working.
However the result is that this implementation relies both on Mapbox draw imperative methods and react reactive code.

For example:

veda-ui/app/scripts/components/common/map/controls/aoi/reset-aoi-control.tsx

Lines 25 to 30 in 2f1e5f0

	const onReset = useCallback(() => {
	const mbDraw = map?._drawControl;
	if (!mbDraw) return;
	mbDraw.deleteAll();
	aoiDeleteAll();
	}, [map, aoiDeleteAll]);

Here, the features are being deleted from the draw plugin and at the same time from the atom that stores them.

There no absolutely clean way to do this though. We are working with a mapbox plugin that is imperative by nature.
A way to reduce this problem is to create custom controls for the deletion and then based on those set the features on the plugin. (deleting would become setting and empty feature array).
This was the approach used in the current analysis. The controls fire AOI events that are captured and processed.

This same behavior could be migrated to this implementation but it would require some additional work and changes. Do you have any thoughts @hanbyul-here @sandrahoang686? Do you feel that this is worth it?
cc @j08lue

[hanbyul-here]

We would eventually want to have one place that syncs these states, but I think it is acceptable to file this as a tech debt and move on for now.

[danielfdsilva]

Rebased to fix the conflict and logged the tech debt.
Will merge.