Fix Grid column resize to take account min width for cells (#16597)

Use Escalator cell size calculation without content to determine the absolute minimum size for cells. This is used in Grid when drag resizing or sorting columns to prevent cells from overflowing to the next row. Change-Id: I2d598232d7d2b8729b11fe190b68ca3e42ee3652
vaadin · Dec 16, 2015 · 3514c57 · 3514c57
1 parent bbe7327
commit 3514c57
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 45 deletions.
diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -1960,54 +1960,67 @@ public Cell getCell(final Element element) {
             return new Cell(domRowIndex, domColumnIndex, cellElement);
         }
 
-        double getMaxCellWidth(int colIndex) throws IllegalArgumentException {
+        double measureCellWidth(TableCellElement cell, boolean withContent) {
-            double maxCellWidth = -1;
+            /*
+             * To get the actual width of the contents, we need to get the cell
+             * content without any hardcoded height or width.
+             * 
+             * But we don't want to modify the existing column, because that
+             * might trigger some unnecessary listeners and whatnot. So,
+             * instead, we make a deep clone of that cell, but without any
+             * explicit dimensions, and measure that instead.
+             */
+
+            TableCellElement cellClone = TableCellElement.as((Element) cell
+                    .cloneNode(withContent));
+            cellClone.getStyle().clearHeight();
+            cellClone.getStyle().clearWidth();
+
+            cell.getParentElement().insertBefore(cellClone, cell);
+            double requiredWidth = WidgetUtil
+                    .getRequiredWidthBoundingClientRectDouble(cellClone);
+            if (BrowserInfo.get().isIE()) {
+                /*
+                 * IE browsers have some issues with subpixels. Occasionally
+                 * content is overflown even if not necessary. Increase the
+                 * counted required size by 0.01 just to be on the safe side.
+                 */
+                requiredWidth += 0.01;
+            }
+
+            cellClone.removeFromParent();
 
+            return requiredWidth;
+        }
+
+        /**
+         * Gets the minimum width needed to display the cell properly.
+         * 
+         * @param colIndex
+         *            index of column to measure
+         * @param withContent
+         *            <code>true</code> if content is taken into account,
+         *            <code>false</code> if not
+         * @return cell width needed for displaying correctly
+         */
+        double measureMinCellWidth(int colIndex, boolean withContent) {
             assert isAttached() : "Can't measure max width of cell, since Escalator is not attached to the DOM.";
 
+            double minCellWidth = -1;
             NodeList<TableRowElement> rows = root.getRows();
-            for (int row = 0; row < rows.getLength(); row++) {
-                TableRowElement rowElement = rows.getItem(row);
-                TableCellElement cellOriginal = rowElement.getCells().getItem(
-                        colIndex);
-
-                if (cellOriginal == null || cellIsPartOfSpan(cellOriginal)) {
-                    continue;
-                }
 
-                /*
+            for (int row = 0; row < rows.getLength(); row++) {
-                 * To get the actual width of the contents, we need to get the
-                 * cell content without any hardcoded height or width.
-                 * 
-                 * But we don't want to modify the existing column, because that
-                 * might trigger some unnecessary listeners and whatnot. So,
-                 * instead, we make a deep clone of that cell, but without any
-                 * explicit dimensions, and measure that instead.
-                 */
 
-                TableCellElement cellClone = TableCellElement
+                TableCellElement cell = rows.getItem(row).getCells()
-                        .as((Element) cellOriginal.cloneNode(true));
+                        .getItem(colIndex);
-                cellClone.getStyle().clearHeight();
-                cellClone.getStyle().clearWidth();
 
-                rowElement.insertBefore(cellClone, cellOriginal);
+                if (cell != null && !cellIsPartOfSpan(cell)) {
-                double requiredWidth = WidgetUtil
+                    double cellWidth = measureCellWidth(cell, withContent);
-                        .getRequiredWidthBoundingClientRectDouble(cellClone);
+                    minCellWidth = Math.max(minCellWidth, cellWidth);
-                if (BrowserInfo.get().isIE()) {
-                    /*
-                     * IE browsers have some issues with subpixels. Occasionally
-                     * content is overflown even if not necessary. Increase the
-                     * counted required size by 0.01 just to be on the safe
-                     * side.
-                     */
-                    requiredWidth += 0.01;
                 }
-
-                maxCellWidth = Math.max(requiredWidth, maxCellWidth);
-                cellClone.removeFromParent();
             }
 
-            return maxCellWidth;
+            return minCellWidth;
         }
 
         private boolean cellIsPartOfSpan(TableCellElement cell) {
@@ -4293,16 +4306,28 @@ public double getColumnWidthActual(int index) {
 
         private double getMaxCellWidth(int colIndex)
                 throws IllegalArgumentException {
-            double headerWidth = header.getMaxCellWidth(colIndex);
+            double headerWidth = header.measureMinCellWidth(colIndex, true);
-            double bodyWidth = body.getMaxCellWidth(colIndex);
+            double bodyWidth = body.measureMinCellWidth(colIndex, true);
-            double footerWidth = footer.getMaxCellWidth(colIndex);
+            double footerWidth = footer.measureMinCellWidth(colIndex, true);
 
             double maxWidth = Math.max(headerWidth,
                     Math.max(bodyWidth, footerWidth));
             assert maxWidth >= 0 : "Got a negative max width for a column, which should be impossible.";
             return maxWidth;
         }
 
+        private double getMinCellWidth(int colIndex)
+                throws IllegalArgumentException {
+            double headerWidth = header.measureMinCellWidth(colIndex, false);
+            double bodyWidth = body.measureMinCellWidth(colIndex, false);
+            double footerWidth = footer.measureMinCellWidth(colIndex, false);
+
+            double minWidth = Math.max(headerWidth,
+                    Math.max(bodyWidth, footerWidth));
+            assert minWidth >= 0 : "Got a negative max width for a column, which should be impossible.";
+            return minWidth;
+        }
+
         /**
          * Calculates the width of the columns in a given range.
          * 
@@ -6659,4 +6684,14 @@ private String getSubPartNameSpacer(Element subElement) {
     private void logWarning(String message) {
         getLogger().warning(message);
     }
+
+    /**
+     * This is an internal method for calculating minimum width for Column
+     * resize.
+     * 
+     * @return minimum width for column
+     */
+    double getMinCellWidth(int colIndex) {
+        return columnConfiguration.getMinCellWidth(colIndex);
+    }
 }
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
@@ -5603,17 +5603,22 @@ public void update(Row row, Iterable<FlyweightCell> cellsToUpdate) {
 
                                 private Column<?, T> col = getVisibleColumn(column);
                                 private double initialWidth = 0;
+                                private double minCellWidth;
 
                                 @Override
                                 public void onUpdate(double deltaX,
                                         double deltaY) {
-                                    col.setWidth(initialWidth + deltaX);
+                                    col.setWidth(Math.max(minCellWidth,
+                                            initialWidth + deltaX));
                                 }
 
                                 @Override
                                 public void onStart() {
                                     initialWidth = col.getWidthActual();
 
+                                    minCellWidth = escalator
+                                            .getMinCellWidth(getColumns()
+                                                    .indexOf(col));
                                     for (Column<?, T> c : getColumns()) {
                                         if (selectionColumn == c) {
                                             // Don't modify selection column.
@@ -5657,18 +5662,21 @@ public void onCancel() {
         private void addSortingIndicatorsToHeaderRow(HeaderRow headerRow,
                 FlyweightCell cell) {
 
+            Element cellElement = cell.getElement();
+
+            boolean sortedBefore = cellElement.hasClassName("sort-asc")
+                    || cellElement.hasClassName("sort-desc");
+
             cleanup(cell);
             if (!headerRow.isDefault()) {
                 // Nothing more to do if not in the default row
                 return;
             }
 
-            Column<?, ?> column = getVisibleColumn(cell.getColumn());
+            final Column<?, T> column = getVisibleColumn(cell.getColumn());
             SortOrder sortingOrder = getSortOrder(column);
             boolean sortable = column.isSortable();
 
-            Element cellElement = cell.getElement();
-
             if (sortable) {
                 cellElement.addClassName("sortable");
             }
@@ -5680,8 +5688,14 @@ private void addSortingIndicatorsToHeaderRow(HeaderRow headerRow,
 
             if (SortDirection.ASCENDING == sortingOrder.getDirection()) {
                 cellElement.addClassName("sort-asc");
+                if (!sortedBefore) {
+                    verifyColumnWidth(column);
+                }
             } else {
                 cellElement.addClassName("sort-desc");
+                if (!sortedBefore) {
+                    verifyColumnWidth(column);
+                }
             }
 
             int sortIndex = Grid.this.getSortOrder().indexOf(sortingOrder);
@@ -5693,6 +5707,25 @@ private void addSortingIndicatorsToHeaderRow(HeaderRow headerRow,
             }
         }
 
+        /**
+         * Sort indicator requires a bit more space from the cell than normally.
+         * This method check that the now sorted column has enough width.
+         * 
+         * @param column
+         *            sorted column
+         */
+        private void verifyColumnWidth(Column<?, T> column) {
+            int colIndex = getColumns().indexOf(column);
+            double minWidth = escalator.getMinCellWidth(colIndex);
+            if (column.getWidthActual() < minWidth) {
+                // Fix column size
+                escalator.getColumnConfiguration().setColumnWidth(colIndex,
+                        minWidth);
+
+                fireEvent(new ColumnResizeEvent<T>(column));
+            }
+        }
+
         /**
          * Finds the sort order for this column
          */

diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnResizeTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridColumnResizeTest.java
@@ -121,4 +121,14 @@ private void assertResizable(GridCellElement cell, boolean resizable) {
                 cell.isElementPresent(By
                         .cssSelector("div.v-grid-column-resize-handle")));
     }
+
+    @Test
+    public void testShrinkColumnToZero() {
+        openTestURL();
+        GridCellElement cell = getGridElement().getCell(0, 1);
+        dragResizeColumn(1, 0, cell.getSize().getWidth());
+
+        assertGreaterOrEqual("Cell got too small.", cell.getSize().getWidth(),
+                10);
+    }
 }