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

Initial commit JavaFX map module #1638

Merged
merged 3 commits into from Jul 11, 2017
Merged

Conversation

awoestmann
Copy link
Contributor

This commit creates the JavaFX map module.

Copy link
Contributor

@bradh bradh left a comment

Choose a reason for hiding this comment

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

Just some initial review

@@ -0,0 +1,30 @@
# MODULE EXAMPLE
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these needs actual values put into it.

The Geotools Project
http://www.geotools.org/

Version: $Id$
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest removing this. CVS ain't coming back.

<!-- -->
<!-- Available profiles are: -->
<!-- -->
<!-- nameOfprofile Explantion of what the profile does -->
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably needs actual values plugged in.

* GeoTools - The Open Source Java GIS Toolkit
* http://geotools.org
*
* (C) 2007-2008, Open Source Geospatial Foundation (OSGeo)
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like the right year.

@@ -215,6 +221,7 @@
<module>css</module>
<module>geojsonstore</module>
<module>polylabel</module>
<module>javafx</module>
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting looks off

Copy link
Member

Choose a reason for hiding this comment

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

Tabs vs spaces :)

double toY = to.getY();

double xOffset = (toX - fromX);
double yOffset = (toY - fromY);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure these calcs are needed.

}

/**
* Draws a marker on a speficic position.
Copy link
Contributor

Choose a reason for hiding this comment

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

specific

* @param yPosition Marker y coordinate in screen coordinates
*/
public void drawMarker(double xPosition, double yPosition) {
double markerSpan = this.mapCanvas.getWidth() / HUNDRED;
Copy link
Contributor

Choose a reason for hiding this comment

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

May not work well if canvas is not square.

&& e.getY() > (mouseYPosOnClick - DRAGGING_OFFSET)) {
drawMarker(mouseXPosOnClick, mouseYPosOnClick);
markerCount++;
if (markerCount == 2) {
Copy link
Contributor

Choose a reason for hiding this comment

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

formatting looks off.

zoomLevel--;
}
scaleMap(zoomLevel - lastZoomLevel);
try{
Copy link
Contributor

Choose a reason for hiding this comment

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

space before the {

@awoestmann
Copy link
Contributor Author

Thanks for your feedback. I should have fixed most of the things you mentioned with 75885e7 .

@jodygarnett
Copy link
Member

Thanks @awoestmann - I am kicking the travis build to confirm it still builds.

@awoestmann
Copy link
Contributor Author

I removed the import that caused build to fail, as it was unused anyway.

@jodygarnett jodygarnett merged commit ad47e66 into geotools:master Jul 11, 2017
jodygarnett pushed a commit that referenced this pull request Jul 11, 2017
@jodygarnett
Copy link
Member

Thanks, can you fix up the tabs to spaces when you get a chance!

@bencaradocdavies
Copy link
Member

This merge breaks the build on OpenJDK (unless OpenJFX is installed). I have remove gt-javafx from the -Dall build:
https://osgeo-org.atlassian.net/browse/GEOT-5779

nprigour pushed a commit to nprigour/geotools that referenced this pull request Nov 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants