Skip to content

Commit

Permalink
fix: force refreshValue to update input value on the client-side (#3712…
Browse files Browse the repository at this point in the history 
…) (#3733)
  • Loading branch information
@vursen
vursen authored Sep 16, 2022
1 parent b5c442a commit f082318
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 82 deletions.
110 changes: 68 additions & 42 deletions ...tegration-tests/src/main/java/com/vaadin/flow/component/combobox/test/ClearValuePage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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<String> comboBox = new ComboBox<>("Ordinary combo box", "one",
INITIAL_VALUE, "three");
createComboBox();
createComboBoxWithClearButton();
createComboBoxWithAllowCustomValue();
}

private void createComboBox() {
ComboBox<String> 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<String> 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<String> 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<String> 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<String> 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);
}
}
75 changes: 48 additions & 27 deletions ...integration-tests/src/test/java/com/vaadin/flow/component/combobox/test/ClearValueIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,59 +38,61 @@ 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
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
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
Expand All @@ -109,21 +111,40 @@ 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
comboBox.sendKeys("foo", Keys.ENTER);
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());
}

Expand Down
20 changes: 7 additions & 13 deletions ...rent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBox.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ public class ComboBox<T> extends GeneratedVaadinComboBox<ComboBox<T>, T>
implements HasSize, HasValidation, HasFilterableDataProvider<T, String>,
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";
Expand Down Expand Up @@ -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()));
}

/**
Expand Down Expand Up @@ -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;
}

Expand All @@ -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));
}

Expand Down

0 comments on commit f082318

Please sign in to comment.