diff options
9 files changed, 266 insertions, 33 deletions
diff --git a/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java b/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java index 7cfaf7572e..1386005351 100644 --- a/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java +++ b/client/src/main/java/com/vaadin/client/ui/VRadioButtonGroup.java @@ -227,9 +227,13 @@ public class VRadioButtonGroup extends FocusableFlowPanelComposite } public void selectItemKey(String selectedItemKey) { - RadioButton radioButton = keyToOptions.get(selectedItemKey); - if (radioButton != null) {// Items might not be loaded yet - radioButton.setValue(true); + if (selectedItemKey != null) { + RadioButton radioButton = keyToOptions.get(selectedItemKey); + if (radioButton != null) { // Items might not be loaded yet + radioButton.setValue(true); + } + } else { + keyToOptions.values().forEach(button -> button.setValue(false)); } } } diff --git a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java index 2b0e6b7d01..f9eed7fef3 100644 --- a/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java +++ b/server/src/main/java/com/vaadin/ui/AbstractSingleSelect.java @@ -230,10 +230,8 @@ public abstract class AbstractSingleSelect<T> extends AbstractListing<T> T oldSelection = getSelectedItem().orElse(getEmptyValue()); doSetSelectedKey(key); - // Update diffstate so that a change will be sent to the client if the - // selection is changed to its original value - updateDiffstate("selectedItemKey", - key == null ? Json.createNull() : Json.create(key)); + // Set diffstate to something that will always send selection to client + updateDiffstate("selectedItemKey", Json.createObject()); fireEvent(new SingleSelectionEvent<>(AbstractSingleSelect.this, oldSelection, true)); diff --git a/server/src/main/java/com/vaadin/ui/ComboBox.java b/server/src/main/java/com/vaadin/ui/ComboBox.java index 73d9ad84cf..af100e03f1 100644 --- a/server/src/main/java/com/vaadin/ui/ComboBox.java +++ b/server/src/main/java/com/vaadin/ui/ComboBox.java @@ -25,11 +25,11 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Stream; -import com.vaadin.data.ValueProvider; import org.jsoup.nodes.Element; import com.vaadin.data.HasFilterableDataProvider; import com.vaadin.data.HasValue; +import com.vaadin.data.ValueProvider; import com.vaadin.data.provider.CallbackDataProvider; import com.vaadin.data.provider.DataCommunicator; import com.vaadin.data.provider.DataKeyMapper; @@ -57,7 +57,6 @@ import com.vaadin.ui.declarative.DesignAttributeHandler; import com.vaadin.ui.declarative.DesignContext; import com.vaadin.ui.declarative.DesignFormatter; -import elemental.json.Json; import elemental.json.JsonObject; /** @@ -189,7 +188,8 @@ public class ComboBox<T> extends AbstractSingleSelect<T> public ComboBox() { super(new DataCommunicator<T>() { @Override - protected DataKeyMapper<T> createKeyMapper(ValueProvider<T,Object> identifierGetter) { + protected DataKeyMapper<T> createKeyMapper( + ValueProvider<T, Object> identifierGetter) { return new KeyMapper<T>(identifierGetter) { @Override public void remove(T removeobj) { @@ -831,23 +831,6 @@ public class ComboBox<T> extends AbstractSingleSelect<T> q -> sizeCallback.applyAsInt(q.getFilter().orElse("")))); } - @Override - protected void setSelectedFromClient(String key) { - super.setSelectedFromClient(key); - - /* - * The client side for combo box always expects a state change for - * selectedItemKey after it has sent a selection change. This means that - * we must store a value in the diffstate that guarantees that a new - * value will be sent, regardless of what the value actually is at the - * time when changes are sent. - * - * Keys are always strings (or null), so using a non-string type will - * always trigger a diff mismatch and a resend. - */ - updateDiffstate("selectedItemKey", Json.create(0)); - } - /** * Predicate to check {@link ComboBox} item captions against user typed * strings. diff --git a/testbench-api/src/main/java/com/vaadin/testbench/elements/AbstractSingleSelectElement.java b/testbench-api/src/main/java/com/vaadin/testbench/elements/AbstractSingleSelectElement.java new file mode 100644 index 0000000000..c66e736a1d --- /dev/null +++ b/testbench-api/src/main/java/com/vaadin/testbench/elements/AbstractSingleSelectElement.java @@ -0,0 +1,47 @@ +/* + * Copyright 2000-2016 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.testbench.elements; + +import com.vaadin.testbench.elementsbase.ServerClass; + +/** + * A common base element class for all single select components. + * + * @since 8.1.1 + */ +@ServerClass("com.vaadin.ui.AbstractSingleSelect") +public abstract class AbstractSingleSelectElement + extends AbstractSelectElement { + + /** + * Selects the first option in this single select component that matches the + * given text. + * + * @param text + * the text to select by + */ + public abstract void selectByText(String text); + + /** + * Return value of this single select component. + * <p> + * <strong>Note:</strong> If there is no value selected the behavior of + * subclasses varies. Pay attention on the actual implementation. + * + * @return the value + */ + public abstract String getValue(); +} diff --git a/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java b/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java index 11b823e656..2eaef4f015 100644 --- a/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java +++ b/testbench-api/src/main/java/com/vaadin/testbench/elements/ComboBoxElement.java @@ -28,7 +28,7 @@ import com.vaadin.testbench.TestBenchElement; import com.vaadin.testbench.elementsbase.ServerClass; @ServerClass("com.vaadin.ui.ComboBox") -public class ComboBoxElement extends AbstractSelectElement { +public class ComboBoxElement extends AbstractSingleSelectElement { private static org.openqa.selenium.By bySuggestionPopup = By .vaadin("#popup"); @@ -199,7 +199,7 @@ public class ComboBoxElement extends AbstractSelectElement { } /** - * Return value of the combo box element + * Return value of the combo box element. * * @return value of the combo box element */ diff --git a/testbench-api/src/main/java/com/vaadin/testbench/elements/NativeSelectElement.java b/testbench-api/src/main/java/com/vaadin/testbench/elements/NativeSelectElement.java index d932af1966..f6b0426c3e 100644 --- a/testbench-api/src/main/java/com/vaadin/testbench/elements/NativeSelectElement.java +++ b/testbench-api/src/main/java/com/vaadin/testbench/elements/NativeSelectElement.java @@ -17,6 +17,7 @@ package com.vaadin.testbench.elements; import java.util.List; +import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.WebElement; import org.openqa.selenium.support.ui.Select; @@ -25,7 +26,7 @@ import com.vaadin.testbench.TestBenchElement; import com.vaadin.testbench.elementsbase.ServerClass; @ServerClass("com.vaadin.ui.NativeSelect") -public class NativeSelectElement extends AbstractSelectElement { +public class NativeSelectElement extends AbstractSingleSelectElement { private Select select; private WebElement selectElement; @@ -70,9 +71,13 @@ public class NativeSelectElement extends AbstractSelectElement { /** * Return value of the selected item in the native select element. * - * @return value of the selected item in the native select element + * @return value of the selected item + * + * @throws NoSuchElementException + * if no value is selected */ - public String getValue() { + + public String getValue() throws NoSuchElementException { return select.getFirstSelectedOption().getText(); } diff --git a/testbench-api/src/main/java/com/vaadin/testbench/elements/RadioButtonGroupElement.java b/testbench-api/src/main/java/com/vaadin/testbench/elements/RadioButtonGroupElement.java index 4feeaae6ff..4cff189ea7 100644 --- a/testbench-api/src/main/java/com/vaadin/testbench/elements/RadioButtonGroupElement.java +++ b/testbench-api/src/main/java/com/vaadin/testbench/elements/RadioButtonGroupElement.java @@ -25,7 +25,7 @@ import com.vaadin.testbench.TestBenchElement; import com.vaadin.testbench.elementsbase.ServerClass; @ServerClass("com.vaadin.ui.RadioButtonGroup") -public class RadioButtonGroupElement extends AbstractSelectElement { +public class RadioButtonGroupElement extends AbstractSingleSelectElement { private static org.openqa.selenium.By bySelectOption = By .className("v-select-option"); diff --git a/uitest/src/main/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelection.java b/uitest/src/main/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelection.java new file mode 100644 index 0000000000..55f61b4134 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelection.java @@ -0,0 +1,68 @@ +package com.vaadin.tests.components.abstractsingleselect; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.data.provider.Query; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.AbstractSingleSelect; +import com.vaadin.ui.Button; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.NativeSelect; +import com.vaadin.ui.RadioButtonGroup; + +@Widgetset("com.vaadin.DefaultWidgetSet") +@SuppressWarnings({ "unchecked", "rawtypes" }) +public class AbstractSingleSelection extends AbstractTestUI { + + /* Initial placeholder component */ + AbstractSingleSelect<String> component = new ComboBox<>(); + + @Override + protected void setup(VaadinRequest request) { + NativeSelect<Class<? extends AbstractSingleSelect>> componentSelect = new NativeSelect<>(); + componentSelect.setItems(RadioButtonGroup.class, NativeSelect.class, + ComboBox.class); + componentSelect.setItemCaptionGenerator(Class::getSimpleName); + + componentSelect.setEmptySelectionAllowed(false); + componentSelect.addValueChangeListener(singleSelectClass -> { + createComponent(singleSelectClass.getValue()); + }); + + addComponent(componentSelect); + addComponent(component); // This will be replaced in createComponent + addComponent( + new Button("Deselect", e -> component.setSelectedItem(null))); + addComponent(new Button("Select Bar", + e -> component.setSelectedItem("Bar"))); + addComponent(new Button("Refresh", + e -> component.getDataProvider().refreshAll())); + + // Select a value from native select to create the initial component + componentSelect.getDataProvider().fetch(new Query<>()).findFirst() + .ifPresent(componentSelect::setSelectedItem); + } + + private void createComponent( + Class<? extends AbstractSingleSelect> singleSelectClass) { + try { + AbstractSingleSelect<String> select = singleSelectClass + .newInstance(); + select.setItems("Foo", "Bar", "Baz", "Reset"); + select.setSelectedItem("Bar"); + + select.addValueChangeListener(e -> { + if ("Reset".equals(e.getValue())) { + select.setSelectedItem("Bar"); + } + }); + + select.setId("testComponent"); + + replaceComponent(component, select); + component = select; + } catch (InstantiationException | IllegalAccessException e1) { + throw new RuntimeException("Component creation failed", e1); + } + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelectionTest.java b/uitest/src/test/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelectionTest.java new file mode 100644 index 0000000000..4623dac714 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/abstractsingleselect/AbstractSingleSelectionTest.java @@ -0,0 +1,128 @@ +package com.vaadin.tests.components.abstractsingleselect; + +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized.Parameters; +import org.openqa.selenium.NoSuchElementException; + +import com.vaadin.testbench.elements.AbstractSingleSelectElement; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.ComboBoxElement; +import com.vaadin.testbench.elements.NativeSelectElement; +import com.vaadin.testbench.elements.RadioButtonGroupElement; +import com.vaadin.tests.tb3.ParameterizedTB3Runner; +import com.vaadin.tests.tb3.SingleBrowserTest; + +@RunWith(ParameterizedTB3Runner.class) +public class AbstractSingleSelectionTest extends SingleBrowserTest { + + private static final Map<String, Class<? extends AbstractSingleSelectElement>> elementClasses = new LinkedHashMap<>(); + + @Parameters + public static Collection<String> getElementClassNames() { + if (elementClasses.isEmpty()) { + elementClasses.put("RadioButtonGroup", + RadioButtonGroupElement.class); + elementClasses.put("NativeSelect", NativeSelectElement.class); + elementClasses.put("ComboBox", ComboBoxElement.class); + } + + return elementClasses.keySet(); + } + + private String elementClassName; + + public void setElementClassName(String elementClassName) { + this.elementClassName = elementClassName; + } + + @Before + public void before() { + openTestURL(); + } + + @Test + public void testSelectNull() { + $(NativeSelectElement.class).first().selectByText(elementClassName); + + assertInitial(); + + $(ButtonElement.class).caption("Deselect").first().click(); + + AbstractSingleSelectElement selectElement = getSelectElement(); + // TODO: TB API behavior should be unified. + if (selectElement instanceof RadioButtonGroupElement) { + Assert.assertNull("No value should be selected", + selectElement.getValue()); + } else if (selectElement instanceof ComboBoxElement) { + Assert.assertTrue("No value should be selected", + selectElement.getValue().isEmpty()); + } else { + // NativeSelectElement throws if no value is selected. + try { + selectElement.getValue(); + Assert.fail("No value should be selected"); + } catch (NoSuchElementException e) { + // All is fine. + } + } + } + + @Test + public void testSelectOnClientAndRefresh() { + $(NativeSelectElement.class).first().selectByText(elementClassName); + + assertInitial(); + + AbstractSingleSelectElement select = getSelectElement(); + select.selectByText("Baz"); + Assert.assertEquals("Value should change", "Baz", select.getValue()); + + $(ButtonElement.class).caption("Refresh").first().click(); + Assert.assertEquals("Value should stay the same through refreshAll", + "Baz", select.getValue()); + } + + @Test + public void testSelectOnClientAndResetValueOnServer() { + $(NativeSelectElement.class).first().selectByText(elementClassName); + + assertInitial(); + + AbstractSingleSelectElement select = getSelectElement(); + select.selectByText("Baz"); + Assert.assertEquals("Value should change", "Baz", select.getValue()); + + $(ButtonElement.class).caption("Select Bar").first().click(); + Assert.assertEquals("Original value should be selected again", "Bar", + select.getValue()); + } + + @Test + public void testSelectOnClientAndResetValueOnServerInListener() { + $(NativeSelectElement.class).first().selectByText(elementClassName); + + assertInitial(); + + AbstractSingleSelectElement rbg = getSelectElement(); + rbg.selectByText("Reset"); + // Selecting "Reset" selects "Bar" on server. Value was initially "Bar" + Assert.assertEquals("Original value should be selected again", "Bar", + rbg.getValue()); + } + + private void assertInitial() { + Assert.assertEquals("Initial state unexpected", "Bar", + getSelectElement().getValue()); + } + + private AbstractSingleSelectElement getSelectElement() { + return $(elementClasses.get(elementClassName)).id("testComponent"); + } +} |