From 8bc470f4404d21a65520d4eae50b434dbe92985a Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Thu, 25 Jan 2018 14:36:51 +0200 Subject: [PATCH] Fix Grid not updating selected item immediately (#10569) --- .../data/provider/DataCommunicator.java | 21 ++++++++---------- .../tests/data/DataProviderRefresh.java | 22 ++++++++++++++----- .../tests/data/DataProviderRefreshTest.java | 5 +++++ 3 files changed, 31 insertions(+), 17 deletions(-) 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 2819543521..43dcab0271 100644 --- a/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java +++ b/server/src/main/java/com/vaadin/data/provider/DataCommunicator.java @@ -91,8 +91,7 @@ public class DataCommunicator extends AbstractExtension { * that are not in the given collection will be cleaned up and * {@link DataGenerator#destroyData(Object)} will be called for them. */ - protected class ActiveDataHandler - implements DataGenerator { + protected class ActiveDataHandler implements DataGenerator { /** * Set of key strings for currently active data objects @@ -754,16 +753,14 @@ public class DataCommunicator extends AbstractExtension { private void attachDataProviderListener() { dataProviderUpdateRegistration = getDataProvider() .addDataProviderListener(event -> { - getUI().access(() -> { - if (event instanceof DataRefreshEvent) { - T item = ((DataRefreshEvent) event).getItem(); - getKeyMapper().refresh(item); - generators.forEach(g -> g.refreshData(item)); - refresh(item); - } else { - hardReset(); - } - }); + if (event instanceof DataRefreshEvent) { + T item = ((DataRefreshEvent) event).getItem(); + getKeyMapper().refresh(item); + generators.forEach(g -> g.refreshData(item)); + getUI().access(() -> refresh(item)); + } else { + getUI().access(this::hardReset); + } }); } diff --git a/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java b/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java index 9f1591407c..faec5084f7 100644 --- a/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java +++ b/uitest/src/main/java/com/vaadin/tests/data/DataProviderRefresh.java @@ -3,16 +3,17 @@ package com.vaadin.tests.data; import java.io.Serializable; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import com.vaadin.data.provider.AbstractDataProvider; import com.vaadin.data.provider.Query; import com.vaadin.server.VaadinRequest; -import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.tests.components.AbstractTestUIWithLog; import com.vaadin.ui.Button; import com.vaadin.ui.Grid; -public class DataProviderRefresh extends AbstractTestUI { +public class DataProviderRefresh extends AbstractTestUIWithLog { public static class Bean implements Serializable { @@ -96,9 +97,10 @@ public class DataProviderRefresh extends AbstractTestUI { } } + private Grid grid = new Grid<>(); + @Override protected void setup(VaadinRequest request) { - Grid grid = new Grid<>(); List arrayList = new ArrayList<>(); Bean foo = new Bean("Foo", 10); arrayList.add(foo); @@ -108,9 +110,19 @@ public class DataProviderRefresh extends AbstractTestUI { grid.setDataProvider(dataProvider); grid.addColumn(Object::toString).setCaption("toString"); addComponent(grid); - addComponent(new Button("Replace item", - event -> dataProvider.refreshItem(new Bean("Bar", 10)))); + addComponent(new Button("Replace item", event -> { + dataProvider.refreshItem(new Bean("Bar", 10)); + logSelectedItem(grid.asSingleSelect().getValue()); + })); addComponent(new Button("Select old", event -> grid.select(foo))); + grid.asSingleSelect().addValueChangeListener(e -> { + logSelectedItem(e.getValue()); + }); } + private void logSelectedItem(Bean bean) { + Optional.ofNullable(bean) + .map(b -> "Currently selected: " + b.toString()) + .ifPresent(this::log); + } } diff --git a/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java b/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java index bbb31535a8..03e566a741 100644 --- a/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java +++ b/uitest/src/test/java/com/vaadin/tests/data/DataProviderRefreshTest.java @@ -2,8 +2,11 @@ package com.vaadin.tests.data; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.hamcrest.CoreMatchers.*; + import org.junit.Test; import com.vaadin.testbench.elements.ButtonElement; @@ -21,12 +24,14 @@ public class DataProviderRefreshTest extends SingleBrowserTest { // Select item before replace $(ButtonElement.class).caption("Select old").first().click(); assertTrue("Row should be selected", grid.getRow(0).isSelected()); + assertThat(getLogRow(0), containsString("{ Foo, 10 }")); $(ButtonElement.class).caption("Replace item").first().click(); assertTrue("Row should still be selected after item replace", grid.getRow(0).isSelected()); assertEquals("Grid content was not updated.", "{ Bar, 10 }", grid.getCell(0, 0).getText()); + assertThat(getLogRow(0), containsString("{ Bar, 10 }")); // Deselect row grid.getCell(0, 0).click(); -- 2.39.5