[Maps] Fixed double click issue when deleting a shape (#124661)

* #117369 - Fixed double click issue when deleting a shape * #117369 - updated deleting shape method; redux changes; * #117369 - waiting state for adding shapes feature * #1173669 - refactoring; removed unused functions * 117369 - refactoring * 117369 - Updates for onDraw methods, now the selected shape is not reset * 117369 - made addNewFeatureToIndex to be called once * 117369 - refactoring * 117369 - refactoring and clean up * 117369 - removed then method from actions * 117369 - refactoring * 1177369 - refactoring * 117369 - refactoring; Added new state in redux * 117369 - refactoring * 117369 - renaming layerId to featureId Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Mar 7, 2022 · 8a8bfd5 · 8a8bfd5
1 parent ccf16e9
commit 8a8bfd5
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 58 deletions.
diff --git a/x-pack/plugins/maps/common/constants.ts b/x-pack/plugins/maps/common/constants.ts
@@ -144,6 +144,7 @@ export enum DRAW_SHAPE {
   LINE = 'LINE',
   SIMPLE_SELECT = 'SIMPLE_SELECT',
   DELETE = 'DELETE',
+  WAIT = 'WAIT',
 }
 
 export const AGG_DELIMITER = '_of_';

diff --git a/x-pack/plugins/maps/public/actions/map_actions.ts b/x-pack/plugins/maps/public/actions/map_actions.ts
@@ -14,6 +14,7 @@ import turfBooleanContains from '@turf/boolean-contains';
 import { Filter } from '@kbn/es-query';
 import { Query, TimeRange } from 'src/plugins/data/public';
 import { Geometry, Position } from 'geojson';
+import { asyncForEach } from '@kbn/std';
 import { DRAW_MODE, DRAW_SHAPE } from '../../common/constants';
 import type { MapExtentState, MapViewContext } from '../reducers/map/types';
 import { MapStoreState } from '../reducers/store';
@@ -63,9 +64,10 @@ import { DrawState, MapCenterAndZoom, MapExtent, Timeslice } from '../../common/
 import { INITIAL_LOCATION } from '../../common/constants';
 import { updateTooltipStateForLayer } from './tooltip_actions';
 import { isVectorLayer, IVectorLayer } from '../classes/layers/vector_layer';
-import { SET_DRAW_MODE } from './ui_actions';
+import { SET_DRAW_MODE, pushDeletedFeatureId, clearDeletedFeatureIds } from './ui_actions';
 import { expandToTileBoundaries } from '../classes/util/geo_tile_utils';
 import { getToasts } from '../kibana_services';
+import { getDeletedFeatureIds } from '../selectors/ui_selectors';
 
 export function setMapInitError(errorMessage: string) {
   return {
@@ -321,6 +323,10 @@ export function updateEditShape(shapeToDraw: DRAW_SHAPE | null) {
         drawShape: shapeToDraw,
       },
     });
+
+    if (shapeToDraw !== DRAW_SHAPE.DELETE) {
+      dispatch(clearDeletedFeatureIds());
+    }
   };
 }
 
@@ -353,7 +359,7 @@ export function updateEditLayer(layerId: string | null) {
   };
 }
 
-export function addNewFeatureToIndex(geometry: Geometry | Position[]) {
+export function addNewFeatureToIndex(geometries: Array<Geometry | Position[]>) {
   return async (
     dispatch: ThunkDispatch<MapStoreState, void, AnyAction>,
     getState: () => MapStoreState
@@ -369,7 +375,10 @@ export function addNewFeatureToIndex(geometry: Geometry | Position[]) {
     }
 
     try {
-      await (layer as IVectorLayer).addFeature(geometry);
+      dispatch(updateEditShape(DRAW_SHAPE.WAIT));
+      await asyncForEach(geometries, async (geometry) => {
+        await (layer as IVectorLayer).addFeature(geometry);
+      });
       await dispatch(syncDataForLayerDueToDrawing(layer));
     } catch (e) {
       getToasts().addError(e, {
@@ -378,6 +387,7 @@ export function addNewFeatureToIndex(geometry: Geometry | Position[]) {
         }),
       });
     }
+    dispatch(updateEditShape(DRAW_SHAPE.SIMPLE_SELECT));
   };
 }
 
