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

2 files changed, 113 insertions, 45 deletions

diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java
index 3705908dcc..3585be1d60 100644
--- a/client/src/com/vaadin/client/widgets/Escalator.java
+++ b/client/src/com/vaadin/client/widgets/Escalator.java
@@ -1960,54 +1960,67 @@ public class Escalator extends Widget implements RequiresResize,
             return new Cell(domRowIndex, domColumnIndex, cellElement);
         }
 
-        double getMaxCellWidth(int colIndex) throws IllegalArgumentException {
-            double maxCellWidth = -1;
+        double measureCellWidth(TableCellElement cell, boolean withContent) {
+            /*
+             * 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;
-                }
 
-                /*
-                 * 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.
-                 */
+            for (int row = 0; row < rows.getLength(); row++) {
 
-                TableCellElement cellClone = TableCellElement
-                        .as((Element) cellOriginal.cloneNode(true));
-                cellClone.getStyle().clearHeight();
-                cellClone.getStyle().clearWidth();
+                TableCellElement cell = rows.getItem(row).getCells()
+                        .getItem(colIndex);
 
-                rowElement.insertBefore(cellClone, cellOriginal);
-                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;
+                if (cell != null && !cellIsPartOfSpan(cell)) {
+                    double cellWidth = measureCellWidth(cell, withContent);
+                    minCellWidth = Math.max(minCellWidth, cellWidth);
                 }
-
-                maxCellWidth = Math.max(requiredWidth, maxCellWidth);
-                cellClone.removeFromParent();
             }
 
-            return maxCellWidth;
+            return minCellWidth;
         }
 
         private boolean cellIsPartOfSpan(TableCellElement cell) {
@@ -4293,9 +4306,9 @@ public class Escalator extends Widget implements RequiresResize,
 
         private double getMaxCellWidth(int colIndex)
                 throws IllegalArgumentException {
-            double headerWidth = header.getMaxCellWidth(colIndex);
-            double bodyWidth = body.getMaxCellWidth(colIndex);
-            double footerWidth = footer.getMaxCellWidth(colIndex);
+            double headerWidth = header.measureMinCellWidth(colIndex, true);
+            double bodyWidth = body.measureMinCellWidth(colIndex, true);
+            double footerWidth = footer.measureMinCellWidth(colIndex, true);
 
             double maxWidth = Math.max(headerWidth,
                     Math.max(bodyWidth, footerWidth));
@@ -4303,6 +4316,18 @@ public class Escalator extends Widget implements RequiresResize,
             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 @@ public class Escalator extends Widget implements RequiresResize,
     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
index 41c8f329eb..2207abe408 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -5603,17 +5603,22 @@ public class Grid<T> extends ResizeComposite implements
 
                                 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 class Grid<T> extends ResizeComposite implements
         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 @@ public class Grid<T> extends ResizeComposite implements
 
             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);
@@ -5694,6 +5708,25 @@ public class Grid<T> extends ResizeComposite implements
         }
 
         /**
+         * 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
          */
         private SortOrder getSortOrder(Column<?, ?> column) {