From 447befae430975748f15b47aab99f58ce54854ad Mon Sep 17 00:00:00 2001 From: Anna Koskinen Date: Wed, 9 Jan 2013 10:20:40 +0200 Subject: [PATCH] Merge of (#10471) to Vaadin 7. ComboBox filtering fix. Change-Id: Ia274155c15a5319ec0c98d7bc0347e2f2bd0ddbd --- server/src/com/vaadin/ui/ComboBox.java | 234 +++++++++--------- ...mboBoxSQLContainerFilteredValueChange.html | 42 ++++ ...mboBoxSQLContainerFilteredValueChange.java | 123 +++++++++ 3 files changed, 286 insertions(+), 113 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.html create mode 100644 uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.java diff --git a/server/src/com/vaadin/ui/ComboBox.java b/server/src/com/vaadin/ui/ComboBox.java index 4a82cf27e2..0f42749acd 100644 --- a/server/src/com/vaadin/ui/ComboBox.java +++ b/server/src/com/vaadin/ui/ComboBox.java @@ -82,10 +82,10 @@ public class ComboBox extends AbstractSelect implements private boolean optionRequest; /** - * True if the container is being filtered temporarily and item set change - * notifications should be suppressed. + * True while painting to suppress item set change notifications that could + * be caused by temporary filtering. */ - private boolean filteringContainer; + private boolean isPainting; /** * Flag to indicate whether to scroll the selected item visible (select the @@ -147,141 +147,151 @@ public class ComboBox extends AbstractSelect implements @Override public void paintContent(PaintTarget target) throws PaintException { - if (inputPrompt != null) { - target.addAttribute(ComboBoxConstants.ATTR_INPUTPROMPT, inputPrompt); - } + isPainting = true; + try { + if (inputPrompt != null) { + target.addAttribute(ComboBoxConstants.ATTR_INPUTPROMPT, + inputPrompt); + } - if (!textInputAllowed) { - target.addAttribute(ComboBoxConstants.ATTR_NO_TEXT_INPUT, true); - } + if (!textInputAllowed) { + target.addAttribute(ComboBoxConstants.ATTR_NO_TEXT_INPUT, true); + } - // clear caption change listeners - getCaptionChangeListener().clear(); + // clear caption change listeners + getCaptionChangeListener().clear(); - // The tab ordering number - if (getTabIndex() != 0) { - target.addAttribute("tabindex", getTabIndex()); - } + // The tab ordering number + if (getTabIndex() != 0) { + target.addAttribute("tabindex", getTabIndex()); + } - // If the field is modified, but not committed, set modified attribute - if (isModified()) { - target.addAttribute("modified", true); - } + // If the field is modified, but not committed, set modified + // attribute + if (isModified()) { + target.addAttribute("modified", true); + } - if (isNewItemsAllowed()) { - target.addAttribute("allownewitem", true); - } + if (isNewItemsAllowed()) { + target.addAttribute("allownewitem", true); + } - boolean needNullSelectOption = false; - if (isNullSelectionAllowed()) { - target.addAttribute("nullselect", true); - needNullSelectOption = (getNullSelectionItemId() == null); - if (!needNullSelectOption) { - target.addAttribute("nullselectitem", true); + boolean needNullSelectOption = false; + if (isNullSelectionAllowed()) { + target.addAttribute("nullselect", true); + needNullSelectOption = (getNullSelectionItemId() == null); + if (!needNullSelectOption) { + target.addAttribute("nullselectitem", true); + } } - } - // Constructs selected keys array - String[] selectedKeys = new String[(getValue() == null - && getNullSelectionItemId() == null ? 0 : 1)]; + // Constructs selected keys array + String[] selectedKeys = new String[(getValue() == null + && getNullSelectionItemId() == null ? 0 : 1)]; - target.addAttribute("pagelength", pageLength); + target.addAttribute("pagelength", pageLength); - target.addAttribute("filteringmode", getFilteringMode().toString()); + target.addAttribute("filteringmode", getFilteringMode().toString()); - // Paints the options and create array of selected id keys - int keyIndex = 0; + // Paints the options and create array of selected id keys + int keyIndex = 0; - target.startTag("options"); + target.startTag("options"); - if (currentPage < 0) { - optionRequest = false; - currentPage = 0; - filterstring = ""; - } + if (currentPage < 0) { + optionRequest = false; + currentPage = 0; + filterstring = ""; + } - boolean nullFilteredOut = filterstring != null - && !"".equals(filterstring) - && filteringMode != FilteringMode.OFF; - // null option is needed and not filtered out, even if not on current - // page - boolean nullOptionVisible = needNullSelectOption && !nullFilteredOut; - - // first try if using container filters is possible - List options = getOptionsWithFilter(nullOptionVisible); - if (null == options) { - // not able to use container filters, perform explicit in-memory - // filtering - options = getFilteredOptions(); - filteredSize = options.size(); - options = sanitetizeList(options, nullOptionVisible); - } + boolean nullFilteredOut = filterstring != null + && !"".equals(filterstring) + && filteringMode != FilteringMode.OFF; + // null option is needed and not filtered out, even if not on + // current + // page + boolean nullOptionVisible = needNullSelectOption + && !nullFilteredOut; + + // first try if using container filters is possible + List options = getOptionsWithFilter(nullOptionVisible); + if (null == options) { + // not able to use container filters, perform explicit in-memory + // filtering + options = getFilteredOptions(); + filteredSize = options.size(); + options = sanitetizeList(options, nullOptionVisible); + } - final boolean paintNullSelection = needNullSelectOption - && currentPage == 0 && !nullFilteredOut; + final boolean paintNullSelection = needNullSelectOption + && currentPage == 0 && !nullFilteredOut; - if (paintNullSelection) { - target.startTag("so"); - target.addAttribute("caption", ""); - target.addAttribute("key", ""); - target.endTag("so"); - } + if (paintNullSelection) { + target.startTag("so"); + target.addAttribute("caption", ""); + target.addAttribute("key", ""); + target.endTag("so"); + } - final Iterator i = options.iterator(); - // Paints the available selection options from data source + final Iterator i = options.iterator(); + // Paints the available selection options from data source - while (i.hasNext()) { + while (i.hasNext()) { - final Object id = i.next(); + final Object id = i.next(); - if (!isNullSelectionAllowed() && id != null - && id.equals(getNullSelectionItemId()) && !isSelected(id)) { - continue; - } + if (!isNullSelectionAllowed() && id != null + && id.equals(getNullSelectionItemId()) + && !isSelected(id)) { + continue; + } - // Gets the option attribute values - final String key = itemIdMapper.key(id); - final String caption = getItemCaption(id); - final Resource icon = getItemIcon(id); - getCaptionChangeListener().addNotifierForItem(id); + // Gets the option attribute values + final String key = itemIdMapper.key(id); + final String caption = getItemCaption(id); + final Resource icon = getItemIcon(id); + getCaptionChangeListener().addNotifierForItem(id); - // Paints the option - target.startTag("so"); - if (icon != null) { - target.addAttribute("icon", icon); - } - target.addAttribute("caption", caption); - if (id != null && id.equals(getNullSelectionItemId())) { - target.addAttribute("nullselection", true); - } - target.addAttribute("key", key); - if (isSelected(id) && keyIndex < selectedKeys.length) { - target.addAttribute("selected", true); - selectedKeys[keyIndex++] = key; + // Paints the option + target.startTag("so"); + if (icon != null) { + target.addAttribute("icon", icon); + } + target.addAttribute("caption", caption); + if (id != null && id.equals(getNullSelectionItemId())) { + target.addAttribute("nullselection", true); + } + target.addAttribute("key", key); + if (isSelected(id) && keyIndex < selectedKeys.length) { + target.addAttribute("selected", true); + selectedKeys[keyIndex++] = key; + } + target.endTag("so"); } - target.endTag("so"); - } - target.endTag("options"); + target.endTag("options"); - target.addAttribute("totalitems", size() - + (needNullSelectOption ? 1 : 0)); - if (filteredSize > 0 || nullOptionVisible) { - target.addAttribute("totalMatches", filteredSize - + (nullOptionVisible ? 1 : 0)); - } + target.addAttribute("totalitems", size() + + (needNullSelectOption ? 1 : 0)); + if (filteredSize > 0 || nullOptionVisible) { + target.addAttribute("totalMatches", filteredSize + + (nullOptionVisible ? 1 : 0)); + } - // Paint variables - target.addVariable(this, "selected", selectedKeys); - if (isNewItemsAllowed()) { - target.addVariable(this, "newitem", ""); - } + // Paint variables + target.addVariable(this, "selected", selectedKeys); + if (isNewItemsAllowed()) { + target.addVariable(this, "newitem", ""); + } - target.addVariable(this, "filter", filterstring); - target.addVariable(this, "page", currentPage); + target.addVariable(this, "filter", filterstring); + target.addVariable(this, "page", currentPage); - currentPage = -1; // current page is always set by client + currentPage = -1; // current page is always set by client - optionRequest = true; + optionRequest = true; + } finally { + isPainting = false; + } } @@ -355,7 +365,6 @@ public class ComboBox extends AbstractSelect implements // change events from the underlying container, but the ComboBox does // not process or propagate them based on the flag filteringContainer if (filter != null) { - filteringContainer = true; filterable.addContainerFilter(filter); } @@ -395,7 +404,6 @@ public class ComboBox extends AbstractSelect implements // to the outside, filtering should not be visible if (filter != null) { filterable.removeContainerFilter(filter); - filteringContainer = false; } } } @@ -435,7 +443,7 @@ public class ComboBox extends AbstractSelect implements @Override public void containerItemSetChange(Container.ItemSetChangeEvent event) { - if (!filteringContainer) { + if (!isPainting) { super.containerItemSetChange(event); } } diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.html b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.html new file mode 100644 index 0000000000..f3f44a5d90 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.html @@ -0,0 +1,42 @@ + + + + + + +ComboBoxSQLContainerFilteredValueChange + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
ComboBoxSQLContainerFilteredValueChange
open/run/com.vaadin.tests.components.combobox.ComboBoxSQLContainerFilteredValueChange?restartApplication
enterCharactervaadin=runcomvaadintestscomponentscomboboxComboBoxSQLContainerFilteredValueChange::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VFilterSelect[0]#textboxa
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxSQLContainerFilteredValueChange::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VFilterSelect[0]#textboxdown
pressSpecialKeyvaadin=runcomvaadintestscomponentscomboboxComboBoxSQLContainerFilteredValueChange::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VVerticalLayout[0]/ChildComponentContainer[0]/VFilterSelect[0]#textboxenter
assertTextvaadin=runcomvaadintestscomponentscomboboxComboBoxSQLContainerFilteredValueChange::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VVerticalLayout[0]/ChildComponentContainer[1]/VLabel[0]Selected: 1
+ + diff --git a/uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.java b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.java new file mode 100644 index 0000000000..23a75ae56e --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/combobox/ComboBoxSQLContainerFilteredValueChange.java @@ -0,0 +1,123 @@ +package com.vaadin.tests.components.combobox; + +import java.sql.Connection; +import java.sql.SQLException; +import java.sql.Statement; + +import com.vaadin.data.Property; +import com.vaadin.data.Property.ValueChangeEvent; +import com.vaadin.data.util.sqlcontainer.SQLContainer; +import com.vaadin.data.util.sqlcontainer.connection.JDBCConnectionPool; +import com.vaadin.data.util.sqlcontainer.connection.SimpleJDBCConnectionPool; +import com.vaadin.data.util.sqlcontainer.query.TableQuery; +import com.vaadin.tests.components.TestBase; +import com.vaadin.ui.AbstractSelect.Filtering; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.Label; +import com.vaadin.ui.VerticalLayout; + +public class ComboBoxSQLContainerFilteredValueChange extends TestBase { + + @Override + protected void setup() { + VerticalLayout layout = new VerticalLayout(); + addComponent(layout); + + final ComboBox myCombo = new ComboBox("MyCaption"); + layout.addComponent(myCombo); + + final Label selectedLabel = new Label("Selected: null"); + layout.addComponent(selectedLabel); + + try { + JDBCConnectionPool connectionPool = new SimpleJDBCConnectionPool( + "org.hsqldb.jdbc.JDBCDriver", + "jdbc:hsqldb:mem:sqlcontainer", "SA", "", 2, 20); + + createTestTable(connectionPool); + insertTestData(connectionPool); + + TableQuery q = new TableQuery("mytable", connectionPool); + q.setVersionColumn("version"); + SQLContainer myContainer = new SQLContainer(q); + + myCombo.setContainerDataSource(myContainer); + + } catch (SQLException e) { + e.printStackTrace(); + } + + myCombo.setItemCaptionPropertyId("MYFIELD"); + myCombo.setFilteringMode(Filtering.FILTERINGMODE_CONTAINS); + myCombo.setImmediate(true); + myCombo.setWidth("100.0%"); + myCombo.setHeight("-1px"); + myCombo.addListener(new Property.ValueChangeListener() { + public void valueChange(ValueChangeEvent event) { + selectedLabel.setValue("Selected: " + + event.getProperty().getValue()); + } + }); + } + + @Override + protected String getDescription() { + return "Selecting the first filtered item should change the value of the label under the ComboBox to 'Selected: 1'."; + } + + @Override + protected Integer getTicketNumber() { + return 10471; + } + + /** + * (Re)creates the test table + * @param connectionPool + */ + private void createTestTable(JDBCConnectionPool connectionPool) { + Connection conn = null; + try { + conn = connectionPool.reserveConnection(); + Statement statement = conn.createStatement(); + try { + statement.executeUpdate("DROP TABLE mytable"); + } catch (SQLException e) { + } + statement.execute("CREATE TABLE mytable " + + "(id INTEGER GENERATED BY DEFAULT AS IDENTITY, " + + "MYFIELD VARCHAR(45), " + "PRIMARY KEY(ID))"); + statement.close(); + conn.commit(); + } catch (SQLException e) { + e.printStackTrace(); + } finally { + connectionPool.releaseConnection(conn); + } + } + + /** + * Adds test data to the test table + * @param connectionPool + * @throws SQLException + */ + private void insertTestData(JDBCConnectionPool connectionPool) + throws SQLException { + Connection conn = null; + try { + conn = connectionPool.reserveConnection(); + Statement statement = conn.createStatement(); + + statement.executeUpdate("INSERT INTO mytable VALUES(1, 'A0')"); + statement.executeUpdate("INSERT INTO mytable VALUES(2, 'A1')"); + statement.executeUpdate("INSERT INTO mytable VALUES(3, 'B0')"); + statement.executeUpdate("INSERT INTO mytable VALUES(4, 'B1')"); + + statement.close(); + conn.commit(); + } catch (SQLException e) { + e.printStackTrace(); + } finally { + connectionPool.releaseConnection(conn); + } + } +} \ No newline at end of file -- 2.39.5