Fix #2668 Increase precision of Query Panel Circle/BBox Details#2720
Fix #2668 Increase precision of Query Panel Circle/BBox Details#2720offtherailz merged 5 commits intogeosolutions-it:masterfrom
Conversation
| }; | ||
|
|
||
| roundValue = (val, prec = 1000000) => Math.round(val * prec) / prec; | ||
| getStep = (zoom = 1) => { |
There was a problem hiding this comment.
I think we can translate this into a formula, in any case we shouldn't use ifs
| const step = this.getStep(zoom); | ||
| return name === 'radius' && step * 100000 || step; | ||
| }; | ||
| isWGS84 = () => (this.props.geometry || {}).projection === 'EPSG:4326' || !this.props.useMapProjection; |
There was a problem hiding this comment.
I think we can use proj4 to know the uom of the projection, and generalize EPSG:4326 to uom === 'degrees'
| }; | ||
|
|
||
| this.props.onChangeDrawingStatus("replace", undefined, "queryform", [geometry]); | ||
| this.props.onChangeDrawingStatus({geometry: [geometry]}); |
There was a problem hiding this comment.
is CHANGE_DRAWING_STATUS without a "command" (e.g. replace) supported?
There was a problem hiding this comment.
maybe was better to change the name, but isn't directly calling the onChangeDrawingStatus action. That action is debounced and then called by enhancer
There was a problem hiding this comment.
Actions have a standard interface, you are changing it here
| const propsStreamFactory = require('../../../misc/enhancers/propsStreamFactory'); | ||
|
|
||
|
|
||
| const dataStreamFactory = $props => { |
There was a problem hiding this comment.
Done to debounce the change.
There was a problem hiding this comment.
it also changes the interface to call onChangeDrawingStatus, which is not immediately clear, look at the other question a bove
There was a problem hiding this comment.
I mean, if you put a "transparent" debounce in the middle, it should be transparent, if I remove it the same code will work, without debouncing. We want to enhance, not to change.
| }; | ||
|
|
||
| getStep = (zoom = 1) => { | ||
| if ( zoom >= 21 ) { |
There was a problem hiding this comment.
Can we translate this into a formula?
| const {compose, withHandlers} = require('recompose'); | ||
| const {debounce} = require("lodash"); | ||
|
|
||
| module.exports = compose(withHandlers((initProp) => { |
There was a problem hiding this comment.
Can't we generalize this into a "debouncing" enhancer? I don't get the purpose of having single use enhancers in separate files
offtherailz
left a comment
There was a problem hiding this comment.
Take a look on my PR for required changes.
I removed debouncing because it caused more issues tha benefits. Please add a test for your changes
| const {debounce} = require("lodash"); | ||
| const emptyFunc = () => {}; | ||
| /** | ||
| * This enhancer debounce an action or of a passed debounceTime |
There was a problem hiding this comment.
This enhancer de-bounce a method passed as prop of the given time.
Description
If Query Panel Geometry details has proj 4326 the precision off numeric fields is increased to 6th decimal, the step is related to the current scale, and the update is debounced by 1 sec.
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)
All numeric fields has tow decimal precision, and the step is always one
What is the new behaviour?
If proj is 4326, coordinates fields has 6 decimal precision, the step is related to the scale and the update is debounced by one sec. The radius has 2 decimal precision.
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: