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

Fix #3601 measure updates #3602

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 50 additions & 2 deletions web/client/actions/__tests__/measurement-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ const {
changeMeasurementState, CHANGE_MEASUREMENT_STATE,
resetGeometry, RESET_GEOMETRY,
changeUom, CHANGE_UOM,
changeGeometry, CHANGED_GEOMETRY
changeFormatMeasurement, CHANGE_FORMAT,
init, INIT,
changeGeometry, CHANGED_GEOMETRY,
changeCoordinates, CHANGE_COORDINATES,
addAnnotation, ADD_MEASURE_AS_ANNOTATION
} = require('../measurement');
const feature = {type: "Feature", geometry: {
coordinates: [],
Expand Down Expand Up @@ -64,5 +68,49 @@ describe('Test correctness of measurement actions', () => {
expect(retval.type).toBe(CHANGE_MEASUREMENT_STATE);
expect(retval.feature.geometry.type).toBe("LineString");
});

it('Test init action creator', () => {
const defaultOptions = { showAddAsAnnotation: true};
const retval = init(defaultOptions);
expect(retval).toExist();
expect(retval.type).toBe(INIT);
expect(retval.defaultOptions).toEqual(defaultOptions);
});
it('Test changeFormatMeasurement action creator', () => {
const format = "decimal";
const retval = changeFormatMeasurement(format);
expect(retval).toExist();
expect(retval.type).toBe(CHANGE_FORMAT);
expect(retval.format).toEqual(format);
});
it('Test addAnnotation action creator', () => {
const ft = {
type: "Feature",
gometry: {
type: "LineString",
coordinates: [[1, 2], [2, 5]]
},
properties: {}
};
const value = "4";
const uom = "km";
const measureTool = "LineString";
const retval = addAnnotation(
ft,
value,
uom,
measureTool);
expect(retval).toExist();
expect(retval.type).toBe(ADD_MEASURE_AS_ANNOTATION);
expect(retval.feature).toEqual(ft);
expect(retval.value).toEqual(value);
expect(retval.uom).toEqual(uom);
expect(retval.measureTool).toEqual(measureTool);
});
it('Test addAnnotation action creator', () => {
const coordinates = [[1, 2], [2, 5]];
const retval = changeCoordinates(coordinates);
expect(retval).toExist();
expect(retval.type).toBe(CHANGE_COORDINATES);
expect(retval.coordinates).toEqual(coordinates);
});
});
39 changes: 39 additions & 0 deletions web/client/actions/measurement.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,23 @@ const CHANGE_MEASUREMENT_STATE = 'CHANGE_MEASUREMENT_STATE';
const CHANGE_UOM = 'MEASUREMENT:CHANGE_UOM';
const CHANGED_GEOMETRY = 'MEASUREMENT:CHANGED_GEOMETRY';
const RESET_GEOMETRY = 'MEASUREMENT:RESET_GEOMETRY';
const CHANGE_FORMAT = 'MEASUREMENT:CHANGE_FORMAT';
const CHANGE_COORDINATES = 'MEASUREMENT:CHANGE_COORDINATES';
const ADD_MEASURE_AS_ANNOTATION = 'MEASUREMENT:ADD_MEASURE_AS_ANNOTATION';
const INIT = 'MEASUREMENT:INIT';

/**
* trigger the epic to add the measure feature into an annotation.
*/
function addAnnotation(feature, value, uom, measureTool) {
return {
type: ADD_MEASURE_AS_ANNOTATION,
feature,
value,
uom,
measureTool
};
}

