Skip to content

Fix #3243 circle radius problem for openlayers#3246

Merged
offtherailz merged 5 commits intogeosolutions-it:masterfrom
MV88:3243_circle_radius_ol
Oct 18, 2018
Merged

Fix #3243 circle radius problem for openlayers#3246
offtherailz merged 5 commits intogeosolutions-it:masterfrom
MV88:3243_circle_radius_ol

Conversation

@MV88
Copy link
Copy Markdown
Contributor

@MV88 MV88 commented Oct 17, 2018

Description

see title

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
now the radius keeps the value inserted in the geometry details form.

What is the new behavior?
see #3243

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@coveralls
Copy link
Copy Markdown

coveralls commented Oct 17, 2018

Coverage Status

Coverage increased (+0.006%) to 81.146% when pulling 09e1631 on MV88:3243_circle_radius_ol into b8ea569 on geosolutions-it:master.

* TODO extend it to the other tests
*/
const renderDrawSupport = (props = {}) => {
ReactDOM.render(<DrawSupport {...props}/>, document.getElementById("container"));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You don't reduce the boilerplate if you use it only in one place / test.
It's useful to refactor code in external functions if they are used at least in a bunch of different places, expecially in tests, where being able to read the full test at once is more important than duplication.
If you have too much code in one test it's better to split it in two different tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I created this function Because i wanted to write less code in particular the parts
ReactDOM.render(<COMPONENT {props}/>, document.getElementById("container"));
I could generalize it so we can use it in general and also in other places, but I see that you prefer the older version.
Going to refactor this.

Copy link
Copy Markdown
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

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

Please document that endDrawing method (and status entry) to suggest it is called from external tools to trigger programmatically the endDrawing handler callback.

@offtherailz offtherailz merged commit dfc751f into geosolutions-it:master Oct 18, 2018
MV88 added a commit to MV88/MapStore2 that referenced this pull request Oct 19, 2018
MV88 added a commit to MV88/MapStore2 that referenced this pull request Oct 19, 2018
MV88 added a commit that referenced this pull request Oct 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Circle radius calculated after confirm in geometry details is wrong (OL only)

4 participants