Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 31 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
acd69fc
#117369 - Fixed double click issue when deleting a shape
maksimkovalev Feb 4, 2022
e17413f
#117369 - updated deleting shape method; redux changes;
maksimkovalev Feb 8, 2022
e319e98
Merge branch 'main' into e-117369
maksimkovalev Feb 8, 2022
44d66e9
#117369 - waiting state for adding shapes feature
maksimkovalev Feb 9, 2022
13d9be4
Merge branch 'e-117369' of https://github.com/maksimkovalev/kibana in…
maksimkovalev Feb 9, 2022
3a2859d
Merge branch 'main' into e-117369
maksimkovalev Feb 9, 2022
6dcd059
Merge branch 'main' into e-117369
maksimkovalev Feb 14, 2022
e8fb91e
Merge branch 'main' into e-117369
kibanamachine Feb 14, 2022
24b00a6
#1173669 - refactoring; removed unused functions
maksimkovalev Feb 15, 2022
cdd8361
117369 - refactoring
maksimkovalev Feb 15, 2022
e047171
Merge branch 'main' into e-117369
kibanamachine Feb 15, 2022
8c1620b
117369 - Updates for onDraw methods, now the selected shape is not reset
maksimkovalev Feb 17, 2022
0aa8a5f
Merge branch 'main' into e-117369
maksimkovalev Feb 17, 2022
86524fc
117369 - made addNewFeatureToIndex to be called once
maksimkovalev Feb 18, 2022
93d1f94
117369 - refactoring
maksimkovalev Feb 21, 2022
f9174a9
Merge branch 'main' into e-117369
maksimkovalev Feb 21, 2022
94a0e7a
Merge branch 'main' into e-117369
kibanamachine Feb 22, 2022
dfe0712
117369 - refactoring and clean up
maksimkovalev Feb 22, 2022
0b44f9a
Merge branch 'main' into e-117369
maksimkovalev Feb 22, 2022
0a31a8e
117369 - removed then method from actions
maksimkovalev Feb 23, 2022
6bf28e5
117369 - refactoring
maksimkovalev Feb 25, 2022
4496b06
Merge branch 'main' into e-117369
kibanamachine Feb 25, 2022
3e03912
Merge branch 'main' into e-117369
kibanamachine Feb 28, 2022
3f64ce0
1177369 - refactoring
maksimkovalev Mar 1, 2022
341e3bf
Merge branch 'main' into e-117369
kibanamachine Mar 2, 2022
b4b627f
117369 - refactoring; Added new state in redux
maksimkovalev Mar 4, 2022
25679b1
Merge branch 'main' into e-117369
maksimkovalev Mar 4, 2022
1d1f08c
Merge branch 'main' into e-117369
kibanamachine Mar 4, 2022
a749222
117369 - refactoring
maksimkovalev Mar 7, 2022
4b4ffc2
Merge branch 'main' into e-117369
maksimkovalev Mar 7, 2022
ca6b6fa
117369 - renaming layerId to featureId
maksimkovalev Mar 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/plugins/maps/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export enum DRAW_SHAPE {
LINE = 'LINE',
SIMPLE_SELECT = 'SIMPLE_SELECT',
DELETE = 'DELETE',
WAIT = 'WAIT',
}

export const AGG_DELIMITER = '_of_';
Expand Down
24 changes: 21 additions & 3 deletions x-pack/plugins/maps/public/actions/map_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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, setDeletedFeatures, removeDeletedFeatures } from './ui_actions';
maksimkovalev marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -321,6 +323,10 @@ export function updateEditShape(shapeToDraw: DRAW_SHAPE | null) {
drawShape: shapeToDraw,
},
});

if (shapeToDraw !== DRAW_SHAPE.DELETE) {
dispatch(removeDeletedFeatures());
}
};
}

Expand Down Expand Up @@ -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
Expand All @@ -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, {
Expand All @@ -378,6 +387,7 @@ export function addNewFeatureToIndex(geometry: Geometry | Position[]) {
}),
});
}
dispatch(updateEditShape(DRAW_SHAPE.SIMPLE_SELECT));
};
}

