new DrawCoordinate command added#329
Conversation
|
Hi @fgdrf , |
| this.useCircle = useCircle; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
I'm wondering if I haven't seen or even found a test case for this. Any chance to provide usefull tests?
There was a problem hiding this comment.
Yes there is no test case as is the case with all DrawCommands. If you think it absolutely necessary, maybe I can provide a test in the form of a test menu item that draws a circle at a coordinate on a map. But it could be tested manually and not be automated. What do you think?
| (int) Math.abs(points[2] - points[0]), | ||
| (int) Math.abs(points[3] - points[1])); | ||
| } catch (TransformException e) { | ||
| if (!errorReported) { |
There was a problem hiding this comment.
does it mean to ignore the first error and write any other to log after that (because errorReported is set to true)? Wondering about the semantic of the variable. I guess it would help me to understand if you could describe the flow?
There was a problem hiding this comment.
Actually as mentioned and in a previous comment this draw command was based on the DrawFeatureCommand. So I retain the logic adopted there without checking it further.
|
|
||
| public void setValid(boolean valid) { | ||
| super.setValid(valid); | ||
| if (!valid) |
There was a problem hiding this comment.
Is it necessary to overwride setValid() here? My read of ViewportPainter is that commands get disposed from within clearCommands. I guess you can remove method setValid from class because no additional behavior is required here to implement (for example removing listeners).
There was a problem hiding this comment.
Indeed it is not. I will remove it in a following commit
| /** | ||
| * @return Returns the paint. | ||
| */ | ||
| public Paint getPaint() { |
There was a problem hiding this comment.
Is this the paintColor for the Coordinate? I guess because internally its a Color object it should return Color here as well. I generall I prefer members to name what it is for, ig. paintColor with named getters and setters (like you almost did for the lineStyle ;))
There was a problem hiding this comment.
OK I will fix it in the next commit.
| * @param lineStyle the style of the line | ||
| * @param lineWidth the width of the line | ||
| */ | ||
| public void setStroke(int lineStyle, int lineWidth) { |
There was a problem hiding this comment.
Confusion for me: Here you set two different properties at once. Why not having two different settes for lineStyle (Would an Enum help here btw?) and lineWidth?
Ohh, I guess lineStyle is one of ViewportGraphics.LINE_XXX stroke styles and your implementation is based on DrawShapeCommand or something familiar, don't you?
Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
fb6bb51 to
3cf777b
Compare
Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
|
Hi @fgdrf , I added a test handler which can be used for testing DrawCoordinateCommand via the UI. The handler centers viewport in the input coordinate and draws a flashing dot for a couple os seconds (see screenshot below) |
|
Hi @fgdrf , what about this PR? It's been quite some time since the test handler was added. Shouldn't we merge and close it? |
* Adds a new DrawCoordinateCommand to UDIG's list of draw commands Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com> * change default shape used for coordination display Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com> * correct useCircle field getter/setter to be public Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com> * remove setValid(...) from class and fix return type of getPaint Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com> * added test handler for testing DrawCoordinate command Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
a new DrawCoordinateCommand to UDIG's list of draw commands