Table: column not expanding after setColumnExpandRatio (#15101)

Partial fix for setting/unsetting expand ratios of existing table.
Result of this patch:
1) when switching off column expand ratio, natural column width is
restored.
2) changes in method run() for the same (though incorrect) treatment of
expand ratio as in sizeInit()

Change-Id: I2119eb1041e205a54373ac40a9ce8fdd14d70ad8

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

@@ -2198,6 +2198,8 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             final FooterCell fCell = (FooterCell) footCells.next();
             boolean needsIndent = hierarchyColumnIndent > 0
                     && hCell.isHierarchyColumn();
+            hCell.saveNaturalColumnWidthIfNotSaved(i);
+            fCell.saveNaturalColumnWidthIfNotSaved(i);
             int w = hCell.getWidth();
             if (hCell.isDefinedWidth()) {
                 // server has defined column width explicitly
@@ -2223,8 +2225,10 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     int footerWidth = fCell.getNaturalColumnWidth(i);
                     w = headerWidth > footerWidth ? headerWidth : footerWidth;
                 }
-                hCell.setNaturalMinimumColumnWidth(w);
-                fCell.setNaturalMinimumColumnWidth(w);
+                if (w != 0) {
+                    hCell.setNaturalMinimumColumnWidth(w);
+                    fCell.setNaturalMinimumColumnWidth(w);
+                }
             }
             widths[i] = w;
             total += w;
@@ -2945,6 +2949,10 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             }
         }
 
