diff options
8 files changed, 193 insertions, 3 deletions
diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java index f605329fd9..7e7389e65d 100644 --- a/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java +++ b/client/src/main/java/com/vaadin/client/connectors/grid/DetailsManagerConnector.java @@ -702,6 +702,7 @@ public class DetailsManagerConnector extends AbstractExtensionConnector { // updated, replace reference indexToDetailConnectorId.put(rowIndex, id); newOrUpdatedDetails = true; + getWidget().resetVisibleDetails(rowIndex); } } else { // new Details content, listeners will get attached to the connector diff --git a/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java b/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java index 1b05eeb488..fc82bd87d7 100644 --- a/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java +++ b/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java @@ -86,6 +86,16 @@ public interface RowContainer { boolean spacerExists(int rowIndex); /** + * Updates the spacer corresponding with the given rowIndex to currently + * provided contents. + * + * @since + * @param rowIndex + * the row index for the spacer in need of updating + */ + void resetSpacer(int rowIndex); + + /** * Sets a new spacer updater. * <p> * Spacers that are currently visible will be updated, i.e. diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java index 31b439a6d8..f48177fa84 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -4965,6 +4965,11 @@ public class Escalator extends Widget } @Override + public void resetSpacer(int rowIndex) { + spacerContainer.resetSpacer(rowIndex); + } + + @Override public void setSpacerUpdater(SpacerUpdater spacerUpdater) throws IllegalArgumentException { spacerContainer.setSpacerUpdater(spacerUpdater); @@ -6111,8 +6116,10 @@ public class Escalator extends Widget root.getStyle().setHeight(height + defaultCellBorderBottomSize, Unit.PX); - // move the visible spacers getRow row onwards. - shiftSpacerPositionsAfterRow(getRow(), heightDiff); + if (!delayRepositioning) { + // move the visible spacers getRow row onwards. + shiftSpacerPositionsAfterRow(getRow(), heightDiff); + } /* * If we're growing, we'll adjust the scroll size first, then @@ -6178,7 +6185,7 @@ public class Escalator extends Widget tBodyScrollTop + moveDiff); verticalScrollbar.setScrollPosByDelta(moveDiff); - } else { + } else if (!delayRepositioning) { body.shiftRowPositions(getRow(), heightDiff); } @@ -6336,6 +6343,8 @@ public class Escalator extends Widget /** Width of the spacers' decos. Calculated once then cached. */ private double spacerDecoWidth = 0.0D; + private boolean delayRepositioning = false; + public void setSpacer(int rowIndex, double height) throws IllegalArgumentException { @@ -6376,6 +6385,23 @@ public class Escalator extends Widget return false; } + void resetSpacer(int rowIndex) { + if (spacerExists(rowIndex)) { + delayRepositioning = true; + double oldHeight = getSpacer(rowIndex).getHeight(); + removeSpacer(rowIndex); + // real height will be determined later + insertNewSpacer(rowIndex, 0); + // reposition content below this point to match lack of height, + // otherwise later repositioning will fail + if (oldHeight > 0) { + shiftSpacerPositionsAfterRow(rowIndex, -oldHeight); + body.shiftRowPositions(rowIndex, -oldHeight); + } + delayRepositioning = false; + } + } + @SuppressWarnings("boxing") void scrollToSpacer(int spacerIndex, ScrollDestination destination, int padding) { diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java index 792fcc98b0..d2b3860ef6 100755 --- a/client/src/main/java/com/vaadin/client/widgets/Grid.java +++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java @@ -9662,6 +9662,17 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, } /** + * Reset the details row with current contents. + * + * @since + * @param rowIndex + * the index of the row for which details should be reset + */ + public void resetVisibleDetails(int rowIndex) { + escalator.getBody().resetSpacer(rowIndex); + } + + /** * Update details row height. * * @since 8.1.3 diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index ec891396e9..f84dbd40cb 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -4997,7 +4997,17 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents, @Override protected void internalSetDataProvider(DataProvider<T, ?> dataProvider) { + boolean newProvider = getDataProvider() != dataProvider; super.internalSetDataProvider(dataProvider); + if (newProvider) { + Set<T> oldVisibleDetails = new HashSet<>( + detailsManager.visibleDetails); + oldVisibleDetails.forEach(item -> { + // close all old details even if the same item exists in the new + // provider + detailsManager.setDetailsVisible(item, false); + }); + } for (Column<T, ?> column : getColumns()) { column.updateSortable(); } diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridDetailsUpdateItems.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridDetailsUpdateItems.java new file mode 100644 index 0000000000..d84e0466a9 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridDetailsUpdateItems.java @@ -0,0 +1,74 @@ +package com.vaadin.tests.components.grid; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; + +import com.vaadin.data.ValueProvider; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Label; +import com.vaadin.ui.VerticalLayout; + +public class GridDetailsUpdateItems extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + addComponent(createExamplleLayout()); + } + + private VerticalLayout createExamplleLayout() { + Collection<String> firstCollection = Arrays.asList("Hello", ",", + "world!"); + Collection<String> secondCollection = Arrays.asList("My", "name", "is", + "Sarah"); + Collection<String> thirdCollection = Arrays.asList("red", "blue"); + Collection<String> fourthCollection = Arrays.asList("spring", "summer", + "autumn", "winter"); + + VerticalLayout mainLayout = new VerticalLayout(); + Grid<Collection<String>> grid = new Grid<>(); + grid.setDetailsGenerator(collection -> { + VerticalLayout detailLayout = new VerticalLayout(); + collection.forEach( + item -> detailLayout.addComponent(new Label(item))); + return detailLayout; + }); + ValueProvider<Collection<String>, String> valueProvider = collection -> String + .join(" ", collection); + grid.addColumn(valueProvider).setCaption("Header"); + + List<Collection<String>> itemsInitial = Arrays.asList(firstCollection, + secondCollection, thirdCollection, fourthCollection); + grid.setItems(itemsInitial); + for (Collection<String> tmp : itemsInitial) { + grid.setDetailsVisible(tmp, true); + } + mainLayout.addComponent(grid); + + Button clickButton = new Button("Change items", event -> { + List<Collection<String>> itemsOverwrite = Arrays + .asList(secondCollection, fourthCollection); + grid.setItems(itemsOverwrite); + for (Collection<String> tmp : itemsOverwrite) { + grid.setDetailsVisible(tmp, true); + } + }); + mainLayout.addComponent(clickButton); + + return mainLayout; + } + + @Override + protected Integer getTicketNumber() { + return 12211; + } + + @Override + protected String getTestDescription() { + return "Details should update and not break the positioning " + + "when the item set is changed."; + } +} diff --git a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java index 65fe75e9fa..404556f2d6 100644 --- a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java +++ b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java @@ -117,6 +117,11 @@ public class EscalatorProxy extends Escalator { } @Override + public void resetSpacer(int rowIndex) { + rowContainer.resetSpacer(rowIndex); + } + + @Override public void setSpacerUpdater(SpacerUpdater spacerUpdater) throws IllegalArgumentException { rowContainer.setSpacerUpdater(spacerUpdater); diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridDetailsUpdateItemsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridDetailsUpdateItemsTest.java new file mode 100644 index 0000000000..3f690ca7cd --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridDetailsUpdateItemsTest.java @@ -0,0 +1,53 @@ +package com.vaadin.tests.components.grid; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.number.IsCloseTo.closeTo; +import static org.junit.Assert.assertNotEquals; + +import org.junit.Test; + +import com.vaadin.testbench.TestBenchElement; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.GridElement.GridCellElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridDetailsUpdateItemsTest extends MultiBrowserTest { + + @Test + public void testDetailsUpdateWithItems() { + openTestURL(); + GridElement grid = $(GridElement.class).first(); + ButtonElement button = $(ButtonElement.class).first(); + + String details0 = grid.getDetails(0).getText(); + System.out.println("details: " + details0); + + // change the contents + button.click(); + sleep(200); + + TestBenchElement detailCell0 = grid.getDetails(0); + // ensure contents have updated + String updatedDetails0 = detailCell0.getText(); + assertNotEquals("Details should not be empty", "", updatedDetails0); + assertNotEquals("Unexpected detail contents for row 0", details0, + updatedDetails0); + + GridCellElement cell1_0 = grid.getCell(1, 0); + TestBenchElement detailCell1 = grid.getDetails(1); + + // ensure positioning is correct + assertDirectlyAbove(detailCell0, cell1_0); + assertDirectlyAbove(cell1_0, detailCell1); + } + + private void assertDirectlyAbove(TestBenchElement above, + TestBenchElement below) { + int aboveBottom = above.getLocation().getY() + + above.getSize().getHeight(); + int belowTop = below.getLocation().getY(); + assertThat("Unexpected positions.", (double) aboveBottom, + closeTo(belowTop, 1d)); + } +} |