diff --git a/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/main/java/com/vaadin/flow/component/combobox/test/ClearValuePage.java b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/main/java/com/vaadin/flow/component/combobox/test/ClearValuePage.java index fc190c92fb9..e8f06d133b4 100644 --- a/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/main/java/com/vaadin/flow/component/combobox/test/ClearValuePage.java +++ b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/main/java/com/vaadin/flow/component/combobox/test/ClearValuePage.java @@ -15,73 +15,99 @@ */ package com.vaadin.flow.component.combobox.test; +import java.util.List; +import java.util.Arrays; + +import com.vaadin.flow.component.Component; import com.vaadin.flow.component.combobox.ComboBox; import com.vaadin.flow.component.html.Div; +import com.vaadin.flow.component.html.H2; import com.vaadin.flow.component.html.NativeButton; import com.vaadin.flow.component.html.Paragraph; +import com.vaadin.flow.component.orderedlayout.VerticalLayout; import com.vaadin.flow.router.Route; @Route("vaadin-combo-box/clear-value") public class ClearValuePage extends Div { static final String INITIAL_VALUE = "two"; + static final List ITEMS = Arrays.asList("one", "two", "three"); - static final String COMBO_BOX_ID = "comboBox"; - static final String BUTTON_SET_NULL_ID = "buttonSetNull"; - static final String BUTTON_CLEAR_ID = "buttonClear"; + static final String COMBO_BOX = "combo-box"; + static final String COMBO_BOX_CLEAR_BUTTON = "combo-box-clear-button"; + static final String COMBO_BOX_SET_NULL_VALUE_BUTTON = "combo-box-set-null-value-button"; - static final String COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_ID = "comboBoxWithAllowCustomValue"; - static final String BUTTON_CUSTOM_VALUE_SET_NULL_ID = "buttonSetNullCustom"; - static final String BUTTON_CUSTOM_VALUE_CLEAR_ID = "buttonClearCustom"; + static final String COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE = "combo-box-with-allow-custom-value"; + static final String COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_CLEAR_BUTTON = "combo-box-with-allow-custom-value-clear-button"; + static final String COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_SET_NULL_VALUE_BUTTON = "combo-box-with-allow-custom-value-set-null-value-button"; - static final String COMBO_BOX_WITH_CLEAR_BUTTON_ID = "comboBoxWithClearButton"; + static final String COMBO_BOX_WITH_CLEAR_BUTTON = "combo-box-with-clear-button"; + static final String COMBO_BOX_WITH_CLEAR_BUTTON_VALUE_MESSAGES = "combo-box-with-clear-button-value-messages"; public ClearValuePage() { - ComboBox comboBox = new ComboBox<>("Ordinary combo box", "one", - INITIAL_VALUE, "three"); + createComboBox(); + createComboBoxWithClearButton(); + createComboBoxWithAllowCustomValue(); + } + + private void createComboBox() { + ComboBox comboBox = new ComboBox<>("Choose option:", ITEMS); comboBox.setValue(INITIAL_VALUE); - comboBox.setId(COMBO_BOX_ID); + comboBox.setId(COMBO_BOX); - NativeButton setNull = new NativeButton( - "Set null to ordinary combo box", + NativeButton setNullValueButton = new NativeButton("Set null value", event -> comboBox.setValue(null)); - setNull.setId(BUTTON_SET_NULL_ID); + setNullValueButton.setId(COMBO_BOX_SET_NULL_VALUE_BUTTON); - NativeButton clear = new NativeButton("Clear ordinary combo box", + NativeButton clearButton = new NativeButton("Clear", event -> comboBox.clear()); - clear.setId(BUTTON_CLEAR_ID); - - ComboBox comboBoxWithAllowCustomValue = new ComboBox<>( - "Combo box with custom value", "one", INITIAL_VALUE, "three"); - comboBoxWithAllowCustomValue.setAllowCustomValue(true); - comboBoxWithAllowCustomValue.setValue(INITIAL_VALUE); - comboBoxWithAllowCustomValue - .setId(COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_ID); - - NativeButton setNullCustom = new NativeButton( - "Set null to combo box with custom value", - event -> comboBoxWithAllowCustomValue.setValue(null)); - setNullCustom.setId(BUTTON_CUSTOM_VALUE_SET_NULL_ID); + clearButton.setId(COMBO_BOX_CLEAR_BUTTON); - NativeButton clearCustom = new NativeButton( - "Clear combo box with custom value", - event -> comboBoxWithAllowCustomValue.clear()); - clearCustom.setId(BUTTON_CUSTOM_VALUE_CLEAR_ID); + addCard("ComboBox", comboBox, setNullValueButton, clearButton); + } + private void createComboBoxWithClearButton() { Div valueMessages = new Div(); - valueMessages.setId("value-messages"); + valueMessages.setId(COMBO_BOX_WITH_CLEAR_BUTTON_VALUE_MESSAGES); - ComboBox comboBoxWithClearButton = new ComboBox<>( - "Combo box with clear button", "one", INITIAL_VALUE, "three"); - comboBoxWithClearButton.setValue(INITIAL_VALUE); - comboBoxWithClearButton.setClearButtonVisible(true); - comboBoxWithClearButton.setId(COMBO_BOX_WITH_CLEAR_BUTTON_ID); + ComboBox comboBox = new ComboBox<>("Choose option:", ITEMS); + comboBox.setValue(INITIAL_VALUE); + comboBox.setClearButtonVisible(true); + comboBox.setId(COMBO_BOX_WITH_CLEAR_BUTTON); - comboBoxWithClearButton.addValueChangeListener(e -> valueMessages.add( + comboBox.addValueChangeListener(e -> valueMessages.add( new Paragraph(e.getValue() == null ? "null" : e.getValue()))); - add(new Div(comboBox, setNull, clear), - new Div(comboBoxWithAllowCustomValue, setNullCustom, - clearCustom), - new Div(comboBoxWithClearButton, valueMessages)); + addCard("ComboBox with clear button", comboBox, valueMessages); + } + + private void createComboBoxWithAllowCustomValue() { + ComboBox comboBox = new ComboBox<>("Choose option:", ITEMS); + comboBox.setAllowCustomValue(true); + comboBox.setValue(INITIAL_VALUE); + comboBox.setId(COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE); + comboBox.addCustomValueSetListener(event -> { + if (event.getDetail().equals("NotAcceptableCustomValue")) { + comboBox.clear(); + } + }); + + NativeButton setNullValueButton = new NativeButton("Set null value", + event -> comboBox.setValue(null)); + setNullValueButton + .setId(COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_SET_NULL_VALUE_BUTTON); + + NativeButton clearButton = new NativeButton("Clear", + event -> comboBox.clear()); + clearButton.setId(COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_CLEAR_BUTTON); + + addCard("ComboBox with custom values", comboBox, setNullValueButton, + clearButton); + } + + private void addCard(String title, Component... components) { + VerticalLayout layout = new VerticalLayout(); + layout.add(new H2(title)); + layout.add(components); + add(layout); } } diff --git a/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/ClearValueIT.java b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/ClearValueIT.java index 5757dc5a082..d28d368c899 100644 --- a/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/ClearValueIT.java +++ b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow-integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/ClearValueIT.java @@ -38,27 +38,27 @@ public void init() { @Test public void valueIsCorrectlyCleared() { - checkEmptyValue(ClearValuePage.COMBO_BOX_ID, - ClearValuePage.BUTTON_CLEAR_ID, false); + checkEmptyValue(ClearValuePage.COMBO_BOX, + ClearValuePage.COMBO_BOX_CLEAR_BUTTON, false); } @Test public void valueIsCorrectlyClearedWithClearButtonBeforeOpened() { - String comboBoxId = ClearValuePage.COMBO_BOX_WITH_CLEAR_BUTTON_ID; - ComboBoxElement comboBox = $(ComboBoxElement.class).id(comboBoxId); - Assert.assertEquals(String.format( - "Unexpected selected item label for combo box with id '%s'", - comboBoxId), ClearValuePage.INITIAL_VALUE, - comboBox.getSelectedText()); + ComboBoxElement comboBox = $(ComboBoxElement.class) + .id(ClearValuePage.COMBO_BOX_WITH_CLEAR_BUTTON); + Assert.assertEquals( + "Unexpected selected item label for combo box with clear button", + ClearValuePage.INITIAL_VALUE, comboBox.getSelectedText()); comboBox.$("vaadin-text-field").get(0).$("[part~='clear-button']") .get(0).click(); - Assert.assertEquals(String.format( - "Combo box with id '%s' should have its value empty after the test", - comboBoxId), "null", - $(TestBenchElement.class).id("value-messages").$("p").first() - .getText()); + Assert.assertEquals( + "Combo box with clear button should have its value empty after the test", + "null", + $(TestBenchElement.class).id( + ClearValuePage.COMBO_BOX_WITH_CLEAR_BUTTON_VALUE_MESSAGES) + .$("p").first().getText()); } @Test @@ -66,14 +66,15 @@ public void valueIsCorrectlySetToNull() { Assert.assertNull( "Combobox empty value is not null, add clear tests also", new ComboBox<>().getEmptyValue()); - checkEmptyValue(ClearValuePage.COMBO_BOX_ID, - ClearValuePage.BUTTON_SET_NULL_ID, false); + checkEmptyValue(ClearValuePage.COMBO_BOX, + ClearValuePage.COMBO_BOX_SET_NULL_VALUE_BUTTON, false); } @Test public void allowCustomValue_setInitialValue_valueIsCorrectlyCleared() { - checkEmptyValue(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_ID, - ClearValuePage.BUTTON_CUSTOM_VALUE_CLEAR_ID, true); + checkEmptyValue(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE, + ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_CLEAR_BUTTON, + true); } @Test @@ -81,16 +82,17 @@ public void allowCustomValue_setInitialValue_valueIsCorrectlySetToNull() { Assert.assertNull( "Combobox empty value is not null, add clear tests also", new ComboBox<>().getEmptyValue()); - checkEmptyValue(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_ID, - ClearValuePage.BUTTON_CUSTOM_VALUE_SET_NULL_ID, true); + checkEmptyValue(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE, + ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_SET_NULL_VALUE_BUTTON, + true); } @Test public void allowCustomValue_enterCustomValue_clearValue_inputElementValueIsCleared() { ComboBoxElement comboBox = $(ComboBoxElement.class) - .id(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_ID); - TestBenchElement clearButton = $("button") - .id(ClearValuePage.BUTTON_CUSTOM_VALUE_CLEAR_ID); + .id(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE); + TestBenchElement clearButton = $("button").id( + ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_CLEAR_BUTTON); // Clear initial value to set the state of the input element value // property to an empty value @@ -109,13 +111,13 @@ public void allowCustomValue_enterCustomValue_clearValue_inputElementValueIsClea @Test public void allowCustomValue_enterCustomValue_setNullValue_inputElementValueIsCleared() { ComboBoxElement comboBox = $(ComboBoxElement.class) - .id(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_ID); - TestBenchElement setNullButton = $("button") - .id(ClearValuePage.BUTTON_CUSTOM_VALUE_SET_NULL_ID); + .id(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE); + TestBenchElement setNullValueButton = $("button").id( + ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_SET_NULL_VALUE_BUTTON); // Set null value to set the state of the input element value property // to an empty value - setNullButton.click(); + setNullValueButton.click(); Assert.assertEquals("", comboBox.getInputElementValue()); // Enter custom value @@ -123,7 +125,26 @@ public void allowCustomValue_enterCustomValue_setNullValue_inputElementValueIsCl Assert.assertEquals("foo", comboBox.getInputElementValue()); // Set null value - setNullButton.click(); + setNullValueButton.click(); + Assert.assertEquals("", comboBox.getInputElementValue()); + } + + @Test + public void allowCustomValue_enterNotAcceptableCustomValue_inputElementValueIsCleared() { + ComboBoxElement comboBox = $(ComboBoxElement.class) + .id(ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE); + + TestBenchElement clearButton = $("button").id( + ClearValuePage.COMBO_BOX_WITH_ALLOW_CUSTOM_VALUE_CLEAR_BUTTON); + + // Clear initial value to set the state of the input element value + // property to an empty value + clearButton.click(); + Assert.assertEquals("", comboBox.getInputElementValue()); + + // Enter a custom value that is cleared in the `customValueSet` + // listener. + comboBox.sendKeys("NotAcceptableCustomValue", Keys.ENTER); Assert.assertEquals("", comboBox.getInputElementValue()); } diff --git a/vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java index 51dd50df6be..a2e36644052 100644 --- a/vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java +++ b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java @@ -93,7 +93,6 @@ public class ComboBox extends GeneratedVaadinComboBox, T> implements HasSize, HasValidation, HasFilterableDataProvider, HasHelper { - private static final String PROP_INPUT_ELEMENT_VALUE = "_inputElementValue"; private static final String PROP_SELECTED_ITEM = "selectedItem"; private static final String PROP_VALUE = "value"; private static final String PROP_AUTO_OPEN_DISABLED = "autoOpenDisabled"; @@ -261,16 +260,6 @@ public ComboBox(int pageSize) { setItems(); } }); - - // Synchronize input element value property state when setting a custom - // value. This is necessary to allow clearing the input value in - // `ComboBox.refreshValue`. If the input element value is not - // synchronized here, then setting the property to an empty value would - // not trigger a client update. Need to use `super` here, in order to - // avoid enabling custom values, which is a side effect of - // `ComboBox.addCustomValueSetListener`. - super.addCustomValueSetListener(e -> this.getElement() - .setProperty(PROP_INPUT_ELEMENT_VALUE, e.getDetail())); } /** @@ -367,7 +356,12 @@ private void refreshValue() { if (value == null) { getElement().setProperty(PROP_SELECTED_ITEM, null); getElement().setProperty(PROP_VALUE, ""); - getElement().setProperty(PROP_INPUT_ELEMENT_VALUE, ""); + // Force _inputElementValue update on the client-side by using + // `executeJs` to ensure the input's value will be cleared even + // if the component's value hasn't changed. The latter can be + // the case when calling `clear()` in a `customValueSet` listener + // which is triggered before any value is committed. + getElement().executeJs("this._inputElementValue = $0", ""); return; } @@ -378,7 +372,7 @@ private void refreshValue() { dataGenerator.generateData(value, json); setSelectedItem(json); getElement().setProperty(PROP_VALUE, keyMapper.key(value)); - getElement().setProperty(PROP_INPUT_ELEMENT_VALUE, + getElement().executeJs("this._inputElementValue = $0", generateLabel(value)); }