Skip to content

Commit

Permalink
[maps] fix drawing shapes (elastic#74689)
Browse files Browse the repository at this point in the history 
* [maps] fix drawing shapes

* prevent re-render on state change
  • Loading branch information
@nreese
nreese committed Aug 11, 2020
1 parent 6c89fd1 commit 8d506b1
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 57 deletions.
4 changes: 2 additions & 2 deletions x-pack/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -398,4 +398,4 @@
"cypress-multi-reporters"
]
}
}
}
33 changes: 23 additions & 10 deletions x-pack/plugins/maps/public/connected_components/map/mb/draw_control/draw_control.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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;
}

Expand All @@ -55,7 +60,7 @@ export class DrawControl extends React.Component {
} else {
this._removeDrawControl();
}
}, 256);
}, 0);

_onDraw = (e) => {
if (!e.features.length) {
Expand Down Expand Up @@ -118,7 +123,7 @@ export class DrawControl extends React.Component {
};

_removeDrawControl() {
if (!this._mbDrawControlAdded) {
if (!this.props.mbMap || !this._mbDrawControlAdded) {
return;
}

Expand All @@ -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);
}
}
Expand Down
6 changes: 2 additions & 4 deletions x-pack/plugins/maps/public/routing/routes/maps_app/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
getRefreshConfig,
getTimeFilters,
hasDirtyState,
hasUnsavedChanges,
getLayerListConfigOnly,
} from '../../../selectors/map_selectors';
import {
replaceLayerList,
Expand Down Expand Up @@ -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),
};
Expand Down
30 changes: 18 additions & 12 deletions x-pack/plugins/maps/public/routing/routes/maps_app/maps_app_view.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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;
Expand Down
24 changes: 4 additions & 20 deletions x-pack/plugins/maps/public/selectors/map_selectors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
19 changes: 10 additions & 9 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down

0 comments on commit 8d506b1

Please sign in to comment.