@@ -386,6 +396,12 @@ export function deleteFeatureFromIndex(featureId: string) {
     dispatch: ThunkDispatch<MapStoreState, void, AnyAction>,
     getState: () => MapStoreState
   ) => {
+    // There is a race condition where users can click on a previously deleted feature before layer has re-rendered after feature delete.
+    // Check ensures delete requests for previously deleted features are aborted.
+    if (getDeletedFeatureIds(getState()).includes(featureId)) {
+      return;
+    }
+
     const editState = getEditState(getState());
     const layerId = editState ? editState.layerId : undefined;
     if (!layerId) {
@@ -395,8 +411,11 @@ export function deleteFeatureFromIndex(featureId: string) {
     if (!layer || !isVectorLayer(layer)) {
       return;
     }
+
     try {
+      dispatch(updateEditShape(DRAW_SHAPE.WAIT));
       await (layer as IVectorLayer).deleteFeature(featureId);
+      dispatch(pushDeletedFeatureId(featureId));
       await dispatch(syncDataForLayerDueToDrawing(layer));
     } catch (e) {
       getToasts().addError(e, {
@@ -405,5 +424,6 @@ export function deleteFeatureFromIndex(featureId: string) {
         }),
       });
     }
+    dispatch(updateEditShape(DRAW_SHAPE.DELETE));
   };
 }
diff --git a/x-pack/plugins/maps/public/actions/ui_actions.ts b/x-pack/plugins/maps/public/actions/ui_actions.ts
@@ -24,6 +24,8 @@ export const SET_OPEN_TOC_DETAILS = 'SET_OPEN_TOC_DETAILS';
 export const SHOW_TOC_DETAILS = 'SHOW_TOC_DETAILS';
 export const HIDE_TOC_DETAILS = 'HIDE_TOC_DETAILS';
 export const SET_DRAW_MODE = 'SET_DRAW_MODE';
+export const PUSH_DELETED_FEATURE_ID = 'PUSH_DELETED_FEATURE_ID';
+export const CLEAR_DELETED_FEATURE_IDS = 'CLEAR_DELETED_FEATURE_IDS';
 
 export function exitFullScreen() {
   return {
@@ -123,3 +125,16 @@ export function closeTimeslider() {
     dispatch(setQuery({ clearTimeslice: true }));
   };
 }
+
+export function pushDeletedFeatureId(featureId: string) {
+  return {
+    type: PUSH_DELETED_FEATURE_ID,
+    featureId,
+  };
+}
+
+export function clearDeletedFeatureIds() {
+  return {
+    type: CLEAR_DELETED_FEATURE_IDS,
+  };
+}
diff --git a/x-pack/plugins/maps/public/connected_components/mb_map/draw_control/draw_control.tsx b/x-pack/plugins/maps/public/connected_components/mb_map/draw_control/draw_control.tsx
@@ -20,15 +20,6 @@ import { DRAW_SHAPE } from '../../../../common/constants';
 import { DrawCircle, DRAW_CIRCLE_RADIUS_LABEL_STYLE } from './draw_circle';
 import { DrawTooltip } from './draw_tooltip';
 
-const mbModeEquivalencies = new Map<string, DRAW_SHAPE>([
-  ['simple_select', DRAW_SHAPE.SIMPLE_SELECT],
-  ['draw_rectangle', DRAW_SHAPE.BOUNDS],
-  ['draw_circle', DRAW_SHAPE.DISTANCE],
-  ['draw_polygon', DRAW_SHAPE.POLYGON],
-  ['draw_line_string', DRAW_SHAPE.LINE],
-  ['draw_point', DRAW_SHAPE.POINT],
-]);
-
 const DRAW_RECTANGLE = 'draw_rectangle';
 const DRAW_CIRCLE = 'draw_circle';
 const mbDrawModes = MapboxDraw.modes;
@@ -41,7 +32,6 @@ export interface Props {
   onClick?: (event: MapMouseEvent, drawControl?: MapboxDraw) => void;
   mbMap: MbMap;
   enable: boolean;
-  updateEditShape: (shapeToDraw: DRAW_SHAPE) => void;
 }
 
 export class DrawControl extends Component<Props> {
@@ -91,12 +81,6 @@ export class DrawControl extends Component<Props> {
     }
   }, 0);
 