+        private void setUndefinedWidthFlagOnly() {
+            definedWidth = false;
+        }
+
         /**
          * Detects if width is fixed by developer on server side or resized to
          * current width by user.
@@ -3361,6 +3369,28 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             return align;
         }
 
+        protected void saveNaturalColumnWidthIfNotSaved(int columnIndex) {
+            if (naturalWidth < 0) {
+                // This is recently revealed column. Try to detect a proper
+                // value (greater of header and data columns)
+
+                int hw = captionContainer.getOffsetWidth() + getHeaderPadding();
+                if (BrowserInfo.get().isGecko()) {
+                    hw += sortIndicator.getOffsetWidth();
+                }
+                if (columnIndex < 0) {
+                    columnIndex = 0;
+                    for (Iterator<Widget> it = tHead.iterator(); it.hasNext(); columnIndex++) {
+                        if (it.next() == this) {
+                            break;
+                        }
+                    }
+                }
+                final int cw = scrollBody.getColWidth(columnIndex);
+                naturalWidth = (hw > cw ? hw : cw);
+            }
+        }
+
         /**
          * Detects the natural minimum width for the column of this header cell.
          * If column is resized by user or the width is defined by server the
@@ -3374,33 +3404,13 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         public int getNaturalColumnWidth(int columnIndex) {
             final int iw = columnIndex == getHierarchyColumnIndex() ? scrollBody
                     .getMaxIndent() : 0;
+            saveNaturalColumnWidthIfNotSaved(columnIndex);
             if (isDefinedWidth()) {
                 if (iw > width) {
                     return iw;
                 }
                 return width;
             } else {
-                if (naturalWidth < 0) {
-                    // This is recently revealed column. Try to detect a proper
-                    // value (greater of header and data columns)
-
-                    int hw = captionContainer.getOffsetWidth()
-                            + getHeaderPadding();
-                    if (BrowserInfo.get().isGecko()) {
-                        hw += sortIndicator.getOffsetWidth();
-                    }
-                    if (columnIndex < 0) {
-                        columnIndex = 0;
-                        for (Iterator<Widget> it = tHead.iterator(); it
-                                .hasNext(); columnIndex++) {
-                            if (it.next() == this) {
-                                break;
-                            }
-                        }
-                    }
-                    final int cw = scrollBody.getColWidth(columnIndex);
-                    naturalWidth = (hw > cw ? hw : cw);
-                }
                 if (iw > naturalWidth) {
                     // indent is temporary value, naturalWidth shouldn't be
                     // updated
@@ -3637,7 +3647,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     }
                 } else if (col.hasAttribute("er")) {
                     c.setExpandRatio(col.getFloatAttribute("er"));
-
+                    c.setUndefinedWidthFlagOnly();
                 } else if (recalcWidths) {
                     c.setUndefinedWidth();
 
@@ -4295,6 +4305,26 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
             return cid;
         }
 
+        protected void saveNaturalColumnWidthIfNotSaved(int columnIndex) {
+            if (naturalWidth < 0) {
+                // This is recently revealed column. Try to detect a proper
+                // value (greater of header and data cols)
+
+                final int hw = ((Element) getElement().getLastChild())
+                        .getOffsetWidth() + getHeaderPadding();
+                if (columnIndex < 0) {
+                    columnIndex = 0;
+                    for (Iterator<Widget> it = tHead.iterator(); it.hasNext(); columnIndex++) {
+                        if (it.next() == this) {
+                            break;
+                        }
+                    }
+                }
+                final int cw = scrollBody.getColWidth(columnIndex);
+                naturalWidth = (hw > cw ? hw : cw);
+            }
+        }
+
         /**
          * Detects the natural minimum width for the column of this header cell.
          * If column is resized by user or the width is defined by server the
@@ -4308,31 +4338,13 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
         public int getNaturalColumnWidth(int columnIndex) {
             final int iw = columnIndex == getHierarchyColumnIndex() ? scrollBody
                     .getMaxIndent() : 0;
+            saveNaturalColumnWidthIfNotSaved(columnIndex);
             if (isDefinedWidth()) {
                 if (iw > width) {
                     return iw;
                 }
                 return width;
             } else {
-                if (naturalWidth < 0) {
-                    // This is recently revealed column. Try to detect a proper
-                    // value (greater of header and data
-                    // cols)
-
-                    final int hw = ((Element) getElement().getLastChild())
-                            .getOffsetWidth() + getHeaderPadding();
-                    if (columnIndex < 0) {
-                        columnIndex = 0;
-                        for (Iterator<Widget> it = tHead.iterator(); it
-                                .hasNext(); columnIndex++) {
-                            if (it.next() == this) {
-                                break;
-                            }
-                        }
-                    }
-                    final int cw = scrollBody.getColWidth(columnIndex);
-                    naturalWidth = (hw > cw ? hw : cw);
-                }
                 if (iw > naturalWidth) {
                     return iw;
                 } else {
@@ -6853,6 +6865,14 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     // natural width already includes indent if any
                     int naturalColumnWidth = hCell
                             .getNaturalColumnWidth(colIndex);
+                    /*
+                     * TODO If there is extra width, expand ratios are for
+                     * additional extra widths, not for absolute column widths.
+                     * Should be fixed in sizeInit(), too.
+                     */
+                    if (hCell.getExpandRatio() > 0) {
+                        naturalColumnWidth = 0;
+                    }
                     usedMinimumWidth += naturalColumnWidth;
                     expandRatioDivider += hCell.getExpandRatio();
                     if (hasIndent) {
@@ -6939,6 +6959,9 @@ public class VScrollTable extends FlowPanel implements HasWidgets,
                     int newSpace;
                     if (expandRatioDivider > 0) {
                         // divide excess space by expand ratios
+                        if (hCell.getExpandRatio() > 0) {
+                            w = 0;
+                        }
                         newSpace = Math.round((w + extraSpace
                                 * hCell.getExpandRatio() / expandRatioDivider));
                     } else {

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

@@ -0,0 +1,119 @@
+/*
+ * 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.table;
+
+/**
+ * Test that column natural width is restored after removing expand ratio
+ * 
+ * @author Vaadin Ltd
+ */
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+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 TableAfterRemovingExpandRatios extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest req) {
+        getLayout().setSizeFull();
+
+        Table tableInitial = createTable();
+        Table table = createTable();
+
+        HorizontalLayout buttons = new HorizontalLayout();
+        buttons.addComponent(createSetExpandRatiosButton(table));
+        buttons.addComponent(createUnsetExpandRatiosButton(table));
+        buttons.addComponent(createAddItemButton(table));
+
+        addComponent(tableInitial);
+        addComponent(table);
+        addComponent(buttons);
+    }
+
+    private Table createTable() {
+        Table table = new Table();
+        table.addContainerProperty("column1", String.class, "Humprtrtwwww");
+        table.addContainerProperty("column2", String.class, "Dumpttttwwww");
+        table.addContainerProperty("column3", String.class, "Dogtt");
+
+        for (int row = 0; row < 4; row++) {
+            table.addItem();
+        }
+        table.setWidth("500px");
+        table.setHeight("300px");
+        return table;
+
+    }
+
+    private NativeButton createSetExpandRatiosButton(final Table table) {
+        NativeButton button = new NativeButton("Set expand",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        table.setColumnExpandRatio("column1", 1.0f);
+                        table.setColumnExpandRatio("column2", 3.0f);
+
+                    }
+                });
+        button.setId("expand-button");
+        return button;
+    }
+
+    private NativeButton createUnsetExpandRatiosButton(final Table table) {
+        NativeButton button = new NativeButton("Unset expand",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        table.setColumnExpandRatio("column1", -1);
+                        table.setColumnExpandRatio("column2", -1);
+                    }
+                });
+        button.setId("unexpand-button");
+        return button;
+    }
+
+    private NativeButton createAddItemButton(final Table table) {
+        NativeButton button = new NativeButton("Add item",
+                new Button.ClickListener() {
+
+                    @Override
+                    public void buttonClick(ClickEvent event) {
+                        table.addItem();
+                    }
+                });
+        button.setId("add-button");
+        return button;
+
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "On setting expand ratios to false previous column widths should be restored";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 15101;
+    }
+
+}

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

