Fixed setting column width back to undefined by setting it to -1. #7922

svn changeset:25591/svn branch:6.8

Conflicts:
	server/src/com/vaadin/ui/Table.java

Reverted change included by mistake in changeset [25591] for #7922

svn changeset:25606/svn branch:6.8

Change-Id: I6765a52ab213c131cca053227ee72b0007552f7f

client/src/com/vaadin/client/ui/VScrollTable.java

@@ -3056,7 +3056,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                                 .hasNext(); columnIndex++) {
                             if (it.next() == this) {
                                 break;
-                            }
+                            }                          
                         }
                     }
                     final int cw = scrollBody.getColWidth(columnIndex);
@@ -3266,10 +3266,9 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
 
                 }
                 if (col.hasAttribute("width")) {
-                    final String widthStr = col.getStringAttribute("width");
                     // Make sure to accomodate for the sort indicator if
                     // necessary.
-                    int width = Integer.parseInt(widthStr);
+                    int width = col.getIntAttribute("width");
                     int widthWithoutAddedIndent = width;
 
                     // get min width with indent, no padding
@@ -3301,12 +3300,25 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                         // save min width without indent
                         c.setWidth(widthWithoutAddedIndent, true);
                     }
+                } else if (col.hasAttribute("er")) {
+                    c.setExpandRatio(col.getFloatAttribute("er"));
+
                 } else if (recalcWidths) {
                     c.setUndefinedWidth();
+
+                } else {
+                    boolean hadExpandRatio = c.getExpandRatio() > 0;
+                    boolean hadDefinedWidth = c.isDefinedWidth();
+                    if (hadExpandRatio || hadDefinedWidth) {
+                        // Someone has removed a expand width or the defined
+                        // width on the server side (setting it to -1), make the
+                        // column undefined again and measure columns again.
+                        c.setUndefinedWidth();
+                        c.setExpandRatio(0);
+                        refreshContentWidths = true;
+                    }
                 }