Expand All @@ -386,6 +396,10 @@ export function deleteFeatureFromIndex(featureId: string) {
dispatch: ThunkDispatch<MapStoreState, void, AnyAction>,
getState: () => MapStoreState
) => {
if (getDeletedFeatureIds(getState()).includes(featureId)) {
maksimkovalev marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const editState = getEditState(getState());
const layerId = editState ? editState.layerId : undefined;
if (!layerId) {
Expand All @@ -395,8 +409,11 @@ export function deleteFeatureFromIndex(featureId: string) {
if (!layer || !isVectorLayer(layer)) {
return;
}

try {
dispatch(updateEditShape(DRAW_SHAPE.WAIT));
await (layer as IVectorLayer).deleteFeature(featureId);
dispatch(setDeletedFeatures(featureId));
await dispatch(syncDataForLayerDueToDrawing(layer));
} catch (e) {
getToasts().addError(e, {
Expand All @@ -405,5 +422,6 @@ export function deleteFeatureFromIndex(featureId: string) {
}),
});
}
dispatch(updateEditShape(DRAW_SHAPE.DELETE));
};
}
15 changes: 15 additions & 0 deletions x-pack/plugins/maps/public/actions/ui_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 SET_DELETED_FEATURES = 'UPDATE_DELETED_FEATURES';
maksimkovalev marked this conversation as resolved.
Show resolved Hide resolved
export const REMOVE_DELETED_FEATURES = 'REMOVE_DELETED_FEATURES';

export function exitFullScreen() {
return {
Expand Down Expand Up @@ -123,3 +125,16 @@ export function closeTimeslider() {
dispatch(setQuery({ clearTimeslice: true }));
};
}

export function setDeletedFeatures(layerId: string) {
maksimkovalev marked this conversation as resolved.
Show resolved Hide resolved
return {
type: SET_DELETED_FEATURES,
layerId,
};
}

export function removeDeletedFeatures() {
return {
type: REMOVE_DELETED_FEATURES,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,7 +32,6 @@ export interface Props {
onClick?: (event: MapMouseEvent, drawControl?: MapboxDraw) => void;
mbMap: MbMap;
enable: boolean;
updateEditShape: (shapeToDraw: DRAW_SHAPE) => void;
nreese marked this conversation as resolved.
Show resolved Hide resolved
}

export class DrawControl extends Component<Props> {
Expand Down Expand Up @@ -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)!);
maksimkovalev marked this conversation as resolved.
Show resolved Hide resolved
}
};

_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();
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {
Expand All @@ -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()) {
Expand All @@ -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', {
Expand All @@ -71,7 +75,6 @@ export class DrawFeatureControl extends Component<Props, {}> {
})
);
} finally {
maksimkovalev marked this conversation as resolved.
Show resolved Hide resolved
this.props.disableDrawState();
try {
mbDrawControl.deleteAll();
} catch (_e) {
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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));
},
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

This file was deleted.

14 changes: 14 additions & 0 deletions x-pack/plugins/maps/public/reducers/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {
SHOW_TOC_DETAILS,
HIDE_TOC_DETAILS,
SET_DRAW_MODE,
SET_DELETED_FEATURES,
REMOVE_DELETED_FEATURES,
} from '../actions';
import { DRAW_MODE } from '../../common/constants';

Expand All @@ -37,6 +39,7 @@ export type MapUiState = {
isLayerTOCOpen: boolean;
isTimesliderOpen: boolean;
openTOCDetails: string[];
deletedFeatureIds: string[];
};

export const DEFAULT_IS_LAYER_TOC_OPEN = true;
Expand All @@ -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
Expand Down Expand Up @@ -82,6 +86,16 @@ export function ui(state: MapUiState = DEFAULT_MAP_UI_STATE, action: any) {
return layerId !== action.layerId;
}),
};
case SET_DELETED_FEATURES:
return {
...state,
deletedFeatureIds: [...state.deletedFeatureIds, action.layerId],
};
case REMOVE_DELETED_FEATURES:
return {
...state,
deletedFeatureIds: [],
};
default:
return state;
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/selectors/ui_selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;