From a062383b16cc1394baa604a0841416d646e91fba Mon Sep 17 00:00:00 2001 From: Henri Sara Date: Wed, 8 Feb 2012 14:18:08 +0200 Subject: [PATCH] Remove support for Select in multi-select mode (#8169, #8326). Also includes some related cleanup as this was the last special case where the widget was selected on the client side based on UIDL fields other than tag. --- .../gwt/client/ApplicationConnection.java | 3 +- .../vaadin/terminal/gwt/client/WidgetSet.java | 56 +++---------------- src/com/vaadin/ui/AbstractSelect.java | 5 +- src/com/vaadin/ui/ComboBox.java | 12 ---- src/com/vaadin/ui/Form.java | 11 ++-- src/com/vaadin/ui/Select.java | 6 +- .../tests/components/select/MultiSelect.java | 32 ----------- .../tests/layouts/OrderedLayoutBasics.java | 23 +++++--- .../com/vaadin/tests/tickets/Ticket1230.java | 1 - 9 files changed, 39 insertions(+), 110 deletions(-) delete mode 100644 tests/testbench/com/vaadin/tests/components/select/MultiSelect.java diff --git a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java index 65827fac0c..201c3a1cd6 100644 --- a/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java +++ b/src/com/vaadin/terminal/gwt/client/ApplicationConnection.java @@ -1886,7 +1886,8 @@ public class ApplicationConnection { final String pid = uidl.getId(); if (!paintableMap.hasPaintable(pid)) { // Create and register a new paintable if no old was found - VPaintableWidget p = widgetSet.createWidget(uidl, configuration); + VPaintableWidget p = widgetSet.createWidget(uidl.getTag(), + configuration); p.setConnection(this); p.setId(pid); p.init(); diff --git a/src/com/vaadin/terminal/gwt/client/WidgetSet.java b/src/com/vaadin/terminal/gwt/client/WidgetSet.java index 390d79ce17..7874c9d068 100644 --- a/src/com/vaadin/terminal/gwt/client/WidgetSet.java +++ b/src/com/vaadin/terminal/gwt/client/WidgetSet.java @@ -6,8 +6,6 @@ package com.vaadin.terminal.gwt.client; import com.google.gwt.core.client.GWT; import com.google.gwt.user.client.ui.Widget; -import com.vaadin.terminal.gwt.client.ui.VFilterSelectPaintable; -import com.vaadin.terminal.gwt.client.ui.VListSelectPaintable; import com.vaadin.terminal.gwt.client.ui.VUnknownComponentPaintable; public class WidgetSet { @@ -24,15 +22,15 @@ public class WidgetSet { * component must be a {@link Widget} that implements * {@link VPaintableWidget}. * - * @param uidl - * UIDL to be painted with returned component. + * @param tag + * component type tag for the component to create * @param client * the application connection that whishes to instantiate widget * * @return New uninitialized and unregistered component that can paint given * UIDL. */ - public VPaintableWidget createWidget(UIDL uidl, + public VPaintableWidget createWidget(String tag, ApplicationConfiguration conf) { /* * Yes, this (including the generated code in WidgetMap) may look very @@ -40,15 +38,14 @@ public class WidgetSet { * Luckily this is mostly written by WidgetSetGenerator, here are just * some hacks. Extra instantiation code is needed if client side widget * has no "native" counterpart on client side. - * - * TODO should try to get rid of these exceptions here */ - final Class classType = resolveWidgetType( - uidl, conf); + Class classType = resolveWidgetType(tag, + conf); + if (classType == null || classType == VUnknownComponentPaintable.class) { String serverSideName = conf - .getUnknownServerClassNameByEncodedTagName(uidl.getTag()); + .getUnknownServerClassNameByEncodedTagName(tag); VUnknownComponentPaintable c = GWT .create(VUnknownComponentPaintable.class); c.setServerSideClassName(serverSideName); @@ -62,42 +59,15 @@ public class WidgetSet { } - protected Class resolveWidgetType(UIDL uidl, + protected Class resolveWidgetType(String tag, ApplicationConfiguration conf) { - final String tag = uidl.getTag(); - Class widgetClass = conf .getWidgetClassByEncodedTag(tag); - // add our historical quirks - - if (widgetClass == VFilterSelectPaintable.class) { - if (uidl.hasAttribute("type")) { - final String type = uidl.getStringAttribute("type").intern(); - if ("legacy-multi" == type) { - return VListSelectPaintable.class; - } - } - } return widgetClass; } - /** - * Test if the given component implementation conforms to UIDL. - * - * @param currentWidget - * Current implementation of the component - * @param uidl - * UIDL to test against - * @return true iff createWidget would return a new component of the same - * class than currentWidget - */ - public boolean isCorrectImplementation(Widget currentWidget, UIDL uidl, - ApplicationConfiguration conf) { - return currentWidget.getClass() == resolveWidgetType(uidl, conf); - } - /** * Due its nature, GWT does not support dynamic classloading. To bypass this * limitation, widgetset must have function that returns Class by its fully @@ -115,16 +85,6 @@ public class WidgetSet { Class implementationByServerSideClassName = widgetMap .getImplementationByServerSideClassName(fullyqualifiedName); - /* - * Also ensure that our historical quirks have their instantiators - * loaded. Without these, legacy code will throw NPEs when e.g. a Select - * is in multiselect mode, causing the clientside implementation to - * *actually* be VListSelect, when the annotation says VFilterSelect - */ - if (fullyqualifiedName.equals("com.vaadin.ui.Select")) { - loadImplementation(VListSelectPaintable.class); - } - return implementationByServerSideClassName; } diff --git a/src/com/vaadin/ui/AbstractSelect.java b/src/com/vaadin/ui/AbstractSelect.java index 5e086f0b8d..47f132aa77 100644 --- a/src/com/vaadin/ui/AbstractSelect.java +++ b/src/com/vaadin/ui/AbstractSelect.java @@ -976,10 +976,13 @@ public abstract class AbstractSelect extends AbstractField implements } /** - * Sets the multiselect mode. Setting multiselect mode false may loose + * Sets the multiselect mode. Setting multiselect mode false may lose * selection information: if selected items set contains one or more * selected items, only one of the selected items is kept as selected. * + * Subclasses of AbstractSelect can choose not to support changing the + * multiselect mode, and may throw {@link UnsupportedOperationException}. + * * @param multiSelect * the New value of property multiSelect. */ diff --git a/src/com/vaadin/ui/ComboBox.java b/src/com/vaadin/ui/ComboBox.java index 013fe6ab85..12db8c18af 100644 --- a/src/com/vaadin/ui/ComboBox.java +++ b/src/com/vaadin/ui/ComboBox.java @@ -34,36 +34,24 @@ public class ComboBox extends Select { private boolean textInputAllowed = true; public ComboBox() { - setMultiSelect(false); setNewItemsAllowed(false); } public ComboBox(String caption, Collection options) { super(caption, options); - setMultiSelect(false); setNewItemsAllowed(false); } public ComboBox(String caption, Container dataSource) { super(caption, dataSource); - setMultiSelect(false); setNewItemsAllowed(false); } public ComboBox(String caption) { super(caption); - setMultiSelect(false); setNewItemsAllowed(false); } - @Override - public void setMultiSelect(boolean multiSelect) { - if (multiSelect && !isMultiSelect()) { - throw new UnsupportedOperationException("Multiselect not supported"); - } - super.setMultiSelect(multiSelect); - } - /** * Gets the current input prompt. * diff --git a/src/com/vaadin/ui/Form.java b/src/com/vaadin/ui/Form.java index c79804c7e7..dd3710cd2c 100644 --- a/src/com/vaadin/ui/Form.java +++ b/src/com/vaadin/ui/Form.java @@ -861,13 +861,16 @@ public class Form extends AbstractField implements Item.Editor, * match. Null values are not supported. *

* + * Note: since Vaadin 7.0, returns an {@link AbstractSelect} instead of a + * {@link Select}. + * * @param propertyId * the id of the property. * @param values * @param descriptions * @return the select property generated */ - public Select replaceWithSelect(Object propertyId, Object[] values, + public AbstractSelect replaceWithSelect(Object propertyId, Object[] values, Object[] descriptions) { // Checks the parameters @@ -927,10 +930,8 @@ public class Form extends AbstractField implements Item.Editor, } // Creates the new field matching to old field parameters - final Select newField = new Select(); - if (isMultiselect) { - newField.setMultiSelect(true); - } + final AbstractSelect newField = isMultiselect ? new ListSelect() + : new Select(); newField.setCaption(oldField.getCaption()); newField.setReadOnly(oldField.isReadOnly()); newField.setReadThrough(oldField.isReadThrough()); diff --git a/src/com/vaadin/ui/Select.java b/src/com/vaadin/ui/Select.java index 38785f3ab9..c1da88acea 100644 --- a/src/com/vaadin/ui/Select.java +++ b/src/com/vaadin/ui/Select.java @@ -754,11 +754,15 @@ public class Select extends AbstractSelect implements AbstractSelect.Filtering, * @deprecated use {@link ListSelect}, {@link OptionGroup} or * {@link TwinColSelect} instead * @see com.vaadin.ui.AbstractSelect#setMultiSelect(boolean) + * @throws UnsupportedOperationException + * if trying to activate multiselect mode */ @Deprecated @Override public void setMultiSelect(boolean multiSelect) { - super.setMultiSelect(multiSelect); + if (multiSelect) { + throw new UnsupportedOperationException("Multiselect not supported"); + } } /** diff --git a/tests/testbench/com/vaadin/tests/components/select/MultiSelect.java b/tests/testbench/com/vaadin/tests/components/select/MultiSelect.java deleted file mode 100644 index d823d1fd10..0000000000 --- a/tests/testbench/com/vaadin/tests/components/select/MultiSelect.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.vaadin.tests.components.select; - -import com.vaadin.tests.components.TestBase; -import com.vaadin.ui.Select; - -public class MultiSelect extends TestBase { - - @SuppressWarnings("deprecation") - @Override - protected void setup() { - Select selectComponent = new Select(); - selectComponent.setMultiSelect(true); - - String[] selection = { "One", "Hund", "Three" }; - for (String word : selection) { - selectComponent.addItem(word); - } - - addComponent(selectComponent); - } - - @Override - protected String getDescription() { - return "The select is in multi select mode and should be rendered as such"; - } - - @Override - protected Integer getTicketNumber() { - return 4553; - } - -} diff --git a/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java b/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java index 1fe11f1a92..9f18ef7887 100644 --- a/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java +++ b/tests/testbench/com/vaadin/tests/layouts/OrderedLayoutBasics.java @@ -16,8 +16,8 @@ import com.vaadin.ui.Component; import com.vaadin.ui.HorizontalLayout; import com.vaadin.ui.Label; import com.vaadin.ui.Layout; +import com.vaadin.ui.ListSelect; import com.vaadin.ui.Panel; -import com.vaadin.ui.Select; import com.vaadin.ui.TextArea; import com.vaadin.ui.TextField; import com.vaadin.ui.VerticalLayout; @@ -116,13 +116,13 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(tf); ol.setComponentAlignment(tf, Alignment.BOTTOM_LEFT); - Select s = new Select("100% high select"); + ListSelect s = new ListSelect("100% high select"); s.setMultiSelect(true); s.setHeight("100%"); s.setWidth(""); ol.addComponent(s); - s = new Select("200 px high select"); + s = new ListSelect("200 px high select"); s.setMultiSelect(true); s.setHeight("200px"); s.setWidth(""); @@ -172,7 +172,8 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(l); ol.setComponentAlignment(l, Alignment.BOTTOM_LEFT); - Select s = new Select("100% high select, should fit into layout"); + ListSelect s = new ListSelect( + "100% high select, should fit into layout"); s.setMultiSelect(true); s.setHeight("100%"); s.setWidth(""); @@ -182,7 +183,7 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(s); - s = new Select("200 px high select, should be partly outside"); + s = new ListSelect("200 px high select, should be partly outside"); s.setMultiSelect(true); s.setHeight("200px"); s.setWidth(""); @@ -224,7 +225,8 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(l); ol.setComponentAlignment(l, Alignment.BOTTOM_LEFT); - Select s = new Select("100% high select, should fit into layout"); + ListSelect s = new ListSelect( + "100% high select, should fit into layout"); s.setMultiSelect(true); s.setHeight("100%"); s.setWidth("100%"); @@ -234,7 +236,8 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(s); - s = new Select("200 px high select, should make the layout 200px high"); + s = new ListSelect( + "200 px high select, should make the layout 200px high"); s.setMultiSelect(true); s.setHeight("200px"); s.setWidth("100%"); @@ -277,7 +280,8 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(l); ol.setComponentAlignment(l, Alignment.BOTTOM_LEFT); - Select s = new Select("100% high select, should fit into layout"); + ListSelect s = new ListSelect( + "100% high select, should fit into layout"); s.setMultiSelect(true); s.setHeight("100%"); s.setWidth("100%"); @@ -287,7 +291,8 @@ public class OrderedLayoutBasics extends TestBase { ol.addComponent(s); - s = new Select("200 px high select, should make the layout 200px high"); + s = new ListSelect( + "200 px high select, should make the layout 200px high"); s.setMultiSelect(true); s.setHeight("200px"); s.setWidth("100%"); diff --git a/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java b/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java index 7cdbb1300e..872084d436 100644 --- a/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java +++ b/tests/testbench/com/vaadin/tests/tickets/Ticket1230.java @@ -127,7 +127,6 @@ public class Ticket1230 extends Application.LegacyApplication { @SuppressWarnings("deprecation") private Select createSelect() { Select select = new Select(); - select.setMultiSelect(false); select.addContainerProperty(PROPERTY_ID, String.class, ""); select.setItemCaptionPropertyId(PROPERTY_ID); -- 2.39.5