Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Commit

Permalink
Don't set value if Element has property "value"
Browse files Browse the repository at this point in the history 
vaadin/vaadin-combo-box#394

Don't set value if Element has property "value"

Fixes: 


Flow-component: vaadin-combo-box
  • Loading branch information
Denis committed Sep 25, 2020
1 parent c081f4c commit 00eea31
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 23 deletions.
36 changes: 19 additions & 17 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 @@ -242,7 +242,7 @@ private enum UserProvidedFilter {
*/
public ComboBox(int pageSize) {
super(null, null, String.class, ComboBox::presentationToModel,
ComboBox::modelToPresentation);
ComboBox::modelToPresentation, true);
dataGenerator.addDataGenerator((item, jsonObject) -> jsonObject
.put("label", generateLabel(item)));

Expand Down Expand Up @@ -515,7 +515,7 @@ public <C> void setDataProvider(DataProvider<T, C> dataProvider,
if (dataCommunicator == null) {
dataCommunicator = new DataCommunicator<>(dataGenerator,
arrayUpdater, data -> getElement()
.callJsFunction("$connector.updateData", data),
.callJsFunction("$connector.updateData", data),
getElement().getNode());
dataCommunicator.setPageSize(getPageSize());
}
Expand All @@ -534,7 +534,7 @@ public <C> void setDataProvider(DataProvider<T, C> dataProvider,
// trigger item count request and data fetch upon clicking on combobox
dataCommunicatorInitializer = () -> {
dataCommunicatorInitializer = null;
if(lazyOpenRegistration != null) {
if (lazyOpenRegistration != null) {
lazyOpenRegistration.remove();
lazyOpenRegistration = null;
}
Expand All @@ -560,16 +560,17 @@ public <C> void setDataProvider(DataProvider<T, C> dataProvider,

// Register an opened listener to initialize the dataprovider
// when the dropdown opens.
lazyOpenRegistration = getElement()
.addPropertyChangeListener("opened", this::executeRegistration);
lazyOpenRegistration = getElement().addPropertyChangeListener("opened",
this::executeRegistration);
}

/**
* Initialize {@link DataCommunicator} with the lazy {@link DataProvider}
* when the open property changes for a lazy combobox. Clean registration
* on initialization.
* when the open property changes for a lazy combobox. Clean registration on
* initialization.
*
* @param event property change event for "open"
* @param event
* property change event for "open"
*/
private void executeRegistration(PropertyChangeEvent event) {
if (event.getValue().equals(Boolean.TRUE)) {
Expand All @@ -584,7 +585,8 @@ private void executeRegistration(PropertyChangeEvent event) {
}
}

private <C> void setupDataProviderListener(DataProvider<T, C> dataProvider) {
private <C> void setupDataProviderListener(
DataProvider<T, C> dataProvider) {
if (dataProviderListener != null) {
dataProviderListener.remove();
}
Expand Down Expand Up @@ -660,7 +662,8 @@ public void setDataProvider(ListDataProvider<T> listDataProvider) {
* size callback.
* <p>
* This method is a shorthand for making a {@link CallbackDataProvider} that
* handles a partial {@link com.vaadin.flow.data.provider.Query Query} object.
* handles a partial {@link com.vaadin.flow.data.provider.Query Query}
* object.
* <p>
* Changing the combo box's data provider resets its current value to
* {@code null}.
Expand Down Expand Up @@ -1165,17 +1168,16 @@ private DataCommunicator<T> getDataCommunicator() {
private void initDataCommunicator() {
if (dataCommunicatorInitializer != null) {
/*
* Init the Data Communicator: 1. Lazily, when the data lazy
* loading is used. Initialization occurs when the user clicks
* on the dropdown to view the list of items. 2. Eagerly, when
* the items are set explicitly or in-memory Data Provider is
* used.
* Init the Data Communicator: 1. Lazily, when the data lazy loading
* is used. Initialization occurs when the user clicks on the
* dropdown to view the list of items. 2. Eagerly, when the items
* are set explicitly or in-memory Data Provider is used.
*/
dataCommunicatorInitializer.init();
} else if (dataCommunicator == null) {
/*
* If the user hasn't provided any data, initialize with empty
* data set.
* If the user hasn't provided any data, initialize with empty data
* set.
*/
setItems();
}
Expand Down
41 changes: 36 additions & 5 deletions ...bo-box-flow/src/main/java/com/vaadin/flow/component/combobox/GeneratedVaadinComboBox.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1425,7 +1425,6 @@ public <P> GeneratedVaadinComboBox(T initialValue, T defaultValue,
super("value", defaultValue, elementPropertyType, presentationToModel,
modelToPresentation);
if (initialValue != null) {
setModelValue(initialValue, false);
setPresentationValue(initialValue);
}
}
Expand All @@ -1445,7 +1444,6 @@ public GeneratedVaadinComboBox(T initialValue, T defaultValue,
boolean acceptNullValues) {
super("value", defaultValue, acceptNullValues);
if (initialValue != null) {
setModelValue(initialValue, false);
setPresentationValue(initialValue);
}
}
Expand All @@ -1466,21 +1464,54 @@ public GeneratedVaadinComboBox(T initialValue, T defaultValue,
* @param modelToPresentation
* a function that accepts this component and a model value and
* returns a property value
* @param isInitialValueOptional
* if {@code isInitialValueOptional} is {@code true} then the
* initial value is used only if element has no {@code "value"}
* property value, otherwise element {@code "value"} property is
* ignored and the initial value is set
* @param <P>
* the property type
*/
public <P> GeneratedVaadinComboBox(T initialValue, T defaultValue,
Class<P> elementPropertyType,
SerializableBiFunction<R, P, T> presentationToModel,
SerializableBiFunction<R, T, P> modelToPresentation) {
SerializableBiFunction<R, T, P> modelToPresentation,
boolean isInitialValueOptional) {
super("value", defaultValue, elementPropertyType, presentationToModel,
modelToPresentation);
if (initialValue != null) {
setModelValue(initialValue, false);
if ((getElement().getProperty("value") == null
|| !isInitialValueOptional) && initialValue != null) {
setPresentationValue(initialValue);
}
}

/**
* Constructs a new GeneratedVaadinComboBox component with the given
* arguments.
*
* @param initialValue
* the initial value to set to the value
* @param defaultValue
* the default value to use if the value isn't defined
* @param elementPropertyType
* the type of the element property
* @param presentationToModel
* a function that accepts this component and a property value
* and returns a model value
* @param modelToPresentation
* a function that accepts this component and a model value and
* returns a property value
* @param <P>
* the property type
*/
public <P> GeneratedVaadinComboBox(T initialValue, T defaultValue,
Class<P> elementPropertyType,
SerializableBiFunction<R, P, T> presentationToModel,
SerializableBiFunction<R, T, P> modelToPresentation) {
this(initialValue, defaultValue, elementPropertyType,
presentationToModel, modelToPresentation, false);
}

/**
* Default constructor.
*/
Expand Down
41 changes: 40 additions & 1 deletion .../vaadin-combo-box-flow/src/test/java/com/vaadin/flow/component/combobox/ComboBoxTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package com.vaadin.flow.component.combobox;

import static org.junit.Assert.assertEquals;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand All @@ -24,15 +26,21 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.mockito.Mockito;

import com.vaadin.flow.component.Component;
import com.vaadin.flow.component.Focusable;
import com.vaadin.flow.component.UI;
import com.vaadin.flow.data.binder.Binder;
import com.vaadin.flow.data.provider.DataProvider;
import com.vaadin.flow.data.provider.ListDataProvider;
import com.vaadin.flow.di.Instantiator;
import com.vaadin.flow.dom.Element;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.shared.Registration;

import elemental.json.Json;
import static org.junit.Assert.assertEquals;

public class ComboBoxTest {

Expand Down Expand Up @@ -67,6 +75,14 @@ public void setCategory(Category category) {
}
}

private class ComboBoxWithInitialValue
extends GeneratedVaadinComboBox<ComboBoxWithInitialValue, String> {
ComboBoxWithInitialValue() {
super("", null, String.class, (combo, value) -> value,
(combo, value) -> value, true);
}
}

@Test
public void setItems_jsonItemsAreSet() {
TestComboBox comboBox = new TestComboBox();
Expand Down Expand Up @@ -290,6 +306,29 @@ public void setClearButtonVisible_isClearButtonVisible() {
combo.isClearButtonVisible());
}

@Test
public void elementHasValue_wrapIntoField_propertyIsNotSetToInitialValue() {
Element element = new Element("vaadin-combo-box");
element.setProperty("value", "foo");
UI ui = new UI();
UI.setCurrent(ui);
VaadinSession session = Mockito.mock(VaadinSession.class);
ui.getInternals().setSession(session);
VaadinService service = Mockito.mock(VaadinService.class);
Mockito.when(session.getService()).thenReturn(service);

Instantiator instantiator = Mockito.mock(Instantiator.class);

Mockito.when(service.getInstantiator()).thenReturn(instantiator);

Mockito.when(
instantiator.createComponent(ComboBoxWithInitialValue.class))
.thenAnswer(invocation -> new ComboBoxWithInitialValue());
ComboBoxWithInitialValue field = Component.from(element,
ComboBoxWithInitialValue.class);
Assert.assertEquals("foo", field.getElement().getPropertyRaw("value"));
}

private void assertItem(TestComboBox comboBox, int index, String caption) {
String value1 = comboBox.items.get(index);
Assert.assertEquals(caption, value1);
Expand Down

0 comments on commit 00eea31

Please sign in to comment.