// TODO: the measurement control should use the "controls" state
function toggleMeasurement(measurement) {
Expand Down Expand Up @@ -45,6 +62,18 @@ function changeGeometry(feature) {
feature
};
}
function changeFormatMeasurement(format) {
return {
type: CHANGE_FORMAT,
format
};
}
function changeCoordinates(coordinates) {
return {
type: CHANGE_COORDINATES,
coordinates
};
}
function resetGeometry() {
return {
type: RESET_GEOMETRY
Expand All @@ -67,13 +96,23 @@ function changeMeasurementState(measureState) {
feature: measureState.feature
};
}
function init(defaultOptions = {}) {
return {
type: INIT,
defaultOptions
};
}

module.exports = {
CHANGE_MEASUREMENT_TOOL,
CHANGE_MEASUREMENT_STATE,
changeUom, CHANGE_UOM,
changeGeometry, CHANGED_GEOMETRY,
changeFormatMeasurement, CHANGE_FORMAT,
changeCoordinates, CHANGE_COORDINATES,
resetGeometry, RESET_GEOMETRY,
addAnnotation, ADD_MEASURE_AS_ANNOTATION,
init, INIT,
changeMeasurement,
toggleMeasurement,
changeMeasurementState
Expand Down
8 changes: 6 additions & 2 deletions web/client/components/map/leaflet/Feature.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,19 @@ class Feature extends React.Component {

componentWillReceiveProps(newProps) {
// TODO check if shallow comparison is enough properties and geometry
if (isEqual(newProps.properties, this.props.properties) || isEqual(newProps.geometry, this.props.geometry) || (newProps.features !== this.props.features) || (newProps.style !== this.props.style)) {
if (
!isEqual(newProps.properties, this.props.properties) ||
!isEqual(newProps.geometry, this.props.geometry) ||
(newProps.features !== this.props.features) ||
(newProps.style !== this.props.style)) {
newProps.container.removeLayer(this._layer);
this.createLayer(newProps);
}
}

shouldComponentUpdate(nextProps) {
// TODO check if shallow comparison is enough properties and geometry
return isEqual(nextProps.properties, this.props.properties) || isEqual(nextProps.geometry, this.props.geometry) || (nextProps.features !== this.props.features);
return !isEqual(nextProps.properties, this.props.properties) || !isEqual(nextProps.geometry, this.props.geometry) || (nextProps.features !== this.props.features);
}

componentWillUnmount() {
Expand Down
9 changes: 8 additions & 1 deletion web/client/components/map/leaflet/MeasurementSupport.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ class MeasurementSupport extends React.Component {
metric: PropTypes.bool,
feet: PropTypes.bool,
nautic: PropTypes.bool,
enabled: PropTypes.bool,
useTreshold: PropTypes.bool,
projection: PropTypes.string,
measurement: PropTypes.object,
Expand Down Expand Up @@ -204,7 +205,13 @@ class MeasurementSupport extends React.Component {
const uomOptions = this.uomAreaOptions(newProps);
this.drawControl.setOptions({...uomOptions, uom: newProps.uom});
}
if (newProps.measurement.geomType && newProps.measurement.geomType !== this.props.measurement.geomType) {
if (newProps.measurement.geomType && newProps.measurement.geomType !== this.props.measurement.geomType ||
/* check also when a default is set
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not too much clear...
what does it means? What's a default?

This checks if geomType exists and existed before, then if at least one of the measure types is enabled, and if enabled is switching from false to true.

It looks like you're triggering addDrawInteraction when the user enable the measure,ensuring that geomType exists and measurement type is set. Not clear why you check the old geomType existence.
Can you be more clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default i meant that when you setup that for example "Bearing" must be selected and active when the Measure tool is opened

In the second condition we can remove the the check for this.props.measurement.geomType

* if so the first condition does not match
* because the old geomType is not changed (it was already defined as default)
* and the measure tool is getting enabled
*/
(newProps.measurement.geomType && this.props.measurement.geomType && (newProps.measurement.lineMeasureEnabled || newProps.measurement.areaMeasureEnabled || newProps.measurement.bearingMeasureEnabled) && !this.props.enabled && newProps.enabled) ) {
this.addDrawInteraction(newProps);
}
if (!newProps.measurement.geomType) {
Expand Down
13 changes: 6 additions & 7 deletions web/client/components/map/openlayers/DrawSupport.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,7 @@ class DrawSupport extends React.Component {
return evtKey;
};


addDrawOrEditInteractions = (newProps) => {
if (this.state && this.state.keySingleClickCallback) {
ol.Observable.unByKey(this.state.keySingleClickCallback);
Expand Down Expand Up @@ -774,7 +775,7 @@ class DrawSupport extends React.Component {
olFt.setProperties(omit(previousFt && previousFt.getProperties() || {}, "geometry"));
break;
}
case "LineString": {
case "LineString": case "MultiPoint": {
actualCoords = previousCoords.length ? [...previousCoords, e.coordinate] : [e.coordinate];
olFt = this.getNewFeature(newDrawMethod, actualCoords);
olFt.setProperties(omit(previousFt && previousFt.getProperties() || {}, "geometry"));
Expand Down Expand Up @@ -842,7 +843,7 @@ class DrawSupport extends React.Component {
props = assign({}, newProps, {features: [newFeature]});
} else {
if (newFeature && newFeature.properties && newFeature.properties.isCircle) {
props = assign({}, newProps, {features: []}); // TODO verify this
props = assign({}, newProps, {features: []});
} else {
props = assign({}, newProps, {features: newFeature.geometry ? [{...newFeature.geometry, properties: newFeature.properties}] : []});
}
Expand All @@ -866,7 +867,7 @@ class DrawSupport extends React.Component {
this.setState({keySingleClickCallback: this.addSingleClickListener(singleClickCallback)});
}
}
if (newProps.options && newProps.options.selectEnabled/* && (newProps.drawMethod !== "Point" && newProps.drawMethod !== "Text")*/) { // TODO fix all call to this which are missing "selectEnabled" flag
if (newProps.options && newProps.options.selectEnabled) {
this.addSelectInteraction(newProps.options && newProps.options.selected, newProps);

}
Expand Down Expand Up @@ -1174,17 +1175,17 @@ class DrawSupport extends React.Component {
*/
const editFilter = props && props.options && props.options.editFilter;
this.modifyFeatureColl = new ol.Collection(filter(this.drawLayer.getSource().getFeatures(), editFilter));


this.modifyInteraction = new ol.interaction.Modify({
features: this.modifyFeatureColl,
condition: (e) => {
return ol.events.condition.primaryAction(e) && !ol.events.condition.altKeyOnly(e);
}
});


this.modifyInteraction.on('modifyend', (e) => {


let features = e.features.getArray().map((f) => {
// transform back circles in polygons
let newFt = f.clone();
Expand All @@ -1202,9 +1203,7 @@ class DrawSupport extends React.Component {
return reprojectGeoJson(geojsonFormat.writeFeatureObject(newFt), this.getMapCrs(), "EPSG:4326");
});
if (this.props.options.transformToFeatureCollection) {
// this.props.onGeometryChanged([{type: "FeatureCollection", features}], this.props.drawOwner, false, "editing", "editing"); // TODO CHECK IF THIS IS NEEDED
this.props.onDrawingFeatures(features);
// this.addModifyInteraction();
} else {
this.props.onGeometryChanged(features, this.props.drawOwner, false, "editing", "editing"); // TODO FIX THIS
}
Expand Down
Loading