From 55864a333a053d7a70dafa235bd5f236d641319f Mon Sep 17 00:00:00 2001 From: Anastasia Smirnova Date: Thu, 15 Nov 2018 08:56:40 +0200 Subject: Refactoring DataCommunicator code (#11304) This refactoring addresses two issues cased by dropping updatedData before it was processed. Issues arise , when visibility has change. Fixes #11274 and similar issue within RadioButton There is no reliable way to test automatically NativeSelects in Grid, but adding UI test, at least --- .../com/vaadin/data/provider/DataCommunicator.java | 18 +++--- .../nativeselect/NativeSelectsInGrid.java | 71 ++++++++++++++++++++++ .../RadioButtonGroupAfterVisibilityChange.java | 35 +++++++++++ .../RadioButtonGroupAfterVisibilityChangeTest.java | 41 +++++++++++++ 4 files changed, 156 insertions(+), 9 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java create mode 100644 uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java diff --git a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java index bb76d7cdcb..449cbc5711 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java +++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java @@ -358,6 +358,15 @@ public class DataCommunicator extends AbstractExtension { rpc.reset(getDataProviderSize()); } + if (!updatedData.isEmpty()) { + JsonArray dataArray = Json.createArray(); + int i = 0; + for (T data : updatedData) { + dataArray.set(i++, getDataObject(data)); + } + rpc.updateData(dataArray); + } + Range requestedRows = getPushRows(); boolean triggerReset = false; if (!requestedRows.isEmpty()) { @@ -373,15 +382,6 @@ public class DataCommunicator extends AbstractExtension { pushData(offset, rowsToPush); } - if (!updatedData.isEmpty()) { - JsonArray dataArray = Json.createArray(); - int i = 0; - for (T data : updatedData) { - dataArray.set(i++, getDataObject(data)); - } - rpc.updateData(dataArray); - } - setPushRows(Range.withLength(0, 0)); reset = triggerReset; updatedData.clear(); diff --git a/uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java b/uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java new file mode 100644 index 0000000000..55efd7d1f7 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/nativeselect/NativeSelectsInGrid.java @@ -0,0 +1,71 @@ +package com.vaadin.tests.components.nativeselect; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Component; +import com.vaadin.ui.Label; +import com.vaadin.ui.NativeSelect; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Button; + +@Widgetset("com.vaadin.DefaultWidgetSet") +public class NativeSelectsInGrid extends AbstractTestUI { + @Override + protected void setup(VaadinRequest request) { + final VerticalLayout layout = new VerticalLayout(new Label( + "Keep opening the editor and selecting an enum value. Eventually the list will not show " + + "all options"), + createGrid()); + NativeSelect ns = new NativeSelect<>(); + ns.setEmptySelectionAllowed(false); + ns.setItems(Enum.values()); + + NativeSelect ns2 = new NativeSelect<>(); + ns2.setItems(Enum.values()); + addComponent(ns); + addComponent(ns2); + addComponent(new Button("Change visibility of NS", e -> { + ns2.setVisible(!ns2.isVisible()); + })); + addComponent(layout); + + } + + public Component createGrid() { + Grid grid = new Grid<>(); + grid.setItems(new Person(Enum.baz), new Person(Enum.foo), + new Person(Enum.bizzle)); + grid.setSizeFull(); + grid.setSelectionMode(Grid.SelectionMode.NONE); + + NativeSelect ns = new NativeSelect<>(); + ns.setEmptySelectionAllowed(false); + ns.setItems(Enum.values()); + grid.addColumn(Person::getEnumValue).setCaption("Enum value") + .setEditorComponent(ns, Person::setEnumValue); + grid.getEditor().setEnabled(true); + return grid; + } + + enum Enum { + foo, bar, baz, bizzle, quux + } + + public static class Person { + Enum enumValue; + + public Person(Enum progress) { + this.enumValue = progress; + } + + public Enum getEnumValue() { + return enumValue; + } + + public void setEnumValue(Enum enumValue) { + this.enumValue = enumValue; + } + } +} diff --git a/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java b/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java new file mode 100644 index 0000000000..8e1d985171 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChange.java @@ -0,0 +1,35 @@ +package com.vaadin.tests.components.radiobuttongroup; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.RadioButtonGroup; + +import java.util.Arrays; + +@Widgetset("com.vaadin.DefaultWidgetSet") +public class RadioButtonGroupAfterVisibilityChange extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + + RadioButtonGroup radio = new RadioButtonGroup<>("Radio", + Arrays.asList(true, false)); + radio.setId("radioButton"); + addComponent(radio); + + Button hideButton = new Button("Hide"); + hideButton.setId("hideB"); + hideButton.addClickListener(event1 -> radio.setVisible(false)); + addComponent(hideButton); + + Button setAndShowButton = new Button("Set and Show"); + setAndShowButton.setId("setAndShow"); + setAndShowButton.addClickListener(event1 -> { + radio.setValue(true); + radio.setVisible(true); + }); + addComponent(setAndShowButton); + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java new file mode 100644 index 0000000000..eff1798e79 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/radiobuttongroup/RadioButtonGroupAfterVisibilityChangeTest.java @@ -0,0 +1,41 @@ +package com.vaadin.tests.components.radiobuttongroup; + +import com.vaadin.testbench.elements.RadioButtonGroupElement; +import com.vaadin.tests.tb3.MultiBrowserTest; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +import java.util.List; + +import static junit.framework.TestCase.assertEquals; +import static junit.framework.TestCase.assertTrue; + +public class RadioButtonGroupAfterVisibilityChangeTest + extends MultiBrowserTest { + + @Test + public void verifyOptionIsSelectable() { + openTestURL(); + + getRadioButtonGroupElement().selectByText("false"); + findElement(By.id("hideB")).click(); + findElement(By.id("setAndShow")).click(); + isSelectedOnClientSide("true"); + getRadioButtonGroupElement().selectByText("false"); + isSelectedOnClientSide("false"); + } + + private RadioButtonGroupElement getRadioButtonGroupElement() { + RadioButtonGroupElement radioButtonGroup = $( + RadioButtonGroupElement.class).first(); + return radioButtonGroup; + } + + private void isSelectedOnClientSide(String selectedText) { + List selectOptions = findElement(By.id("radioButton")) + .findElements(By.className("v-select-option-selected")); + assertEquals("Item should be selected", selectOptions.size(), 1); + assertTrue(selectOptions.get(0).getText().equals(selectedText)); + } +} -- cgit v1.2.3