[7.x] [maps] fix drawing shapes (#74689)

x-pack/package.json

@@ -218,7 +218,7 @@
     "@kbn/interpreter": "1.0.0",
     "@kbn/ui-framework": "1.0.0",
     "@mapbox/geojson-rewind": "^0.4.1",
-    "@mapbox/mapbox-gl-draw": "^1.1.2",
+    "@mapbox/mapbox-gl-draw": "^1.2.0",
     "@mapbox/mapbox-gl-rtl-text": "^0.2.3",
     "@scant/router": "^0.1.0",
     "@slack/webhook": "^5.0.0",
@@ -398,4 +398,4 @@
       "cypress-multi-reporters"
     ]
   }
-}
+}

x-pack/plugins/maps/public/connected_components/map/mb/draw_control/draw_control.js

@@ -18,9 +18,12 @@ import {
 } from '../../../../elasticsearch_geo_utils';
 import { DrawTooltip } from './draw_tooltip';
 
+const DRAW_RECTANGLE = 'draw_rectangle';
+const DRAW_CIRCLE = 'draw_circle';
+
 const mbDrawModes = MapboxDraw.modes;
-mbDrawModes.draw_rectangle = DrawRectangle;
-mbDrawModes.draw_circle = DrawCircle;
+mbDrawModes[DRAW_RECTANGLE] = DrawRectangle;
+mbDrawModes[DRAW_CIRCLE] = DrawCircle;
 
 export class DrawControl extends React.Component {
   constructor() {
@@ -45,8 +48,10 @@ export class DrawControl extends React.Component {
     this._removeDrawControl();
   }
 
+  // debounce with zero timeout needed to allow mapbox-draw finish logic to complete
+  // before _removeDrawControl is called
   _syncDrawControl = _.debounce(() => {
-    if (!this.props.mbMap) {
+    if (!this._isMounted) {
       return;
     }
 
@@ -55,7 +60,7 @@ export class DrawControl extends React.Component {
     } else {
       this._removeDrawControl();
     }
-  }, 256);
+  }, 0);
 
   _onDraw = (e) => {
     if (!e.features.length) {
@@ -118,7 +123,7 @@ export class DrawControl extends React.Component {
   };
 
   _removeDrawControl() {
-    if (!this._mbDrawControlAdded) {
+    if (!this.props.mbMap || !this._mbDrawControlAdded) {
       return;
     }
 
@@ -129,18 +134,26 @@ export class DrawControl extends React.Component {
   }
 
   _updateDrawControl() {
+    if (!this.props.mbMap) {
+      return;
+    }
+
     if (!this._mbDrawControlAdded) {
       this.props.mbMap.addControl(this._mbDrawControl);
       this._mbDrawControlAdded = true;
       this.props.mbMap.getCanvas().style.cursor = 'crosshair';
       this.props.mbMap.on('draw.create', this._onDraw);
     }
 
-    if (this.props.drawState.drawType === DRAW_TYPE.BOUNDS) {
-      this._mbDrawControl.changeMode('draw_rectangle');
-    } else if (this.props.drawState.drawType === DRAW_TYPE.DISTANCE) {
-      this._mbDrawControl.changeMode('draw_circle');
-    } else if (this.props.drawState.drawType === DRAW_TYPE.POLYGON) {
+    const drawMode = this._mbDrawControl.getMode();
+    if (drawMode !== DRAW_RECTANGLE && this.props.drawState.drawType === DRAW_TYPE.BOUNDS) {
+      this._mbDrawControl.changeMode(DRAW_RECTANGLE);
+    } else if (drawMode !== DRAW_CIRCLE && this.props.drawState.drawType === DRAW_TYPE.DISTANCE) {
+      this._mbDrawControl.changeMode(DRAW_CIRCLE);
+    } else if (
+      drawMode !== this._mbDrawControl.modes.DRAW_POLYGON &&
+      this.props.drawState.drawType === DRAW_TYPE.POLYGON
+    ) {
       this._mbDrawControl.changeMode(this._mbDrawControl.modes.DRAW_POLYGON);
     }
   }

x-pack/plugins/maps/public/routing/routes/maps_app/index.js

@@ -14,7 +14,7 @@ import {
   getRefreshConfig,
   getTimeFilters,
   hasDirtyState,
-  hasUnsavedChanges,
+  getLayerListConfigOnly,
 } from '../../../selectors/map_selectors';
 import {
   replaceLayerList,
@@ -45,9 +45,7 @@ function mapStateToProps(state = {}) {
     flyoutDisplay: getFlyoutDisplay(state),
     refreshConfig: getRefreshConfig(state),
     filters: getFilters(state),
-    hasUnsavedChanges: (savedMap, initialLayerListConfig) => {
-      return hasUnsavedChanges(state, savedMap, initialLayerListConfig);
-    },
+    layerListConfigOnly: getLayerListConfigOnly(state),
     query: getQuery(state),
     timeFilters: getTimeFilters(state),
   };

x-pack/plugins/maps/public/routing/routes/maps_app/maps_app_view.js

@@ -103,7 +103,15 @@ export class MapsAppView extends React.Component {
   }
 
   _hasUnsavedChanges() {
-    return this.props.hasUnsavedChanges(this.props.savedMap, this.state.initialLayerListConfig);
+    const savedLayerList = this.props.savedMap.getLayerList();
+    return !savedLayerList
+      ? !_.isEqual(this.props.layerListConfigOnly, this.state.initialLayerListConfig)
+      : // savedMap stores layerList as a JSON string using JSON.stringify.
+        // JSON.stringify removes undefined properties from objects.
+        // savedMap.getLayerList converts the JSON string back into Javascript array of objects.
+        // Need to perform the same process for layerListConfigOnly to compare apples to apples
+        // and avoid undefined properties in layerListConfigOnly triggering unsaved changes.
+        !_.isEqual(JSON.parse(JSON.stringify(this.props.layerListConfigOnly)), savedLayerList);
   }
 
   _setBreadcrumbs = () => {
@@ -356,22 +364,20 @@ export class MapsAppView extends React.Component {
     );
   }
 
-  render() {
-    const { filters, isFullScreen } = this.props;
+  _addFilter = (newFilters) => {
+    newFilters.forEach((filter) => {
+      filter.$state = { store: esFilters.FilterStateStore.APP_STATE };
+    });
+    this._onFiltersChange([...this.props.filters, ...newFilters]);
+  };
 
+  render() {
     return this.state.initialized ? (
-      <div id="maps-plugin" className={isFullScreen ? 'mapFullScreen' : ''}>
+      <div id="maps-plugin" className={this.props.isFullScreen ? 'mapFullScreen' : ''}>
         {this._renderTopNav()}
         <h1 className="euiScreenReaderOnly">{`screenTitle placeholder`}</h1>
         <div id="react-maps-root">
-          <MapContainer
-            addFilters={(newFilters) => {
-              newFilters.forEach((filter) => {
-                filter.$state = { store: esFilters.FilterStateStore.APP_STATE };
-              });
-              this._onFiltersChange([...filters, ...newFilters]);
-            }}
-          />
+          <MapContainer addFilters={this._addFilter} />
         </div>
       </div>
     ) : null;

x-pack/plugins/maps/public/selectors/map_selectors.ts

@@ -52,7 +52,6 @@ import { ISource } from '../classes/sources/source';
 import { ITMSSource } from '../classes/sources/tms_source';
 import { IVectorSource } from '../classes/sources/vector_source';
 import { ILayer } from '../classes/layers/layer';
-import { ISavedGisMap } from '../routing/bootstrap/services/saved_gis_map';
 
 function createLayerInstance(
   layerDescriptor: LayerDescriptor,
@@ -298,6 +297,10 @@ export const getLayerList = createSelector(
   }
 );
 
+export const getLayerListConfigOnly = createSelector(getLayerListRaw, (layerDescriptorList) => {
+  return copyPersistentState(layerDescriptorList);
+});
+
 export function getLayerById(layerId: string | null, state: MapStoreState): ILayer | undefined {
   return getLayerList(state).find((layer) => {
     return layerId === layer.getId();
@@ -417,22 +420,3 @@ export const areLayersLoaded = createSelector(
     return true;
   }
 );
-
-export function hasUnsavedChanges(
-  state: MapStoreState,
-  savedMap: ISavedGisMap,
-  initialLayerListConfig: LayerDescriptor[]
-) {
-  const layerListConfigOnly = copyPersistentState(getLayerListRaw(state));
-
-  const savedLayerList = savedMap.getLayerList();
-
-  return !savedLayerList
-    ? !_.isEqual(layerListConfigOnly, initialLayerListConfig)
-    : // savedMap stores layerList as a JSON string using JSON.stringify.
-      // JSON.stringify removes undefined properties from objects.
-      // savedMap.getLayerList converts the JSON string back into Javascript array of objects.
-      // Need to perform the same process for layerListConfigOnly to compare apples to apples
-      // and avoid undefined properties in layerListConfigOnly triggering unsaved changes.
-      !_.isEqual(JSON.parse(JSON.stringify(layerListConfigOnly)), savedLayerList);
-}

yarn.lock

@@ -3057,10 +3057,10 @@
   resolved "https://registry.yarnpkg.com/@mapbox/geojson-types/-/geojson-types-1.0.2.tgz#9aecf642cb00eab1080a57c4f949a65b4a5846d6"
   integrity sha512-e9EBqHHv3EORHrSfbR9DqecPNn+AmuAoQxV6aL8Xu30bJMJR1o8PZLZzpk1Wq7/NfCbuhmakHTPYRhoqLsXRnw==
 
-"@mapbox/geojsonhint@^2.0.0":
-  version "2.2.0"
-  resolved "https://registry.yarnpkg.com/@mapbox/geojsonhint/-/geojsonhint-2.2.0.tgz#75ca94706e9a56e6debf4e1c78fabdc67978b883"
-  integrity sha512-8qQYRB+/2z2JsN5s6D0WAnpo69+3V3nvJsSFLwMB1dsaWz1V4oZeuoje9srbYAxxL8PXCwIywfhYa3GxOkBv5Q==
+"@mapbox/geojsonhint@3.0.0":
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/@mapbox/geojsonhint/-/geojsonhint-3.0.0.tgz#42448232ce4236cb89c1b69c36b0cadeac99e02e"
+  integrity sha512-zHcyh1rDHYnEBd6NvOWoeHLuvazlDkIjvz9MJx4cKwcKTlfrqgxVnTv1QLnVJnsSU5neJnhQJcgscR/Zl4uYgw==
   dependencies:
     concat-stream "^1.6.1"
     jsonlint-lines "1.7.1"
@@ -3073,16 +3073,17 @@
   resolved "https://registry.yarnpkg.com/@mapbox/jsonlint-lines-primitives/-/jsonlint-lines-primitives-2.0.2.tgz#ce56e539f83552b58d10d672ea4d6fc9adc7b234"
   integrity sha1-zlblOfg1UrWNENZy6k1vya3HsjQ=
 
-"@mapbox/mapbox-gl-draw@^1.1.2":
-  version "1.1.2"
-  resolved "https://registry.yarnpkg.com/@mapbox/mapbox-gl-draw/-/mapbox-gl-draw-1.1.2.tgz#247b3f0727db34c2641ab718df5eebeee69a2585"
-  integrity sha512-DWtATUAnJaGZYoH/y2O+QTRybxrp5y3w3eV5FXHFNVcKsCAojKEMB8ALKUG2IsiCKqV/JCAguK9AlPWR7Bjafw==
+"@mapbox/mapbox-gl-draw@^1.2.0":
+  version "1.2.0"
+  resolved "https://registry.yarnpkg.com/@mapbox/mapbox-gl-draw/-/mapbox-gl-draw-1.2.0.tgz#b6e5278afef65bd5d7d92366034997768e478ad9"
+  integrity sha512-gMrP2zn8PzDtrs72FMJTPytCumX5vUn9R7IK38qBOVy9UfqbdWr56KYuNA/2X+jKn4FIOpmWf8CWkKpOaQkv7w==
   dependencies:
     "@mapbox/geojson-area" "^0.2.1"
     "@mapbox/geojson-extent" "^0.3.2"
     "@mapbox/geojson-normalize" "0.0.1"
-    "@mapbox/geojsonhint" "^2.0.0"
+    "@mapbox/geojsonhint" "3.0.0"
     "@mapbox/point-geometry" "0.1.0"
+    eslint-plugin-import "^2.19.1"
     hat "0.0.3"
     lodash.isequal "^4.2.0"
     xtend "^4.0.1"