Fix #3601 measure updates #3602
Conversation
this will add some changes to the measurement components. there will be two ways of rendering measure panel, a side panel with coord editor and a modal without it
ghost
added
| @@ -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 | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
| replaceFeatures = (features) => { | ||
| this.source = new ol.source.Vector(); | ||
| features.forEach((geoJSON) => { | ||
| let geometry = reprojectGeoJson(geoJSON, "EPSG:4326", this.props.map.getView().getProjection().getCode()).geometry; | ||
| let geoJSONFT = geoJSON; | ||
| if (geoJSONFT.geometry.type === "Polygon" && geoJSONFT.geometry.coordinates[0].length >= 3) { |
There was a problem hiding this comment.
polygons must have 4 coords to be drawn correctly in ol (the first equal to the last)
| isPolygon = (feature) => { | ||
| return feature.geometry.type === "Polygon"; | ||
| } | ||
| updateFeatures = (props) => { |
| this.measureTooltip.setPosition(this.tooltipCoord); | ||
| props.map.addOverlay(this.measureTooltip); | ||
| } | ||
| this.updateMeasurementResults(props, this.source.getFeatures()[0]); |
There was a problem hiding this comment.
Istead of adding an argument to updateMeasurementResults, could you simply do something like)
this.sketchFeature = this.source.getFeatures()[0];
this.updateMeasurementResult(props)
There was a problem hiding this comment.
but I need to know that the update has been made via UI (changing of feature geometry) because then i'll update the state props and then this flag (updatebyUI) it is checked via willReceiveProps in order to avoid endless loop
There was a problem hiding this comment.
Correct me if I'm wrong.
I see the second argument of updateMeasurementResults is used only here and that argument, called olFeatureUpdateByUI, is used, inside the function definition, only to do this:
let olFeatureToUse = this.sketchFeature;
if (!this.sketchFeature) {
return;
}
if (olFeatureUpdateByUI) {
olFeatureToUse = olFeatureUpdateByUI;
this.sketchFeature = olFeatureUpdateByUI;
}This means that olFeatureByUI, that is this.source.getFeatures()[0] is assigned to this.sketchFeature, if present, and used for nothing else. olFeatureToUse is always this.sketchFeature, so the 2 variables can be removed.
The only corner case is when this.source.getFeatures()[0] != null and this.sketchFeature = null. You can solve this corner case outside the function, without complicating the logic or adding non-sense arguments to the function.
|
closed because of conflict |
Description
@example
Issues
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
What is the current behavior? (You can also link to an open issue here)
you can only draw a measure with the map
What is the new behavior?
You can use a coordinate editor in the measure tool. It updates the measures as the coordinates are changing
Does this PR introduce a breaking change? (check one with "x", remove the other)
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: