changeset 4065:5b75776bb378

RT-31149 NPE showing NumberAxis without setting Side, RT-31018 LineChart plots obsolete series data Some unit tests to increase code coverage.
author psomashe
date Tue, 25 Jun 2013 16:47:19 -0700
parents e0434ac66df4
children 1c00e6382807
files javafx-ui-charts/src/javafx/scene/chart/XYChart.java javafx-ui-charts/test/javafx/scene/chart/ChartTestBase.java javafx-ui-charts/test/javafx/scene/chart/LineChartTest.java javafx-ui-charts/test/javafx/scene/chart/PieChartTest.java javafx-ui-charts/test/javafx/scene/chart/XYChartTest.java javafx-ui-controls/src/javafx/scene/chart/Axis.java javafx-ui-controls/src/javafx/scene/chart/ValueAxis.java
diffstat 7 files changed, 145 insertions(+), 56 deletions(-) [+]
line wrap: on
line diff
--- a/javafx-ui-charts/src/javafx/scene/chart/XYChart.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-charts/src/javafx/scene/chart/XYChart.java	Tue Jun 25 16:47:19 2013 -0700
@@ -1258,7 +1258,9 @@
         public final void setXValue(X value) {
             xValue.set(value);
             // handle the case where this is a init because the default constructor was used
-            if (currentX.get() == null) currentX.setValue(value);
+            // and the case when series is not associated to a chart due to a remove series
+            if (currentX.get() == null ||
+                    (series != null && series.getChart() == null)) currentX.setValue(value);
         }
         /** 
          * The generic data value to be plotted on the X axis.
@@ -1303,7 +1305,10 @@
         public final void setYValue(Y value) {
             yValue.set(value);
             // handle the case where this is a init because the default constructor was used
-            if (currentY.get() == null) currentY.setValue(value);
+            // and the case when series is not associated to a chart due to a remove series
+            if (currentY.get() == null || 
+                    (series != null && series.getChart() == null)) currentY.setValue(value);
+            
         }
         /** 
          * The generic data value to be plotted on the Y axis.
--- a/javafx-ui-charts/test/javafx/scene/chart/ChartTestBase.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-charts/test/javafx/scene/chart/ChartTestBase.java	Tue Jun 25 16:47:19 2013 -0700
@@ -74,6 +74,22 @@
         toolkit.fireTestPulse();
     }
     
+    protected Scene getTestScene() {
+        return this.scene;
+    }
+    
+    protected void setTestScene(Scene scene) {
+        this.scene = scene;
+    }
+    
+    protected Stage getTestStage() {
+        return this.stage;
+    }
+    
+    protected void setTestStage(Stage stage) {
+        this.stage = stage;
+    }
+    
     protected abstract Chart createChart();
     
     StringBuffer computeSVGPath(Path line) {
--- a/javafx-ui-charts/test/javafx/scene/chart/LineChartTest.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-charts/test/javafx/scene/chart/LineChartTest.java	Tue Jun 25 16:47:19 2013 -0700
@@ -61,7 +61,6 @@
         yAxis.setLabel("Y Axis");
         lineChart.setTitle("HelloLineChart");
         // add starting data
-        ObservableList<XYChart.Data> data = FXCollections.observableArrayList();
         series1.getData().add(new XYChart.Data(10d, 10d));
         series1.getData().add(new XYChart.Data(25d, 20d));
         series1.getData().add(new XYChart.Data(30d, 15d));
@@ -113,4 +112,53 @@
          lineChart.getData().addAll(series1);
          assertEquals(5, countSymbols(lineChart, "chart-line-symbol"));
      }
+     
+    @Test
+    public void testDataItemAdd() {
+        startApp();
+        lineChart.getData().addAll(series1);
+        pulse();
+        series1.getData().add(new XYChart.Data(60d, 30d));
+        pulse();
+        // 5 stackpane nodes and 1 path node + new stackpane for data added
+        assertEquals(7, lineChart.getPlotChildren().size());
+    }
+    
+     @Test
+    public void testDataItemAddWithAnimation() {
+        startApp();
+        lineChart.setAnimated(true);
+        lineChart.getData().addAll(series1);
+        pulse();
+        series1.getData().add(new XYChart.Data(60d, 30d));
+        pulse();
+        // 5 stackpane nodes and 1 path node + new stackpane for data added
+        assertEquals(7, lineChart.getPlotChildren().size());
+    }
+      
+    @Test
+    public void testDataItemRemove() {
+        startApp();
+        lineChart.getData().addAll(series1);
+        pulse();
+        if (!lineChart.getData().isEmpty()) {
+            series1.getData().remove(0);
+            pulse();
+            // 4 stackpane nodes and one path node
+            assertEquals(5, lineChart.getPlotChildren().size());
+        }
+    }
+     
+     @Test
+    public void testSeriesAddWithAnimation() {
+        startApp();
+        lineChart.setAnimated(true);
+        final XYChart.Series<Number, Number> series2 = new XYChart.Series<Number, Number>();
+        series1.getData().add(new XYChart.Data(15d, 40d));
+        series1.getData().add(new XYChart.Data(25d, 10d));
+        series1.getData().add(new XYChart.Data(40d, 35d));
+        lineChart.getData().addAll(series1);
+        pulse();
+        assertEquals(true, lineChart.getAnimated());
+    }
 }
--- a/javafx-ui-charts/test/javafx/scene/chart/PieChartTest.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-charts/test/javafx/scene/chart/PieChartTest.java	Tue Jun 25 16:47:19 2013 -0700
@@ -86,4 +86,18 @@
         }
     }
     
+    @Test
+    public void testDataItemRemovedWithAnimation() {
+        startApp();
+        data.add(new PieChart.Data("Sun", 20));
+        data.add(new PieChart.Data("IBM", 12));
+        data.add(new PieChart.Data("HP", 25));
+        data.add(new PieChart.Data("Dell", 22));
+        data.add(new PieChart.Data("Apple", 30));
+        pc.setAnimated(true);
+        pc.getData().addAll(data);
+        pc.getData().remove(0);
+        assertEquals(4, pc.getData().size());
+    }
+    
 }
--- a/javafx-ui-charts/test/javafx/scene/chart/XYChartTest.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-charts/test/javafx/scene/chart/XYChartTest.java	Tue Jun 25 16:47:19 2013 -0700
@@ -34,10 +34,13 @@
 import javafx.css.StyleableProperty;
 import com.sun.javafx.css.parser.CSSParser;
 import javafx.scene.Node;
+import javafx.scene.Scene;
+import javafx.scene.layout.StackPane;
 import javafx.scene.paint.Color;
 
 import javafx.scene.text.Font;
 import javafx.scene.text.Text;
+import javafx.stage.Stage;
 import org.junit.Assert;
 import static org.junit.Assert.assertEquals;
 
@@ -109,4 +112,19 @@
             }
         }
     }
+    
+    @Test public void testAddAxisWithoutSpecifyingSide() {
+        final NumberAxis axis = new NumberAxis(0, 12, 1);
+        axis.setMaxWidth(Double.MAX_VALUE);
+        axis.setPrefWidth(400);
+        pulse();
+        StackPane layout = new StackPane();
+        layout.getChildren().addAll(axis);
+        pulse();
+        setTestScene(new Scene(layout));
+        setTestStage(new Stage());
+        getTestStage().setScene(getTestScene());
+        getTestStage().show();
+        pulse();
+    }
 }
--- a/javafx-ui-controls/src/javafx/scene/chart/Axis.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/chart/Axis.java	Tue Jun 25 16:47:19 2013 -0700
@@ -541,9 +541,11 @@
      */
     @Override protected double computePrefHeight(double width) {
         final Side side = getSide();
-        if (side == null) {
-            return 50;
-        } else if (side.equals(Side.TOP) || side.equals(Side.BOTTOM)) { // HORIZONTAL
+        if (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) { // VERTICAL
+            // TODO for now we have no hard and fast answer here, I guess it should work
+            // TODO out the minimum size needed to display min, max and zero tick mark labels.
+            return 100;
+        } else { // HORIZONTAL
             // we need to first auto range as this may/will effect tick marks
             Object range = autoRange(width);
             // calculate max tick label height
@@ -562,11 +564,7 @@
                     axisLabel.getText() == null || axisLabel.getText().length() == 0 ?
                     0 : axisLabel.prefHeight(-1);
             return maxLabelHeight + getTickLabelGap() + tickMarkLength + labelHeight;
-        } else { // VERTICAL
-            // TODO for now we have no hard and fast answer here, I guess it should work
-            // TODO out the minimum size needed to display min, max and zero tick mark labels.
-            return 100;
-        }
+        } 
     }
 
     /**
@@ -578,13 +576,7 @@
      */
     @Override protected double computePrefWidth(double height) {
         final Side side = getSide();
-        if (side == null) {
-            return 50;
-        } else if (side.equals(Side.TOP) || side.equals(Side.BOTTOM)) { // HORIZONTAL
-            // TODO for now we have no hard and fast answer here, I guess it should work
-            // TODO out the minimum size needed to display min, max and zero tick mark labels.
-            return 100;
-        } else { // VERTICAL
+        if (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) { // VERTICAL
             // we need to first auto range as this may/will effect tick marks
             Object range = autoRange(height);
             // calculate max tick label width
@@ -603,7 +595,11 @@
                     axisLabel.getText() == null || axisLabel.getText().length() == 0 ?
                     0 : axisLabel.prefHeight(-1);
             return maxLabelWidth + getTickLabelGap() + tickMarkLength + labelHeight;
-        }
+        } else  { // HORIZONTAL
+            // TODO for now we have no hard and fast answer here, I guess it should work
+            // TODO out the minimum size needed to display min, max and zero tick mark labels.
+            return 100;
+        } 
     }
 
     /**
@@ -622,7 +618,7 @@
         final boolean isFirstPass = oldLength == 0;
         // auto range if it is not valid
         final Side side = getSide();
-        final double length = (Side.TOP.equals(side) || Side.BOTTOM.equals(side)) ? width : height;
+        final double length = (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) ? height : width;
         int numLabelsToSkip = 1;
         int tickIndex = 0;
         if (oldLength != length || !isRangeValid() || tickPropertyChanged || formatterValid) {
@@ -641,18 +637,15 @@
 
              // calculate maxLabelWidth / maxLabelHeight for respective orientations
             maxWidth = 0; maxHeight = 0;
-            if (side != null) {
-                if (Side.TOP.equals(side) || Side.BOTTOM.equals(side)) {
-                    for (T value: newTickValues) {
-                        maxWidth = Math.round(Math.max(maxWidth, measureTickMarkSize(value, range).getWidth()));
-                    }
-                } else {
-                    for (T value: newTickValues) {
-                        maxHeight = Math.round(Math.max(maxHeight, measureTickMarkSize(value, range).getHeight()));
-                    }
+            if (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) {
+                for (T value: newTickValues) {
+                    maxHeight = Math.round(Math.max(maxHeight, measureTickMarkSize(value, range).getHeight()));
+                }
+            } else {
+                for (T value: newTickValues) {
+                    maxWidth = Math.round(Math.max(maxWidth, measureTickMarkSize(value, range).getWidth()));
                 }
             }
-           
             // we have to work out what new or removed tick marks there are, then create new tick marks and their
             // text nodes where needed
             // find everything added or removed
@@ -736,21 +729,18 @@
 
         // RT-12272 : tick labels overlapping
         int numLabels = 0;
-        if (side != null) {
-            if (Side.TOP.equals(side) || Side.BOTTOM.equals(side)) {
-                numLabels = (maxWidth > 0) ? (int)(length/maxWidth) : 0;
-            } else {
-                numLabels = (maxHeight > 0) ? (int) (length/maxHeight) : 0;
-            }
+        if (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) {
+            numLabels = (maxHeight > 0) ? (int) (length/maxHeight) : 0;
+        } else {
+            numLabels = (maxWidth > 0) ? (int)(length/maxWidth) : 0;
         }
-       
         if (numLabels > 0) {
             numLabelsToSkip = ((int)(tickMarks.size()/numLabels)) + 1;
         }
         // clear tick mark path elements as we will recreate
         tickMarkPath.getElements().clear();
         // do layout of axis label, tick mark lines and text
-        if (getSide().equals(Side.LEFT)) {
+        if (Side.LEFT.equals(side)) {
             // offset path to make strokes snap to pixel
             tickMarkPath.setLayoutX(-0.5);
             tickMarkPath.setLayoutY(0.5);
@@ -782,7 +772,7 @@
                     tick.textNode.setVisible(false);
                 }
             }
-        } else if (getSide().equals(Side.RIGHT)) {
+        } else if (Side.RIGHT.equals(side)) {
             // offset path to make strokes snap to pixel
             tickMarkPath.setLayoutX(0.5);
             tickMarkPath.setLayoutY(0.5);
@@ -814,7 +804,7 @@
                 //noinspection SuspiciousNameCombination
                 axisLabel.resize(height, axisLabelWidth);
             }
-        } else if (getSide().equals(Side.TOP)) {
+        } else if (Side.TOP.equals(side)) {
             // offset path to make strokes snap to pixel
             tickMarkPath.setLayoutX(0.5);
             tickMarkPath.setLayoutY(-0.5);
@@ -896,13 +886,13 @@
         node.setLayoutY(0);
         node.setRotate(angle);
         final Bounds bounds = node.getBoundsInParent();
-        if (side.equals(Side.LEFT)) {
+        if (Side.LEFT.equals(side)) {
             node.setLayoutX(posX-bounds.getWidth()-bounds.getMinX());
             node.setLayoutY(posY - (bounds.getHeight() / 2d) - bounds.getMinY());
-        } else if (side.equals(Side.RIGHT)) {
+        } else if (Side.RIGHT.equals(side)) {
             node.setLayoutX(posX-bounds.getMinX());
             node.setLayoutY(posY-(bounds.getHeight()/2d)-bounds.getMinY());
-        } else if (side.equals(Side.TOP)) {
+        } else if (Side.TOP.equals(side)) {
             node.setLayoutX(posX-(bounds.getWidth()/2d)-bounds.getMinX());
             node.setLayoutY(posY-bounds.getHeight()-bounds.getMinY());
         } else {
--- a/javafx-ui-controls/src/javafx/scene/chart/ValueAxis.java	Wed Jun 26 07:34:46 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/chart/ValueAxis.java	Tue Jun 25 16:47:19 2013 -0700
@@ -298,14 +298,12 @@
     protected final double calculateNewScale(double length, double lowerBound, double upperBound) {
         double newScale = 1;
         final Side side = getSide();
-        if(side != null) {
-            if (side.equals(Side.TOP) || side.equals(Side.BOTTOM)) { // HORIZONTAL
-                offset = 0;
-                newScale = ((upperBound-lowerBound) == 0) ? length : length / (upperBound - lowerBound);
-            } else { // VERTICAL
-                offset = length;
-                newScale = ((upperBound-lowerBound) == 0) ? -length : -(length / (upperBound - lowerBound));
-            }
+        if (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) { // VERTICAL 
+            offset = length;
+            newScale = ((upperBound-lowerBound) == 0) ? -length : -(length / (upperBound - lowerBound));
+        } else { // HORIZONTAL
+            offset = 0;
+            newScale = ((upperBound-lowerBound) == 0) ? length : length / (upperBound - lowerBound);
         }
         return newScale;
     }
@@ -349,7 +347,7 @@
      */
     @Override protected void layoutChildren() {
         final Side side = getSide();
-        final double length = (Side.TOP.equals(side) || Side.BOTTOM.equals(side)) ? getWidth() : getHeight();
+        final double length = (Side.LEFT.equals(side) || Side.RIGHT.equals(side)) ? getHeight() :getWidth() ;
         // if we are not auto ranging we need to calculate the new scale
         if(!isAutoRanging()) {
             // calculate new scale
@@ -376,7 +374,7 @@
         // Update minor tickmarks
         minorTickPath.getElements().clear();
         if (getMinorTickLength() > 0) {
-            if (side.equals(Side.LEFT)) {
+            if (Side.LEFT.equals(side)) {
                 // snap minorTickPath to pixels
                 minorTickPath.setLayoutX(-0.5);
                 minorTickPath.setLayoutY(0.5);
@@ -388,7 +386,7 @@
                                 new LineTo(getWidth()-1, y));
                     }
                 }
-            } else if (side.equals(Side.RIGHT)) {
+            } else if (Side.RIGHT.equals(side)) {
                 // snap minorTickPath to pixels
                 minorTickPath.setLayoutX(0.5);
                 minorTickPath.setLayoutY(0.5);
@@ -400,7 +398,7 @@
                                 new LineTo(getMinorTickLength(), y));
                     }
                 }
-            } else if (side.equals(Side.TOP)) {
+            } else if (Side.TOP.equals(side)) {
                 // snap minorTickPath to pixels
                 minorTickPath.setLayoutX(0.5);
                 minorTickPath.setLayoutY(-0.5);
@@ -412,7 +410,7 @@
                                 new LineTo(x, getHeight() - getMinorTickLength()));
                     }
                 }
-            } else if (side.equals(Side.BOTTOM)) {
+            } else { // BOTTOM
                 // snap minorTickPath to pixels
                 minorTickPath.setLayoutX(0.5);
                 minorTickPath.setLayoutY(0.5);