-                if (col.hasAttribute("er")) {
-                    c.setExpandRatio(col.getFloatAttribute("er"));
-                }
+
                 if (col.hasAttribute("collapsed")) {
                     // ensure header is properly removed from parent (case when
                     // collapsing happens via servers side api)

server/src/com/vaadin/ui/Table.java

@@ -396,10 +396,14 @@ public class Table extends AbstractSelect implements Action.Container,
     private HashMap<Object, Align> columnAlignments = new HashMap<Object, Align>();
 
     /**
-     * Holds column widths in pixels (Integer) or expand ratios (Float) for
-     * visible columns (by propertyId).
+     * Holds column widths in pixels for visible columns (by propertyId).
      */
-    private final HashMap<Object, Object> columnWidths = new HashMap<Object, Object>();
+    private final HashMap<Object, Integer> columnWidths = new HashMap<Object, Integer>();
+
+    /**
+     * Holds column expand rations for visible columns (by propertyId).
+     */
+    private final HashMap<Object, Float> columnExpandRatios = new HashMap<Object, Float>();
 
     /**
      * Holds column generators
@@ -886,10 +890,14 @@ public class Table extends AbstractSelect implements Action.Container,
             // id to store the width of the row header.
             propertyId = ROW_HEADER_FAKE_PROPERTY_ID;
         }
+
+        // Setting column width should remove any expand ratios as well
+        columnExpandRatios.remove(propertyId);
+
         if (width < 0) {
             columnWidths.remove(propertyId);
         } else {
-            columnWidths.put(propertyId, Integer.valueOf(width));
+            columnWidths.put(propertyId, width);
         }
         markAsDirty();
     }
@@ -930,21 +938,39 @@ public class Table extends AbstractSelect implements Action.Container,
      *            the expandRatio used to divide excess space for this column
      */
     public void setColumnExpandRatio(Object propertyId, float expandRatio) {
+        if (propertyId == null) {
+            // Since propertyId is null, this is the row header. Use the magic
+            // id to store the width of the row header.
+            propertyId = ROW_HEADER_FAKE_PROPERTY_ID;
+        }
+
+        // Setting the column expand ratio should remove and defined column
+        // width
+        columnWidths.remove(propertyId);
+
         if (expandRatio < 0) {
-            columnWidths.remove(propertyId);
+            columnExpandRatios.remove(propertyId);
         } else {
-            columnWidths.put(propertyId, new Float(expandRatio));
+            columnExpandRatios.put(propertyId, expandRatio);
         }
+
+        requestRepaint();
     }
 
+    /**
+     * Gets the column expand ratio for a columnd. See
+     * {@link #setColumnExpandRatio(Object, float)}
+     * 
+     * @param propertyId
+     *            columns property id
+     * @return the expandRatio used to divide excess space for this column
+     */
     public float getColumnExpandRatio(Object propertyId) {
-        final Object width = columnWidths.get(propertyId);
-        if (width == null || !(width instanceof Float)) {
+        final Float width = columnExpandRatios.get(propertyId);
+        if (width == null) {
             return -1;
         }
-        final Float value = (Float) width;
-        return value.floatValue();
-
+        return width.floatValue();
     }
 
     /**
@@ -959,12 +985,11 @@ public class Table extends AbstractSelect implements Action.Container,
             // id to retrieve the width of the row header.
             propertyId = ROW_HEADER_FAKE_PROPERTY_ID;
         }
-        final Object width = columnWidths.get(propertyId);
-        if (width == null || !(width instanceof Integer)) {
+        final Integer width = columnWidths.get(propertyId);
+        if (width == null) {
             return -1;
         }
-        final Integer value = (Integer) width;
-        return value.intValue();
+        return width.intValue();
     }
 
     /**
@@ -3434,6 +3459,7 @@ public class Table extends AbstractSelect implements Action.Container,
             target.startTag("column");
             target.addAttribute("cid", ROW_HEADER_COLUMN_KEY);
             paintColumnWidth(target, ROW_HEADER_FAKE_PROPERTY_ID);
+            paintColumnExpandRatio(target, ROW_HEADER_FAKE_PROPERTY_ID);
             target.endTag("column");
         }
         final Collection<?> sortables = getSortableContainerPropertyIds();
@@ -3461,6 +3487,7 @@ public class Table extends AbstractSelect implements Action.Container,
                             .toString());
                 }
                 paintColumnWidth(target, colId);
+                paintColumnExpandRatio(target, colId);
                 target.endTag("column");
             }
         }
@@ -3706,12 +3733,14 @@ public class Table extends AbstractSelect implements Action.Container,
     private void paintColumnWidth(PaintTarget target, final Object columnId)
             throws PaintException {
         if (columnWidths.containsKey(columnId)) {
-            if (getColumnWidth(columnId) > -1) {
-                target.addAttribute("width",
-                        String.valueOf(getColumnWidth(columnId)));
-            } else {
-                target.addAttribute("er", getColumnExpandRatio(columnId));
-            }
+            target.addAttribute("width", getColumnWidth(columnId));
+        }
+    }
+
+    private void paintColumnExpandRatio(PaintTarget target,
+            final Object columnId) throws PaintException {
+        if (columnExpandRatios.containsKey(columnId)) {
+            target.addAttribute("er", getColumnExpandRatio(columnId));
         }
     }
 

uitest/src/com/vaadin/tests/components/table/TableColumnWidthsAndExpandRatios.html

@@ -0,0 +1,83 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
+<head profile="http://selenium-ide.openqa.org/profiles/test-case">
+<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+<link rel="selenium.base" href="http://localhost:8888/" />
+<title>New Test</title>
+</head>
+<body>
+<table cellpadding="1" cellspacing="1" border="1">
+<thead>
+<tr><td rowspan="1" colspan="3">New Test</td></tr>
+</thead><tbody>
+<tr>
+    <td>open</td>
+    <td>/run/com.vaadin.tests.components.table.TableColumnWidthsAndExpandRatios?restartApplication</td>
+    <td></td>
+</tr>
+<tr>
+    <td>screenCapture</td>
+    <td></td>
+    <td>initial-all-columns-undefined</td>
+</tr>
+<!--Resize column 1 making it defined size-->
+<tr>
+    <td>dragAndDrop</td>
+    <td>vaadin=runcomvaadintestscomponentstableTableColumnWidthsAndExpandRatios::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VScrollTable[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]</td>
+    <td>-300,0</td>
+</tr>
+<!--Resize column 2 making it defined size-->
+<tr>
+    <td>dragAndDrop</td>
+    <td>vaadin=runcomvaadintestscomponentstableTableColumnWidthsAndExpandRatios::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VScrollTable[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[1]/domChild[0]</td>
+    <td>-300,0</td>
+</tr>
+<!--Resize column 3 making it defined size-->
+<tr>
+    <td>dragAndDrop</td>
+    <td>vaadin=runcomvaadintestscomponentstableTableColumnWidthsAndExpandRatios::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VScrollTable[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[0]/domChild[2]/domChild[0]</td>
+    <td>-300,0</td>
+</tr>
+<tr>
+    <td>screenCapture</td>
+    <td></td>
+    <td>columns-defined-width</td>
+</tr>
+<!--Reset column 1 width-->
+<tr>
+    <td>mouseClick</td>
+    <td>vaadin=runcomvaadintestscomponentstableTableColumnWidthsAndExpandRatios::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VHorizontalLayout[0]/ChildComponentContainer[0]/VNativeButton[0]</td>
+    <td>114,4</td>
+</tr>
+<tr>
+    <td>screenCapture</td>
+    <td></td>
+    <td>column1-undefined</td>
+</tr>
+<!--Reset column2 width-->
+<tr>
+    <td>mouseClick</td>
+    <td>vaadin=runcomvaadintestscomponentstableTableColumnWidthsAndExpandRatios::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VHorizontalLayout[0]/ChildComponentContainer[1]/VNativeButton[0]</td>
+    <td>98,13</td>
+</tr>
+<tr>
+    <td>screenCapture</td>
+    <td></td>
+    <td>column2-undefined</td>
+</tr>
+<!--Reset column 3 width-->
+<tr>
+    <td>mouseClick</td>
+    <td>vaadin=runcomvaadintestscomponentstableTableColumnWidthsAndExpandRatios::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VHorizontalLayout[0]/ChildComponentContainer[2]/VNativeButton[0]</td>
+    <td>40,10</td>
+</tr>
+<tr>
+    <td>screenCapture</td>
+    <td></td>
+    <td>columns-undefined-width</td>
+</tr>
+
+</tbody></table>
+</body>
+</html>

uitest/src/com/vaadin/tests/components/table/TableColumnWidthsAndExpandRatios.java

@@ -0,0 +1,57 @@
+package com.vaadin.tests.components.table;
+
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.HorizontalLayout;
+import com.vaadin.ui.NativeButton;
+import com.vaadin.ui.Table;
+
+public class TableColumnWidthsAndExpandRatios extends TestBase {
+
+    @Override
+    protected void setup() {
+        getLayout().setSizeFull();
+
+        final Table table = new Table();
+        table.setSizeFull();
+
+        table.addContainerProperty("column1", String.class, "Humpty");
+        table.addContainerProperty("column2", String.class, "Dumpty");
+        table.addContainerProperty("column3", String.class, "Doe");
+
+        for (int row = 0; row < 100; row++) {
+            table.addItem();
+        }
+
+        HorizontalLayout buttons = new HorizontalLayout();
+        for (Object col : table.getContainerPropertyIds()) {
+            buttons.addComponent(createResetButton(col, table));
+        }
+
+        addComponent(table);
+        addComponent(buttons);
+    }
+
+    private NativeButton createResetButton(final Object property,
+            final Table table) {
+        return new NativeButton("Reset " + property + " width",
+                new Button.ClickListener() {
+
+            public void buttonClick(ClickEvent event) {
+                table.setColumnWidth(property, -1);
+            }
+        });
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Changing column width to -1 should remove any previous size measurements";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 7922;
+    }
+
+}