Restore column based width for compatibility selects

10 files changed, 234 insertions, 10 deletions

diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VOptionGroupBase.java b/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VOptionGroupBase.java
index 239e6ff53a..2e5ed2319f 100644
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VOptionGroupBase.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VOptionGroupBase.java
@@ -60,7 +60,8 @@ public abstract class VOptionGroupBase extends Composite implements Field,
 
     private boolean readonly;
 
-    // Intentional removal of cols in compatibility package
+    /** For internal use only. May be removed or replaced in the future. */
+    public int cols = 0;
 
     /** For internal use only. May be removed or replaced in the future. */
     public int rows = 0;
@@ -136,7 +137,14 @@ public abstract class VOptionGroupBase extends Composite implements Field,
         return nullSelectionItemAvailable;
     }
 
-    // Intentional removal of getColumns in compatibility package
+    /**
+     * For internal use only. May be removed or replaced in the future.
+     *
+     * @return "cols" specified in uidl, 0 if not specified
+     */
+    public int getColumns() {
+        return cols;
+    }
 
     /**
      * For internal use only. May be removed or replaced in the future.
diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VTwinColSelect.java b/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VTwinColSelect.java
index 2403750b03..70e8d1892f 100644
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VTwinColSelect.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/ui/VTwinColSelect.java
@@ -367,8 +367,12 @@ public class VTwinColSelect extends VOptionGroupBase implements KeyDownHandler,
 
     /** For internal use only. May be removed or replaced in the future. */
     public void clearInternalWidths() {
-        // Intentional removal of cols in compatibility package
-        int cols = DEFAULT_COLUMN_COUNT;
+        int cols = -1;
+        if (getColumns() > 0) {
+            cols = getColumns();
+        } else {
+            cols = DEFAULT_COLUMN_COUNT;
+        }
 
         if (cols >= 0) {
             String colWidth = cols + "em";
diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/ui/optiongroup/OptionGroupBaseConnector.java b/compatibility-client/src/main/java/com/vaadin/v7/client/ui/optiongroup/OptionGroupBaseConnector.java
index 6fe94fa5a5..1de28c832a 100644
--- a/compatibility-client/src/main/java/com/vaadin/v7/client/ui/optiongroup/OptionGroupBaseConnector.java
+++ b/compatibility-client/src/main/java/com/vaadin/v7/client/ui/optiongroup/OptionGroupBaseConnector.java
@@ -51,15 +51,21 @@ public abstract class OptionGroupBaseConnector extends AbstractFieldConnector
         getWidget().nullSelectionItemAvailable = uidl
                 .getBooleanAttribute("nullselectitem");
 
-        // Support for cols has been dropped.
-
+        if (uidl.hasAttribute("cols")) {
+            getWidget().cols = uidl.getIntAttribute("cols");
+        }
         if (uidl.hasAttribute("rows")) {
             getWidget().rows = uidl.getIntAttribute("rows");
         }
 
         final UIDL ops = uidl.getChildUIDL(0);
 
-        // Method getColumns has been removed
+        if (getWidget().getColumns() > 0) {
+            getWidget().container.setWidth(getWidget().getColumns() + "em");
+            if (getWidget().container != getWidget().optionsContainer) {
+                getWidget().optionsContainer.setWidth("100%");
+            }
+        }
 
         getWidget().buildOptions(ops);
 
diff --git a/compatibility-server/src/main/java/com/vaadin/v7/ui/ListSelect.java b/compatibility-server/src/main/java/com/vaadin/v7/ui/ListSelect.java
index 91b3c8375d..4a741b293c 100644
--- a/compatibility-server/src/main/java/com/vaadin/v7/ui/ListSelect.java
+++ b/compatibility-server/src/main/java/com/vaadin/v7/ui/ListSelect.java
@@ -33,6 +33,7 @@ import com.vaadin.v7.data.Container;
 @Deprecated
 public class ListSelect extends AbstractSelect {
 
+    private int columns = 0;
     private int rows = 0;
 
     public ListSelect() {
@@ -51,7 +52,45 @@ public class ListSelect extends AbstractSelect {
         super(caption);
     }
 
-    // Columns are no longer used for width.
+    /**
+     * Sets the width of the component so that it can display approximately the
+     * given number of letters.
+     * <p>
+     * Calling {@code setColumns(10);} is equivalent to calling
+     * {@code setWidth("10em");}
+     * </p>
+     *
+     * @deprecated As of 7.0. "Columns" does not reflect the exact number of
+     *             characters that will be displayed. It is better to use
+     *             setWidth together with "em" to control the width of the
+     *             field.
+     * @param columns
+     *            the number of columns to set.
+     */
+    @Deprecated
+    public void setColumns(int columns) {
+        if (columns < 0) {
+            columns = 0;
+        }
+        if (this.columns != columns) {
+            this.columns = columns;
+            markAsDirty();
+        }
+    }
+
+    /**
+     * Gets the number of columns for the component.
+     *
+     * @see #setColumns(int)
+     * @deprecated As of 7.0. "Columns" does not reflect the exact number of
+     *             characters that will be displayed. It is better to use
+     *             setWidth together with "em" to control the width of the
+     *             field.
+     */
+    @Deprecated
+    public int getColumns() {
+        return columns;
+    }
 
     public int getRows() {
         return rows;
@@ -78,6 +117,10 @@ public class ListSelect extends AbstractSelect {
     @Override
     public void paintContent(PaintTarget target) throws PaintException {
         target.addAttribute("type", "list");
+        // Adds the number of columns
+        if (columns != 0) {
+            target.addAttribute("cols", columns);
+        }
         // Adds the number of rows
         if (rows != 0) {
             target.addAttribute("rows", rows);
diff --git a/compatibility-server/src/main/java/com/vaadin/v7/ui/NativeSelect.java b/compatibility-server/src/main/java/com/vaadin/v7/ui/NativeSelect.java
index 3f53969bc7..e69f7c620b 100644
--- a/compatibility-server/src/main/java/com/vaadin/v7/ui/NativeSelect.java
+++ b/compatibility-server/src/main/java/com/vaadin/v7/ui/NativeSelect.java
@@ -42,6 +42,9 @@ import com.vaadin.v7.event.FieldEvents;
 public class NativeSelect extends AbstractSelect
         implements FieldEvents.BlurNotifier, FieldEvents.FocusNotifier {
 
+    // width in characters, mimics TextField
+    private int columns = 0;
+
     public NativeSelect() {
         super();
         registerRpc(new FocusAndBlurServerRpcDecorator(this, this::fireEvent));
@@ -62,9 +65,53 @@ public class NativeSelect extends AbstractSelect
         registerRpc(new FocusAndBlurServerRpcDecorator(this, this::fireEvent));
     }
 
+    /**
+     * Sets the width of the component so that it can display approximately the
+     * given number of letters.
+     * <p>
+     * Calling {@code setColumns(10);} is equivalent to calling
+     * {@code setWidth("10em");}
+     * </p>
+     *
+     * @deprecated As of 7.0. "Columns" does not reflect the exact number of
+     *             characters that will be displayed. It is better to use
+     *             setWidth together with "em" to control the width of the
+     *             field.
+     * @param columns
+     *            the number of columns to set.
+     */
+    @Deprecated
+    public void setColumns(int columns) {
+        if (columns < 0) {
+            columns = 0;
+        }
+        if (this.columns != columns) {
+            this.columns = columns;
+            markAsDirty();
+        }
+    }
+
+    /**
+     * Gets the number of columns for the component.
+     *
+     * @see #setColumns(int)
+     * @deprecated As of 7.0. "Columns" does not reflect the exact number of
+     *             characters that will be displayed. It is better to use
+     *             setWidth together with "em" to control the width of the
+     *             field.
+     */
+    @Deprecated
+    public int getColumns() {
+        return columns;
+    }
+
     @Override
     public void paintContent(PaintTarget target) throws PaintException {
         target.addAttribute("type", "native");
+        // Adds the number of columns
+        if (columns != 0) {
+            target.addAttribute("cols", columns);
+        }
 
         super.paintContent(target);
     }
diff --git a/compatibility-server/src/main/java/com/vaadin/v7/ui/TwinColSelect.java b/compatibility-server/src/main/java/com/vaadin/v7/ui/TwinColSelect.java
index 965fe85e99..c90cddb595 100644
--- a/compatibility-server/src/main/java/com/vaadin/v7/ui/TwinColSelect.java
+++ b/compatibility-server/src/main/java/com/vaadin/v7/ui/TwinColSelect.java
@@ -35,6 +35,7 @@ import com.vaadin.v7.shared.ui.twincolselect.TwinColSelectState;
 @Deprecated
 public class TwinColSelect extends AbstractSelect {
 
+    private int columns = 0;
     private int rows = 0;
 
     private String leftColumnCaption;
@@ -65,7 +66,45 @@ public class TwinColSelect extends AbstractSelect {
         setMultiSelect(true);
     }
 
-    // Columns are no longer used for width.
+    /**
+     * Sets the width of the component so that it displays approximately the
+     * given number of letters in each of the two selects.
+     * <p>
+     * Calling {@code setColumns(10);} is roughly equivalent to calling
+     * {@code setWidth((10*2+4)+"10em");}
+     * </p>
+     *
+     * @deprecated As of 7.0. "Columns" does not reflect the exact number of
+     *             characters that will be displayed. It is better to use
+     *             setWidth together with "em" to control the width of the
+     *             field.
+     * @param columns
+     *            the number of columns to set.
+     */
+    @Deprecated
+    public void setColumns(int columns) {
+        if (columns < 0) {
+            columns = 0;
+        }
+        if (this.columns != columns) {
+            this.columns = columns;
+            markAsDirty();
+        }
+    }
+
+    /**
+     * Gets the number of columns for the component.
+     *
+     * @see #setColumns(int)
+     * @deprecated As of 7.0. "Columns" does not reflect the exact number of
+     *             characters that will be displayed. It is better to use
+     *             setWidth together with "em" to control the width of the
+     *             field.
+     */
+    @Deprecated
+    public int getColumns() {
+        return columns;
+    }
 
     public int getRows() {
         return rows;
@@ -107,7 +146,10 @@ public class TwinColSelect extends AbstractSelect {
     @Override
     public void paintContent(PaintTarget target) throws PaintException {
         target.addAttribute("type", "twincol");
-        // Width is no longer based on columns
+        // Adds the number of columns
+        if (columns != 0) {
+            target.addAttribute("cols", columns);
+        }
         // Adds the number of rows
         if (rows != 0) {
             target.addAttribute("rows", rows);
diff --git a/uitest/src/main/java/com/vaadin/tests/components/listselect/ListSelects.java b/uitest/src/main/java/com/vaadin/tests/components/listselect/ListSelects.java
index f42098eb88..c98c64ccd0 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/listselect/ListSelects.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/listselect/ListSelects.java
@@ -7,6 +7,13 @@ import com.vaadin.v7.ui.ListSelect;
 
 public class ListSelects extends AbstractSelectTestCase<ListSelect> {
 
+    private Command<ListSelect, Integer> colsCommand = new Command<ListSelect, Integer>() {
+        @Override
+        public void execute(ListSelect c, Integer value, Object data) {
+            c.setColumns(value);
+        }
+    };
+
     private Command<ListSelect, Integer> rowsCommand = new Command<ListSelect, Integer>() {
         @Override
         public void execute(ListSelect c, Integer value, Object data) {
@@ -23,6 +30,7 @@ public class ListSelects extends AbstractSelectTestCase<ListSelect> {
     protected void createActions() {
         super.createActions();
         createRowsAction(CATEGORY_FEATURES);
+        createColsAction(CATEGORY_FEATURES);
     }
 
     private void createRowsAction(String category) {
@@ -30,4 +38,8 @@ public class ListSelects extends AbstractSelectTestCase<ListSelect> {
         createSelectAction("Rows", category, options, "0", rowsCommand);
     }
 
+    private void createColsAction(String category) {
+        LinkedHashMap<String, Integer> options = createIntegerOptions(20);
+        createSelectAction("Columns", category, options, "0", colsCommand);
+    }
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/select/TwinColSelectTest.java b/uitest/src/main/java/com/vaadin/tests/components/select/TwinColSelectTest.java
index 411cb361a3..ed36033c50 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/select/TwinColSelectTest.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/select/TwinColSelectTest.java
@@ -13,6 +13,13 @@ public class TwinColSelectTest extends AbstractSelectTestCase<TwinColSelect> {
         }
     };
 
+    private Command<TwinColSelect, Integer> colsCommand = new Command<TwinColSelect, Integer>() {
+        @Override
+        public void execute(TwinColSelect c, Integer value, Object data) {
+            c.setColumns(value);
+        }
+    };
+
     private Command<TwinColSelect, String> leftColumnCaptionCommand = new Command<TwinColSelect, String>() {
 
         @Override
@@ -38,6 +45,7 @@ public class TwinColSelectTest extends AbstractSelectTestCase<TwinColSelect> {
     protected void createActions() {
         super.createActions();
         createRowsAction(CATEGORY_FEATURES);
+        createColsAction(CATEGORY_FEATURES);
         createCaptionActions(CATEGORY_FEATURES);
     }
 
@@ -46,6 +54,11 @@ public class TwinColSelectTest extends AbstractSelectTestCase<TwinColSelect> {
         createSelectAction("Rows", category, options, "0", rowsCommand);
     }
 
+    private void createColsAction(String category) {
+        LinkedHashMap<String, Integer> options = createIntegerOptions(20);
+        createSelectAction("Columns", category, options, "0", colsCommand);
+    }
+
     private void createCaptionActions(String category) {
         createSelectAction("Left column caption", category,
                 createCaptionOptions(), "-", leftColumnCaptionCommand);
diff --git a/uitest/src/main/java/com/vaadin/tests/components/twincolselect/TwinColSelects.java b/uitest/src/main/java/com/vaadin/tests/components/twincolselect/TwinColSelects.java
index 230825d808..602916d509 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/twincolselect/TwinColSelects.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/twincolselect/TwinColSelects.java
@@ -16,6 +16,22 @@ public class TwinColSelects extends AbstractSelectTestCase<TwinColSelect> {
     protected void createActions() {
         super.createActions();
         createRowSelectAction();
+        createColumnSelectAction();
+    }
+
+    private void createColumnSelectAction() {
+        LinkedHashMap<String, Integer> options = new LinkedHashMap<String, Integer>();
+        options.put("-", 0);
+        for (int i = 1; i <= 10; i++) {
+            options.put(String.valueOf(i), i);
+        }
+        options.put("50", 50);
+        options.put("100", 100);
+        options.put("1000", 1000);
+
+        super.createSelectAction("Columns", CATEGORY_DATA_SOURCE, options, "-",
+                columnsAction);
+
     }
 
     private void createRowSelectAction() {
@@ -33,6 +49,14 @@ public class TwinColSelects extends AbstractSelectTestCase<TwinColSelect> {
 
     }
 
+    private Command<TwinColSelect, Integer> columnsAction = new Command<TwinColSelect, Integer>() {
+
+        @Override
+        public void execute(TwinColSelect c, Integer value, Object data) {
+            c.setColumns(value);
+        }
+    };
+
     private Command<TwinColSelect, Integer> rowsAction = new Command<TwinColSelect, Integer>() {
 
         @Override
diff --git a/uitest/src/main/java/com/vaadin/v7/tests/components/nativeselect/NativeSelects.java b/uitest/src/main/java/com/vaadin/v7/tests/components/nativeselect/NativeSelects.java
index bb84e88d46..5679957669 100644
--- a/uitest/src/main/java/com/vaadin/v7/tests/components/nativeselect/NativeSelects.java
+++ b/uitest/src/main/java/com/vaadin/v7/tests/components/nativeselect/NativeSelects.java
@@ -1,5 +1,7 @@
 package com.vaadin.v7.tests.components.nativeselect;
 
+import java.util.LinkedHashMap;
+
 import com.vaadin.tests.components.select.AbstractSelectTestCase;
 import com.vaadin.v7.ui.NativeSelect;
 
@@ -13,6 +15,29 @@ public class NativeSelects extends AbstractSelectTestCase<NativeSelect> {
     @Override
     protected void createActions() {
         super.createActions();
+        createColumnSelectAction();
+    }
+
+    private void createColumnSelectAction() {
+        LinkedHashMap<String, Integer> options = new LinkedHashMap<String, Integer>();
+        options.put("-", 0);
+        for (int i = 1; i <= 10; i++) {
+            options.put(String.valueOf(i), i);
+        }
+        options.put("50", 50);
+        options.put("100", 100);
+        options.put("1000", 1000);
+
+        super.createSelectAction("Columns", CATEGORY_DATA_SOURCE, options, "-",
+                columnsAction);
+
     }
 
+    private Command<NativeSelect, Integer> columnsAction = new Command<NativeSelect, Integer>() {
+
+        @Override
+        public void execute(NativeSelect c, Integer value, Object data) {
+            c.setColumns(value);
+        }
+    };
 }