-  _onModeChange = ({ mode }: { mode: string }) => {
-    if (mbModeEquivalencies.has(mode)) {
-      this.props.updateEditShape(mbModeEquivalencies.get(mode)!);
-    }
-  };
-
   _removeDrawControl() {
     // Do not remove draw control after mbMap.remove is called, causes execeptions and mbMap.remove cleans up all map resources.
     const isMapRemoved = !this.props.mbMap.loaded();
@@ -105,7 +89,6 @@ export class DrawControl extends Component<Props> {
     }
 
     this.props.mbMap.getCanvas().style.cursor = '';
-    this.props.mbMap.off('draw.modechange', this._onModeChange);
     this.props.mbMap.off('draw.create', this._onDraw);
     if (this.props.onClick) {
       this.props.mbMap.off('click', this._onClick);
@@ -118,7 +101,6 @@ export class DrawControl extends Component<Props> {
     if (!this._mbDrawControlAdded) {
       this.props.mbMap.addControl(this._mbDrawControl);
       this._mbDrawControlAdded = true;
-      this.props.mbMap.on('draw.modechange', this._onModeChange);
       this.props.mbMap.on('draw.create', this._onDraw);
 
       if (this.props.onClick) {
@@ -144,6 +126,9 @@ export class DrawControl extends Component<Props> {
       this._mbDrawControl.changeMode(DRAW_POINT);
     } else if (this.props.drawShape === DRAW_SHAPE.DELETE) {
       this._mbDrawControl.changeMode(SIMPLE_SELECT);
+    } else if (this.props.drawShape === DRAW_SHAPE.WAIT) {
+      this.props.mbMap.getCanvas().style.cursor = 'wait';
+      this._mbDrawControl.changeMode(SIMPLE_SELECT);
     } else {
       this._mbDrawControl.changeMode(SIMPLE_SELECT);
     }

diff --git a/...ic/connected_components/mb_map/draw_control/draw_feature_control/draw_feature_control.tsx b/...ic/connected_components/mb_map/draw_control/draw_feature_control/draw_feature_control.tsx
@@ -15,7 +15,7 @@ import { i18n } from '@kbn/i18n';
 import * as jsts from 'jsts';
 import { MapMouseEvent } from '@kbn/mapbox-gl';
 import { getToasts } from '../../../../kibana_services';
-import { DrawControl } from '../';
+import { DrawControl } from '../draw_control';
 import { DRAW_MODE, DRAW_SHAPE } from '../../../../../common/constants';
 import { ILayer } from '../../../../classes/layers/layer';
 import { EXCLUDE_CENTROID_FEATURES } from '../../../../classes/util/mb_filter_expressions';
@@ -29,9 +29,8 @@ export interface ReduxStateProps {
 }
 
 export interface ReduxDispatchProps {
-  addNewFeatureToIndex: (geometry: Geometry | Position[]) => void;
+  addNewFeatureToIndex: (geometries: Array<Geometry | Position[]>) => void;
   deleteFeatureFromIndex: (featureId: string) => void;
-  disableDrawState: () => void;
 }
 
 export interface OwnProps {
@@ -43,6 +42,7 @@ type Props = ReduxStateProps & ReduxDispatchProps & OwnProps;
 export class DrawFeatureControl extends Component<Props, {}> {
   _onDraw = async (e: { features: Feature[] }, mbDrawControl: MapboxDraw) => {
     try {
+      const geometries: Array<Geometry | Position[]> = [];
       e.features.forEach((feature: Feature) => {
         const { geometry } = geoJSONReader.read(feature);
         if (!geometry.isSimple() || !geometry.isValid()) {
@@ -58,9 +58,13 @@ export class DrawFeatureControl extends Component<Props, {}> {
             this.props.drawMode === DRAW_MODE.DRAW_POINTS
               ? feature.geometry.coordinates
               : feature.geometry;
-          this.props.addNewFeatureToIndex(featureGeom);
+          geometries.push(featureGeom);
         }
       });
+
+      if (geometries.length) {
+        this.props.addNewFeatureToIndex(geometries);
+      }
     } catch (error) {
       getToasts().addWarning(
         i18n.translate('xpack.maps.drawFeatureControl.unableToCreateFeature', {
@@ -71,7 +75,6 @@ export class DrawFeatureControl extends Component<Props, {}> {
         })
       );
     } finally {
-      this.props.disableDrawState();
       try {
         mbDrawControl.deleteAll();
       } catch (_e) {
@@ -86,6 +89,7 @@ export class DrawFeatureControl extends Component<Props, {}> {
     if (!this.props.editLayer || this.props.drawShape !== DRAW_SHAPE.DELETE) {
       return;
     }
+
     const mbEditLayerIds = this.props.editLayer
       .getMbLayerIds()
       .filter((mbLayerId) => !!this.props.mbMap.getLayer(mbLayerId));

diff --git a/...lugins/maps/public/connected_components/mb_map/draw_control/draw_feature_control/index.ts b/...lugins/maps/public/connected_components/mb_map/draw_control/draw_feature_control/index.ts
@@ -15,7 +15,7 @@ import {
   ReduxStateProps,
   OwnProps,
 } from './draw_feature_control';
-import { addNewFeatureToIndex, deleteFeatureFromIndex, updateEditShape } from '../../../../actions';
+import { addNewFeatureToIndex, deleteFeatureFromIndex } from '../../../../actions';
 import { MapStoreState } from '../../../../reducers/store';
 import { getEditState, getLayerById } from '../../../../selectors/map_selectors';
 import { getDrawMode } from '../../../../selectors/ui_selectors';
@@ -34,15 +34,12 @@ function mapDispatchToProps(
   dispatch: ThunkDispatch<MapStoreState, void, AnyAction>
 ): ReduxDispatchProps {
   return {
-    addNewFeatureToIndex(geometry: Geometry | Position[]) {
-      dispatch(addNewFeatureToIndex(geometry));
+    addNewFeatureToIndex(geometries: Array<Geometry | Position[]>) {
+      dispatch(addNewFeatureToIndex(geometries));
     },
     deleteFeatureFromIndex(featureId: string) {
       dispatch(deleteFeatureFromIndex(featureId));
     },
-    disableDrawState() {
-      dispatch(updateEditShape(null));
-    },
   };
 }
 

diff --git a/...blic/connected_components/mb_map/draw_control/draw_filter_control/draw_filter_control.tsx b/...blic/connected_components/mb_map/draw_control/draw_filter_control/draw_filter_control.tsx
@@ -20,7 +20,7 @@ import {
   roundCoordinates,
 } from '../../../../../common/elasticsearch_util';
 import { getToasts } from '../../../../kibana_services';
-import { DrawControl } from '../';
+import { DrawControl } from '../draw_control';
 import { DrawCircleProperties } from '../draw_circle';
 
 export interface Props {

diff --git a/x-pack/plugins/maps/public/connected_components/mb_map/draw_control/index.ts b/x-pack/plugins/maps/public/connected_components/mb_map/draw_control/index.ts
diff --git a/x-pack/plugins/maps/public/reducers/ui.ts b/x-pack/plugins/maps/public/reducers/ui.ts
@@ -19,6 +19,8 @@ import {
   SHOW_TOC_DETAILS,
   HIDE_TOC_DETAILS,
   SET_DRAW_MODE,
+  PUSH_DELETED_FEATURE_ID,
+  CLEAR_DELETED_FEATURE_IDS,
 } from '../actions';
 import { DRAW_MODE } from '../../common/constants';
 
@@ -37,6 +39,7 @@ export type MapUiState = {
   isLayerTOCOpen: boolean;
   isTimesliderOpen: boolean;
   openTOCDetails: string[];
+  deletedFeatureIds: string[];
 };
 
 export const DEFAULT_IS_LAYER_TOC_OPEN = true;
@@ -51,6 +54,7 @@ export const DEFAULT_MAP_UI_STATE = {
   // storing TOC detail visibility outside of map.layerList because its UI state and not map rendering state.
   // This also makes for easy read/write access for embeddables.
   openTOCDetails: [],
+  deletedFeatureIds: [],
 };
 
 // Reducer
@@ -82,6 +86,16 @@ export function ui(state: MapUiState = DEFAULT_MAP_UI_STATE, action: any) {
           return layerId !== action.layerId;
         }),
       };
+    case PUSH_DELETED_FEATURE_ID:
+      return {
+        ...state,
+        deletedFeatureIds: [...state.deletedFeatureIds, action.featureId],
+      };
+    case CLEAR_DELETED_FEATURE_IDS:
+      return {
+        ...state,
+        deletedFeatureIds: [],
+      };
     default:
       return state;
   }

diff --git a/x-pack/plugins/maps/public/selectors/ui_selectors.ts b/x-pack/plugins/maps/public/selectors/ui_selectors.ts
@@ -17,3 +17,4 @@ export const getIsTimesliderOpen = ({ ui }: MapStoreState): boolean => ui.isTime
 export const getOpenTOCDetails = ({ ui }: MapStoreState): string[] => ui.openTOCDetails;
 export const getIsFullScreen = ({ ui }: MapStoreState): boolean => ui.isFullScreen;
 export const getIsReadOnly = ({ ui }: MapStoreState): boolean => ui.isReadOnly;
+export const getDeletedFeatureIds = ({ ui }: MapStoreState): string[] => ui.deletedFeatureIds;