Fixes column header drop indicator (#18925)

Calculation of the frozen column width was wrong in multiselect mode.
It did not take into account the last frozen column, but added
the select column twice.
The same problem was in AutoScroller (as the same methods were
copy-pasted there), for which reason the autoscrolling was not
scrolling till the end.
This patch fixes the calculation error and removes copy-pasted code,
reusing the same code in both places.

Change-Id: I164e2fc96688088b620ad8785c533c593723f83e

diff --git a/client/src/com/vaadin/client/widget/grid/AutoScroller.java b/client/src/com/vaadin/client/widget/grid/AutoScroller.java
index 90d206d98b2aa396fd08365680ad1c09d47bda3b..9cc238ac15485043ba8455b4252710cf0d4c8e87 100644 (file)
--- a/client/src/com/vaadin/client/widget/grid/AutoScroller.java
+++ b/client/src/com/vaadin/client/widget/grid/AutoScroller.java
@@ -610,22 +610,25 @@ public class AutoScroller {
         }
     }
 
-    private double getFrozenColumnsWidth() {
-        double value = getMultiSelectColumnWidth();
-        for (int i = 0; i < grid.getFrozenColumnCount(); i++) {
+    public double getFrozenColumnsWidth() {
+        double value = 0;
+
+        for (int i = 0; i < getRealFrozenColumnCount(); i++) {
             value += grid.getColumn(i).getWidthActual();
         }
+
         return value;
     }
 
-    private double getMultiSelectColumnWidth() {
-        if (grid.getFrozenColumnCount() >= 0
-                && grid.getSelectionModel().getSelectionColumnRenderer() != null) {
-            // frozen checkbox column is present
-            return getTheadElement().getFirstChildElement()
-                    .getFirstChildElement().getOffsetWidth();
+    private int getRealFrozenColumnCount() {
+        if (grid.getFrozenColumnCount() < 0) {
+            return 0;
+        } else if (grid.getSelectionModel().getSelectionColumnRenderer() != null) {
+            // includes the selection column
+            return grid.getFrozenColumnCount() + 1;
+        } else {
+            return grid.getFrozenColumnCount();
         }
-        return 0.0;
     }
 
     private double getMaxScrollLeft() {

diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index ccc909465f532afc3aa60956af56c9a3ad59e0eb..a77499e41f1ad9bcf72c0763488078b476cf999e 100644 (file)
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -4113,7 +4113,8 @@ public class Grid<T> extends ResizeComposite implements
 
             dropMarkerLeft += autoScrollX;
 
-            final double frozenColumnsWidth = getFrozenColumnsWidth();
+            final double frozenColumnsWidth = autoScroller
+                    .getFrozenColumnsWidth();
             final double rightBoundaryForDrag = getSidebarBoundaryComparedTo(dropMarkerLeft);
             final int visibleColumns = getVisibleColumns().size();
 
@@ -4156,7 +4157,8 @@ public class Grid<T> extends ResizeComposite implements
                     .min(sidebarBoundary, gridBoundary);
 
             // Do not show on left of the frozen columns (even if scrolled)
-            final int frozenColumnsWidth = (int) getFrozenColumnsWidth();
+            final int frozenColumnsWidth = (int) autoScroller
+                    .getFrozenColumnsWidth();
 
             left = Math.max(frozenColumnsWidth, Math.min(left, rightBoundary));
 
@@ -4317,25 +4319,6 @@ public class Grid<T> extends ResizeComposite implements
             autoScroller.stop();
         }
 
-        private double getFrozenColumnsWidth() {
-            double value = getMultiSelectColumnWidth();
-            for (int i = 0; i < getFrozenColumnCount(); i++) {
-                value += getColumn(i).getWidthActual();
-            }
-            return value;
-        }
-
-        private double getMultiSelectColumnWidth() {
-            if (getSelectionModel().getSelectionColumnRenderer() != null) {
-                // frozen checkbox column is present, it is always the first
-                // column
-                return escalator.getHeader().getElement()
-                        .getFirstChildElement().getFirstChildElement()
-                        .getOffsetWidth();
-            }
-            return 0.0;
-        }
-
         /**
          * Returns the amount of frozen columns. The selection column is always
          * considered frozen, since it can't be moved.
@@ -4436,7 +4419,7 @@ public class Grid<T> extends ResizeComposite implements
                 return;
             }
 
-            double position = getFrozenColumnsWidth();
+            double position = autoScroller.getFrozenColumnsWidth();
             // iterate column indices and add possible drop positions
             for (int i = frozenColumns; i < getColumnCount(); i++) {
                 Column<?, T> column = getColumn(i);
@@ -8714,4 +8697,4 @@ public class Grid<T> extends ResizeComposite implements
         }
         return null;
     }
-}
+}
\ No newline at end of file

diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDragAndDrop.java b/uitest/src/com/vaadin/tests/components/grid/GridDragAndDrop.java
new file mode 100644 (file)
index 0000000..d77a8dd
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/grid/GridDragAndDrop.java
@@ -0,0 +1,62 @@
+/*
+ * Copyright 2000-2014 Vaadin Ltd.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.grid;
+
+import java.util.Arrays;
+import java.util.List;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Grid;
+
+@SuppressWarnings("serial")
+public class GridDragAndDrop extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+
+        List<String> columnIds = Arrays.asList("Hello", "this", "are",
+                "multiple", "columns", "plus", "these", "resemble", "a",
+                "group", "here", "no", "more");
+
+        Grid grid = new Grid();
+
+        for (String columnId : columnIds) {
+            grid.addColumn(columnId);
+        }
+
+        for (int i = 0; i < 100; i++) {
+            grid.addRow(columnIds.toArray());
+        }
+
+        grid.setColumnReorderingAllowed(true);
+
+        grid.setFrozenColumnCount(1);
+        grid.setSelectionMode(Grid.SelectionMode.MULTI);
+
+        addComponent(grid);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "Start dragging a column header and move left and right.<br> The drop indicator should appear exactly on the lines between column headers.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 18925;
+    }
+}