From f644e05d9e30b0a23de564a0d11464a0469d475a Mon Sep 17 00:00:00 2001 From: Matteo Velludini Date: Thu, 21 Jun 2018 17:56:57 +0200 Subject: [PATCH] Fix #77, #76, #73 circle, styler and coordinate editor bugs --- .../annotations/AnnotationsEditor.jsx | 2 +- .../annotations/CoordinatesEditor.jsx | 6 +- .../epics/__tests__/annotations-test.js | 12 ++-- web/client/epics/annotations.js | 65 ++++++++++++++++--- .../reducers/__tests__/annotations-test.js | 37 ++++++++++- web/client/reducers/annotations.js | 15 ++++- 6 files changed, 114 insertions(+), 23 deletions(-) diff --git a/web/client/components/mapcontrols/annotations/AnnotationsEditor.jsx b/web/client/components/mapcontrols/annotations/AnnotationsEditor.jsx index bfe0ce238a..05b9da6d0d 100644 --- a/web/client/components/mapcontrols/annotations/AnnotationsEditor.jsx +++ b/web/client/components/mapcontrols/annotations/AnnotationsEditor.jsx @@ -486,7 +486,7 @@ class AnnotationsEditor extends React.Component { { diff --git a/web/client/components/mapcontrols/annotations/CoordinatesEditor.jsx b/web/client/components/mapcontrols/annotations/CoordinatesEditor.jsx index 5eb9ce7978..bd6892ceb7 100644 --- a/web/client/components/mapcontrols/annotations/CoordinatesEditor.jsx +++ b/web/client/components/mapcontrols/annotations/CoordinatesEditor.jsx @@ -96,7 +96,8 @@ class CoordinateEditor extends React.Component { this.props.onChangeRadius(parseFloat(radius), this.props.components.map(coordToArray)); } else if (radius !== "") { this.props.onChangeRadius(parseFloat(radius), []); - } else if (this.props.properties.isValidFeature) { + } else { + this.props.onChangeRadius(null, this.props.components.map(coordToArray)); this.props.onSetInvalidSelected("radius", this.props.components.map(coordToArray)); } }} @@ -121,7 +122,8 @@ class CoordinateEditor extends React.Component { this.props.onChangeText(valueText, this.props.components.map(coordToArray)); } else if (valueText !== "") { this.props.onChangeText(valueText, this.props.components.map(coordToArray)); - } else if (this.props.properties.isValidFeature) { + } else { + this.props.onChangeText("", this.props.components.map(coordToArray)); this.props.onSetInvalidSelected("text", this.props.components.map(coordToArray)); } }} diff --git a/web/client/epics/__tests__/annotations-test.js b/web/client/epics/__tests__/annotations-test.js index 502e798675..2e11d1da48 100644 --- a/web/client/epics/__tests__/annotations-test.js +++ b/web/client/epics/__tests__/annotations-test.js @@ -24,13 +24,13 @@ const {TOGGLE_CONTROL, toggleControl} = require('../../actions/controls'); const {addAnnotationsLayerEpic, editAnnotationEpic, removeAnnotationEpic, saveAnnotationEpic, newAnnotationEpic, addAnnotationEpic, disableInteractionsEpic, cancelEditAnnotationEpic, startDrawingMultiGeomEpic, endDrawGeomEpic, endDrawTextEpic, cancelTextAnnotationsEpic, setStyleEpic, restoreStyleEpic, highlighAnnotationEpic, cleanHighlightAnnotationEpic, closeAnnotationsEpic, confirmCloseAnnotationsEpic, - downloadAnnotations, onLoadAnnotations, onChangedSelectedFeatureEpic, onBackToEditingFeatureEpic, redrawOnChangeRadiusTextEpic, + downloadAnnotations, onLoadAnnotations, onChangedSelectedFeatureEpic, onBackToEditingFeatureEpic, redrawOnChangeRadiusEpic, redrawOnChangeTextEpic, editSelectedFeatureEpic, editCircleFeatureEpic, closeMeasureToolEpic } = require('../annotations')({}); const rootEpic = combineEpics(addAnnotationsLayerEpic, editAnnotationEpic, removeAnnotationEpic, saveAnnotationEpic, newAnnotationEpic, addAnnotationEpic, disableInteractionsEpic, cancelEditAnnotationEpic, startDrawingMultiGeomEpic, endDrawGeomEpic, endDrawTextEpic, cancelTextAnnotationsEpic, setStyleEpic, restoreStyleEpic, highlighAnnotationEpic, cleanHighlightAnnotationEpic, closeAnnotationsEpic, confirmCloseAnnotationsEpic, - downloadAnnotations, onLoadAnnotations, onChangedSelectedFeatureEpic, onBackToEditingFeatureEpic, redrawOnChangeRadiusTextEpic, + downloadAnnotations, onLoadAnnotations, onChangedSelectedFeatureEpic, onBackToEditingFeatureEpic, redrawOnChangeRadiusEpic, redrawOnChangeTextEpic, editSelectedFeatureEpic, editCircleFeatureEpic, closeMeasureToolEpic ); const epicMiddleware = createEpicMiddleware(rootEpic); @@ -485,7 +485,7 @@ describe('annotations Epics', () => { }, selected); selected = set("properties", { id: "Polygon1"}, selected); store = mockStore( - set("annotations.selected", selected, set("annotations.editing.features", defaultState.annotations.editing.features.concat(selected), defaultState)) + set("annotations.selected", selected, set("annotations.editing.features", defaultState.annotations.editing.features.concat([selected]), defaultState)) ); store.subscribe(() => { @@ -506,9 +506,9 @@ describe('annotations Epics', () => { type: "Text", coordinates: textCoords }, selected); - selected = set("properties", { id: "text1", isText: true, valueText: "text"}, selected); + selected = set("properties", { id: "text1", isText: true, isValidFeature: true, valueText: "text"}, selected); store = mockStore( - set("annotations.selected", selected, set("annotations.editing.features", defaultState.annotations.editing.features.concat(selected), defaultState)) + set("annotations.selected", selected, set("annotations.editing.features", defaultState.annotations.editing.features.concat([selected]), defaultState)) ); store.subscribe(() => { @@ -532,7 +532,7 @@ describe('annotations Epics', () => { selected = set("geometry", polygonGeom, selected); selected = set("properties", { id: "text1", radius: 200, center: [2, 2], isCircle: true, polygonGeom}, selected); store = mockStore( - set("annotations.selected", selected, set("annotations.editing.features", defaultState.annotations.editing.features.concat(selected), defaultState)) + set("annotations.selected", selected, set("annotations.editing.features", defaultState.annotations.editing.features.concat([selected]), defaultState)) ); store.subscribe(() => { diff --git a/web/client/epics/annotations.js b/web/client/epics/annotations.js index 392a0d11b0..d05e78c475 100644 --- a/web/client/epics/annotations.js +++ b/web/client/epics/annotations.js @@ -408,20 +408,29 @@ module.exports = (viewer) => ({ selected = set("geometry.coordinates", [selected.geometry.coordinates].filter(validateCoordsArray)[0] || [], selected); } } + let method = selected.properties.isCircle ? "Circle" : selected.geometry.type; + if (selected.properties && selected.properties.isCircle) { selected = set("geometry", selected.properties.polygonGeom, selected); } - let method = selected.properties.isCircle ? "Circle" : selected.geometry.type; // TODO update selected feature in editing features let selectedIndex = findIndex(feature.features, (f) => f.properties.id === selected.properties.id); - if (selectedIndex === -1) { - feature = set(`features`, feature.features.concat([selected]), feature); - - } else { - feature = set(`features[${selectedIndex}]`, selected, feature); + if (selected.properties.isValidFeature || selected.geometry.type === "LineString" || selected.geometry.type === "Polygon") { + if (selectedIndex === -1) { + feature = set(`features`, feature.features.concat([selected]), feature); + } else { + feature = set(`features[${selectedIndex}]`, selected, feature); + } + } + if (selectedIndex !== -1 && !selected.properties.isValidFeature && (selected.geometry.type !== "LineString" && selected.geometry.type !== "Polygon")) { + feature = set(`features`, feature.features.filter((f, i) => i !== selectedIndex ), feature); } + + /*if (!selected.properties.isValidFeature) { + feature = feature.features.filter((f, i) => selectedIndex !== i); + }*/ const multiGeometry = state.annotations.config.multiGeometry; const style = feature.style; const action = changeDrawingStatus("drawOrEdit", method, "annotations", [feature], { @@ -453,8 +462,8 @@ module.exports = (viewer) => ({ }, assign({}, style, {highlight: false})); return Rx.Observable.of(action); }), - redrawOnChangeRadiusTextEpic: (action$, {getState}) => action$.ofType( CHANGE_RADIUS, CHANGE_TEXT ) - .switchMap((a) => { + redrawOnChangeTextEpic: (action$, {getState}) => action$.ofType( CHANGE_TEXT ) + .switchMap(() => { const state = getState(); let feature = state.annotations.editing; let selected = state.annotations.selected; @@ -464,13 +473,51 @@ module.exports = (viewer) => ({ selected = set("geometry.coordinates", [selected.geometry.coordinates].filter(validateCoordsArray)[0] || [], selected); selected = set("geometry.type", "Point", selected); let selectedIndex = findIndex(feature.features, (f) => f.properties.id === selected.properties.id); + if (selected.properties.isValidFeature) { + if (selectedIndex === -1) { + feature = set(`features`, feature.features.concat([selected]), feature); + } else { + feature = set(`features[${selectedIndex}]`, selected, feature); + } + } + const action = changeDrawingStatus("drawOrEdit", "Text", "annotations", [feature], { + featureProjection: "EPSG:4326", + stopAfterDrawing: !multiGeometry, + editEnabled: true, + translateEnabled: false, + editFilter: (f) => f.getProperties().canEdit, + drawEnabled: false, + useSelectedStyle: true, + transformToFeatureCollection: true + }, assign({}, style, {highlight: false})); + return Rx.Observable.of(action); + }), + redrawOnChangeRadiusEpic: (action$, {getState}) => action$.ofType( CHANGE_RADIUS ) + .switchMap(() => { + const state = getState(); + let feature = state.annotations.editing; + let selected = state.annotations.selected; + const multiGeometry = state.annotations.config.multiGeometry; + const style = feature.style; + + // selected = set("geometry.coordinates", [selected.geometry.coordinates].filter(validateCoordsArray)[0] || [], selected); + // selected = set("geometry.type", "Polygon", selected); + let selectedIndex = findIndex(feature.features, (f) => f.properties.id === selected.properties.id); + if (!selected.properties.isValidFeature) { + selected = set("geometry", { + type: "Polygon", + coordinates: [[]] + }, selected); + } else { + selected = set("geometry", selected.properties.polygonGeom, selected); + } if (selectedIndex === -1) { feature = set(`features`, feature.features.concat([selected]), feature); } else { feature = set(`features[${selectedIndex}]`, selected, feature); } // this should run only if the feature has a valid geom - const action = changeDrawingStatus("drawOrEdit", a.type === CHANGE_TEXT ? "Text" : "Circle", "annotations", [feature], { + const action = changeDrawingStatus("drawOrEdit", "Circle", "annotations", [feature], { featureProjection: "EPSG:4326", stopAfterDrawing: !multiGeometry, editEnabled: true, diff --git a/web/client/reducers/__tests__/annotations-test.js b/web/client/reducers/__tests__/annotations-test.js index bc0eec012a..ef6e3a5e95 100644 --- a/web/client/reducers/__tests__/annotations-test.js +++ b/web/client/reducers/__tests__/annotations-test.js @@ -804,8 +804,41 @@ describe('Test the annotations reducer', () => { expect(state.selected).toBe(null); expect(state.drawing).toBe(false); expect(state.showUnsavedGeometryModal).toBe(false); - expect(state.editing.features.length).toBe(1); - expect(state.editing.features[0].geometry.coordinates[0]).toBe(1); + expect(state.editing.features.length).toBe(0); + + }); + + it('resetCoordEditor in edit mode of a Circle, with no Changes ', () => { + + const featureChanged = { + properties: { + id: '1', + isCircle: true + }, + geometry: { + type: "Point", + coordinates: [10, 1] + } + }; + const featureColl = { + type: "FeatureCollection", + features: [], + properties: { + id: '1asdfads' + }, + featureType: "Circle", + style: {} + }; + const state = annotations({ + editing: featureColl, + selected: featureChanged, + unsavedGeometry: false + }, resetCoordEditor()); + expect(state.unsavedGeometry).toBe(false); + expect(state.selected).toBe(null); + expect(state.drawing).toBe(false); + expect(state.showUnsavedGeometryModal).toBe(false); + expect(state.editing.features.length).toBe(0); }); diff --git a/web/client/reducers/annotations.js b/web/client/reducers/annotations.js index eb7dc670ae..d598ba4c43 100644 --- a/web/client/reducers/annotations.js +++ b/web/client/reducers/annotations.js @@ -25,7 +25,7 @@ const {REMOVE_ANNOTATION, CONFIRM_REMOVE_ANNOTATION, CANCEL_REMOVE_ANNOTATION, C const {validateCoordsArray, getAvailableStyler, DEFAULT_ANNOTATIONS_STYLES, convertGeoJSONToInternalModel, addIds, validateFeature, getComponents} = require('../utils/AnnotationsUtils'); const {set} = require('../utils/ImmutableUtils'); -const {head, includes, findIndex, isNil} = require('lodash'); +const {head, includes, findIndex, isNil, slice} = require('lodash'); const uuid = require('uuid'); @@ -219,12 +219,18 @@ function annotations(state = { validationErrors: {} }, action) { case CHANGE_RADIUS: { let newState; let selected = set("properties.radius", action.radius, state.selected); - if (action.components.length === 0) { + if (action.components.length === 0 || action.radius === null) { + selected = set("properties.isValidFeature", false, selected); return assign({}, state, { selected, unsavedChanges: true }); } + selected = set("properties.isValidFeature", validateFeature({ + properties: selected.properties, + components: getComponents({coordinates: action.components[0] || [], type: "Circle"}), + type: selected.geometry.type + }), selected); selected = set("properties.center", action.components[0], selected); selected = set("geometry.coordinates", action.components[0], selected); let center = reproject(selected.properties.center, "EPSG:4326", "EPSG:3857"); @@ -291,7 +297,10 @@ function annotations(state = { validationErrors: {} }, action) { let newState = set(`editing.features`, state.editing.features.map(f => { return set("properties.canEdit", false, f); }), state); - const features = state.unsavedGeometry ? state.editing.tempFeatures : newState.editing.features; + const oldfeatures = newState.editing.features; + // only for the circles the feature is not being added + const features = state.unsavedGeometry ? state.editing.tempFeatures : + (newState.featureType !== "Circle" ? slice(oldfeatures, 0, oldfeatures.length - 1) : oldfeatures); return assign({}, newState, { editing: { ...newState.editing,