@@ -0,0 +1,92 @@
+/*
+ * 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.table;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+
+import java.util.List;
+
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Tests checks that column width is restored after removing expand ratios.
+ * 
+ * @author Vaadin Ltd
+ */
+public class TableAfterRemovingExpandRatiosTest extends MultiBrowserTest {
+
+    private WebElement initialHeader;
+    private WebElement expandedHeader;
+
+    private WebElement expandButton;
+    private WebElement unExpandButton;
+
+    @Override
+    public void setup() throws Exception {
+        super.setup();
+        openTestURL();
+
+        List<WebElement> tables = driver.findElements(By.className("v-table"));
+
+        initialHeader = tables.get(0).findElement(
+                By.className("v-table-header-cell"));
+        expandedHeader = tables.get(1).findElement(
+                By.className("v-table-header-cell"));
+
+        expandButton = getDriver().findElement(By.id("expand-button"));
+        unExpandButton = getDriver().findElement(By.id("unexpand-button"));
+    }
+
+    @Test
+    public void testRemovingExpandRatios() {
+
+        clickAndWait(expandButton);
+        assertThat("Column widths should not be equal after expanding",
+                initialHeader.getSize().getWidth(), not(expandedHeader
+                        .getSize().getWidth()));
+
+        clickAndWait(unExpandButton);
+        assertThat("Column widths should be equal after unexpanding",
+                initialHeader.getSize().getWidth(), is(expandedHeader.getSize()
+                        .getWidth()));
+    }
+
+    @Test
+    public void testRemovingExpandRatiosAfterAddingNewItem() {
+
+        WebElement addItemButton = getDriver().findElement(By.id("add-button"));
+
+        clickAndWait(expandButton);
+        clickAndWait(addItemButton);
+        clickAndWait(unExpandButton);
+        assertThat(
+                "Column widths should be equal after adding item and unexpanding",
+                initialHeader.getSize().getWidth(), is(expandedHeader.getSize()
+                        .getWidth()));
+    }
+
+    private void clickAndWait(WebElement elem) {
+        elem.click();
+        testBench(driver).waitForVaadin();
+    }
+
+}