From fa10456957a880e5860080a9dcccf39827c97700 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Mon, 19 Jun 2017 13:23:47 +0300 Subject: [PATCH] Fix displaying Grid sort order set from server (#9530) Fixes #8316 --- .../client/connectors/grid/GridConnector.java | 28 ++++++++-- server/src/main/java/com/vaadin/ui/Grid.java | 12 +++++ .../components/grid/GridSortIndicator.java | 51 +++++------------- .../grid/GridSortIndicatorTest.java | 52 +++++++++---------- 4 files changed, 73 insertions(+), 70 deletions(-) diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java index 0e234cc968..e94493757e 100644 --- a/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java +++ b/client/src/main/java/com/vaadin/client/connectors/grid/GridConnector.java @@ -16,6 +16,7 @@ package com.vaadin.client.connectors.grid; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -419,6 +420,21 @@ public class GridConnector extends AbstractListingConnector } } + @OnStateChange({ "sortColumns", "sortDirs" }) + void updateSortOrder() { + List sortOrder = new ArrayList(); + + String[] sortColumns = getState().sortColumns; + SortDirection[] sortDirs = getState().sortDirs; + + for (int i = 0; i < sortColumns.length; i++) { + sortOrder + .add(new SortOrder(getColumn(sortColumns[i]), sortDirs[i])); + } + + getWidget().setSortOrder(sortOrder); + } + @Override public void setDataSource(DataSource dataSource) { super.setDataSource(dataSource); @@ -501,9 +517,15 @@ public class GridConnector extends AbstractListingConnector sortDirections.add(so.getDirection()); } } - getRpcProxy(GridServerRpc.class).sort(columnIds.toArray(new String[0]), - sortDirections.toArray(new SortDirection[0]), - event.isUserOriginated()); + String[] colArray = columnIds.toArray(new String[0]); + SortDirection[] dirArray = sortDirections.toArray(new SortDirection[0]); + + if (!Arrays.equals(colArray, getState().sortColumns) + || !Arrays.equals(dirArray, getState().sortDirs)) { + // State has actually changed, send to server + getRpcProxy(GridServerRpc.class).sort(colArray, dirArray, + event.isUserOriginated()); + } } /* HasComponentsConnector */ diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index 1254814b3f..ac7eb4f0e9 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -4220,6 +4220,18 @@ public class Grid extends AbstractListing implements HasComponents, private void setSortOrder(List> order, boolean userOriginated) { Objects.requireNonNull(order, "Sort order list cannot be null"); + + // Update client state to display sort order. + List sortColumns = new ArrayList<>(); + List directions = new ArrayList<>(); + order.stream().forEach(sortOrder -> { + sortColumns.add(sortOrder.getSorted().getInternalId()); + directions.add(sortOrder.getDirection()); + }); + + getState().sortColumns = sortColumns.toArray(new String[0]); + getState().sortDirs = directions.toArray(new SortDirection[0]); + sortOrder.clear(); if (order.isEmpty()) { // Grid is not sorted anymore. diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortIndicator.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortIndicator.java index 5176b25fbd..0e63f672d5 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortIndicator.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortIndicator.java @@ -15,54 +15,28 @@ */ package com.vaadin.tests.components.grid; -import java.util.ArrayList; -import java.util.List; - +import com.vaadin.annotations.Widgetset; import com.vaadin.data.provider.GridSortOrder; import com.vaadin.server.VaadinRequest; import com.vaadin.shared.data.sort.SortDirection; -import com.vaadin.tests.components.AbstractReindeerTestUI; +import com.vaadin.tests.components.AbstractTestUI; import com.vaadin.tests.data.bean.Person; +import com.vaadin.ui.Button; import com.vaadin.ui.Grid; import com.vaadin.ui.renderers.NumberRenderer; -/* - * Test UI for checking that sort indicators of a Grid are updated when the sort order is changed by a - * SortListener. - */ -public class GridSortIndicator extends AbstractReindeerTestUI { - - private SortDirection oldSortDirection = null; +@Widgetset("com.vaadin.DefaultWidgetSet") +public class GridSortIndicator extends AbstractTestUI { @Override protected void setup(VaadinRequest request) { final Grid grid = getGrid(); addComponent(grid); - grid.addSortListener(order -> { - ArrayList> currentSortOrder = new ArrayList<>( - order.getSortOrder()); - if (currentSortOrder.size() == 1) { - // If the name column was clicked, set a new sort order for - // both columns. Otherwise, revert to oldSortDirection if it - // is not null. - List> newSortOrder = new ArrayList<>(); - SortDirection newSortDirection = oldSortDirection; - if (currentSortOrder.get(0).getSorted().getId() - .equals("name")) { - newSortDirection = SortDirection.ASCENDING - .equals(oldSortDirection) ? SortDirection.DESCENDING - : SortDirection.ASCENDING; - } - if (newSortDirection != null) { - newSortOrder.add(new GridSortOrder<>(grid.getColumn("name"), - newSortDirection)); - newSortOrder.add(new GridSortOrder<>(grid.getColumn("age"), - newSortDirection)); - grid.setSortOrder(newSortOrder); - } - oldSortDirection = newSortDirection; - } - }); + addComponent(new Button("Sort first", e -> grid + .sort(grid.getColumn("name"), SortDirection.ASCENDING))); + addComponent(new Button("Sort both", + e -> grid.setSortOrder(GridSortOrder.asc(grid.getColumn("name")) + .thenAsc(grid.getColumn("age"))))); } private final Grid getGrid() { @@ -86,9 +60,8 @@ public class GridSortIndicator extends AbstractReindeerTestUI { @Override public String getTestDescription() { - return "When the first column is the primary sort column, both columns should have " - + "a sort indicator with the same sort direction. Clicking on the right column " - + "in that state should have no effect."; + return "UI to test server-side sorting of grid columns " + + "and displaying sort indicators"; } @Override diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortIndicatorTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortIndicatorTest.java index 3fc50223b1..429e5a671e 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortIndicatorTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortIndicatorTest.java @@ -15,44 +15,40 @@ */ package com.vaadin.tests.components.grid; -import static org.junit.Assert.assertTrue; - -import org.junit.Ignore; +import org.junit.Assert; import org.junit.Test; +import com.vaadin.testbench.elements.ButtonElement; import com.vaadin.testbench.elements.GridElement; import com.vaadin.testbench.parallel.TestCategory; -import com.vaadin.tests.tb3.MultiBrowserTest; +import com.vaadin.tests.tb3.SingleBrowserTest; @TestCategory("grid") -public class GridSortIndicatorTest extends MultiBrowserTest { +public class GridSortIndicatorTest extends SingleBrowserTest { @Test - @Ignore - /* - * Should be enabled once #8316 is fixed. - */ public void testIndicators() throws InterruptedException { openTestURL(); GridElement grid = $(GridElement.class).first(); - // Clicking the left header cell should set ascending sort order for - // both columns. - grid.getHeaderCell(0, 0).click(); - assertTrue(grid.getHeaderCell(0, 0).getAttribute("class") - .contains("sort-asc")); - assertTrue(grid.getHeaderCell(0, 1).getAttribute("class") - .contains("sort-asc")); - // Click the left column to change the sort direction. - grid.getHeaderCell(0, 0).click(); - assertTrue(grid.getHeaderCell(0, 0).getAttribute("class") - .contains("sort-desc")); - assertTrue(grid.getHeaderCell(0, 1).getAttribute("class") - .contains("sort-desc")); - // Clicking on the right column should have no effect. - grid.getHeaderCell(0, 1).click(); - assertTrue(grid.getHeaderCell(0, 0).getAttribute("class") - .contains("sort-desc")); - assertTrue(grid.getHeaderCell(0, 1).getAttribute("class") - .contains("sort-desc")); + + $(ButtonElement.class).caption("Sort both").first().click(); + Assert.assertTrue("First column should be sorted ascending", + grid.getHeaderCell(0, 0).getAttribute("class") + .contains("sort-asc")); + Assert.assertEquals("First column should be first in sort order", "1", + grid.getHeaderCell(0, 0).getAttribute("sort-order")); + Assert.assertTrue("Second column should be sorted ascending", + grid.getHeaderCell(0, 1).getAttribute("class") + .contains("sort-asc")); + Assert.assertEquals("Second column should be also sorted", "2", + grid.getHeaderCell(0, 1).getAttribute("sort-order")); + + $(ButtonElement.class).caption("Sort first").first().click(); + Assert.assertTrue("First column should be sorted ascending", + grid.getHeaderCell(0, 0).getAttribute("class") + .contains("sort-asc")); + Assert.assertFalse("Second column should not be sorted", + grid.getHeaderCell(0, 1).getAttribute("class") + .contains("sort-asc")); } } -- 2.39.5