From 46d1a95f046739a8e158f4fcec4e06fbf3656a11 Mon Sep 17 00:00:00 2001 From: Ahmed Ashour Date: Mon, 23 Oct 2017 09:05:44 +0200 Subject: [PATCH] Grid column to be sortable when implemented/supported (Fixes #8792). (#10190) * Grid column to be sortable when implemented/supported * Fix GridDeclarativeTest * Parameterize to Grid * Revert Parameterize to Grid, JDK with generics. * Assertions for other columns * Fix test Fixes #8792 --- server/src/main/java/com/vaadin/ui/Grid.java | 17 ++ .../component/grid/GridDeclarativeTest.java | 17 +- .../vaadin/shared/ui/grid/ColumnState.java | 4 +- .../components/grid/GridResizeTerror.java | 2 - .../components/grid/GridSingleColumn.java | 4 +- .../components/grid/GridSortWhenPossible.java | 103 +++++++++++ .../components/grid/GridAriaRowcountTest.java | 8 +- .../components/grid/GridEditorEventsTest.java | 13 +- .../components/grid/GridSelectionTest.java | 1 - .../grid/GridSortWhenPossibleTest.java | 175 ++++++++++++++++++ .../grid/basics/GridBasicDetailsTest.java | 29 ++- 11 files changed, 336 insertions(+), 37 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridSortWhenPossible.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridSortWhenPossibleTest.java diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java index 50361528a7..7282afb44c 100644 --- a/server/src/main/java/com/vaadin/ui/Grid.java +++ b/server/src/main/java/com/vaadin/ui/Grid.java @@ -1157,10 +1157,16 @@ public class Grid extends AbstractListing implements HasComponents, } this.userId = id; getGrid().setColumnId(id, this); + updateSortable(); return this; } + private void updateSortable() { + setSortable(getGrid().getDataProvider().isInMemory() + || getSortOrder(SortDirection.ASCENDING).count() != 0); + } + /** * Gets the function used to produce the value for data in this column * based on the row item. @@ -2711,6 +2717,8 @@ public class Grid extends AbstractListing implements HasComponents, if (getDefaultHeaderRow() != null) { getDefaultHeaderRow().getCell(column).setText(column.getCaption()); } + + column.updateSortable(); } /** @@ -4587,4 +4595,13 @@ public class Grid extends AbstractListing implements HasComponents, order -> order.getSorted().getComparator(order.getDirection())) .reduce((x, y) -> 0, operator); } + + @Override + protected void internalSetDataProvider(DataProvider dataProvider) { + super.internalSetDataProvider(dataProvider); + for (Column column : getColumns()) { + column.updateSortable(); + } + } + } diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java index a4f824d6fa..93966ed01f 100644 --- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java +++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridDeclarativeTest.java @@ -200,13 +200,18 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest { int expandRatio = 83; column2.setExpandRatio(expandRatio); + + String sortableSuffix = ""; + if (sortable) { + sortableSuffix = "='true'"; + } String design = String.format("<%s>" - + "" + + "" + "" + "" + "" + "" + "" - + "
%s%s
", getComponentTag(), sortable, resizable, + + "", getComponentTag(), sortableSuffix, resizable, hidingToggleCaption, width, minWidth, maxWidth, expandRatio, caption, "Id", getComponentTag()); @@ -694,8 +699,14 @@ public class GridDeclarativeTest extends AbstractListingDeclarativeTest { col2.getMinimumWidth()); assertEquals(baseError + "Expand ratio", col1.getExpandRatio(), col2.getExpandRatio()); - assertEquals(baseError + "Sortable", col1.isSortable(), + + String id1 = col1.getId(); + String id2 = col2.getId(); + // column.getId() affects .isSortable() + if ((id1 != null && id2 != null) || (id1 == null && id2 == null)) { + assertEquals(baseError + "Sortable", col1.isSortable(), col2.isSortable()); + } assertEquals(baseError + "Editable", col1.isEditable(), col2.isEditable()); assertEquals(baseError + "Hidable", col1.isHidable(), diff --git a/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java b/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java index cda5382670..ae781951c6 100644 --- a/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java +++ b/shared/src/main/java/com/vaadin/shared/ui/grid/ColumnState.java @@ -27,7 +27,7 @@ public class ColumnState extends AbstractGridExtensionState { public String caption; public String internalId; - public boolean sortable = true; + public boolean sortable = false; public boolean editable = false; /** The caption for the column hiding toggle. */ @@ -66,7 +66,7 @@ public class ColumnState extends AbstractGridExtensionState { public Connector renderer; /** * Whether the contents define the minimum width for this column. - * + * * @since 8.1 */ public boolean minimumWidthFromContent = true; diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridResizeTerror.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridResizeTerror.java index c960d2363a..437d7b7c4e 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridResizeTerror.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridResizeTerror.java @@ -30,8 +30,6 @@ public class GridResizeTerror extends UI { protected void init(VaadinRequest request) { Grid grid = new Grid<>(); - int cols = 10; - IntStream.range(0, 10).forEach(i -> grid.addColumn(item -> "Data" + i)); grid.setItems(IntStream.range(0, 500).boxed()); diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSingleColumn.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSingleColumn.java index 105da4f96f..cac1969467 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSingleColumn.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSingleColumn.java @@ -21,7 +21,6 @@ import com.vaadin.data.ValueProvider; import com.vaadin.server.VaadinRequest; import com.vaadin.tests.components.AbstractReindeerTestUI; import com.vaadin.ui.Grid; -import com.vaadin.ui.Grid.Column; import com.vaadin.ui.Grid.SelectionMode; public class GridSingleColumn extends AbstractReindeerTestUI { @@ -33,8 +32,7 @@ public class GridSingleColumn extends AbstractReindeerTestUI { grid.setItems(IntStream.range(0, 100).mapToObj(indx -> "cell")); - Column column = grid.addColumn(ValueProvider.identity()) - .setCaption("Header"); + grid.addColumn(ValueProvider.identity()).setCaption("Header"); addComponent(grid); grid.scrollTo(50); diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortWhenPossible.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortWhenPossible.java new file mode 100644 index 0000000000..92881c7457 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSortWhenPossible.java @@ -0,0 +1,103 @@ +/* + * 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.tests.components.grid; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +import com.vaadin.data.provider.CallbackDataProvider; +import com.vaadin.data.provider.QuerySortOrder; +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.data.sort.SortDirection; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.tests.data.bean.Person; +import com.vaadin.ui.Button; +import com.vaadin.ui.CheckBox; +import com.vaadin.ui.Grid; +import com.vaadin.ui.renderers.NumberRenderer; + +public class GridSortWhenPossible extends AbstractTestUI { + + private List persons; + + @Override + protected void setup(VaadinRequest request) { + persons = Collections.unmodifiableList(Arrays.asList( + createPerson("a", 4, true), createPerson("b", 5, false), + createPerson("c", 3, false), createPerson("a", 6, false), + createPerson("a", 2, true), createPerson("c", 7, false), + createPerson("b", 1, true))); + + CheckBox inMemoryCheckBox = new CheckBox("In memory"); + addComponent(inMemoryCheckBox); + addComponent(new Button("Create Grid", + e -> addComponent(getGrid(inMemoryCheckBox.getValue())))); + } + + private final Grid getGrid(boolean inMemory) { + Grid grid = new Grid<>(); + grid.addColumn(Person::getFirstName).setId("name"); + grid.addColumn(Person::getAge, new NumberRenderer()).setId("age"); + grid.addColumn(Person::getDeceased); + + if (inMemory) { + grid.setItems(persons); + } else { + grid.setDataProvider(new CallbackDataProvider<>(query -> { + List list = new ArrayList<>(persons); + if (!query.getSortOrders().isEmpty()) { + QuerySortOrder order = query.getSortOrders().get(0); + + Comparator comparator; + if ("name".equals(order.getSorted())) { + comparator = Comparator.comparing(Person::getFirstName); + } else { + comparator = Comparator.comparing(Person::getAge); + } + + if (order.getDirection() == SortDirection.DESCENDING) { + comparator = comparator.reversed(); + } + Collections.sort(list, comparator); + } + return list.stream(); + }, query -> persons.size())); + } + return grid; + } + + private Person createPerson(String name, int age, boolean deceased) { + Person person = new Person(); + person.setFirstName(name); + person.setAge(age); + person.setDeceased(deceased); + return person; + + } + + @Override + public String getTestDescription() { + return "Grid columns are sorted, only when sorting is implemented"; + } + + @Override + public Integer getTicketNumber() { + return 8792; + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridAriaRowcountTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridAriaRowcountTest.java index 2c8ddb3b01..db15fa33c8 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridAriaRowcountTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridAriaRowcountTest.java @@ -15,10 +15,13 @@ */ package com.vaadin.tests.components.grid; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + import com.vaadin.testbench.elements.ButtonElement; import com.vaadin.testbench.elements.GridElement; import com.vaadin.tests.tb3.SingleBrowserTest; -import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -77,6 +80,7 @@ public class GridAriaRowcountTest extends SingleBrowserTest { } private boolean containsRows(int rowcount) { - return grid.getHTML().contains("aria-rowcount=\"" + String.valueOf(rowcount) + "\""); + return grid.getHTML() + .contains("aria-rowcount=\"" + String.valueOf(rowcount) + "\""); } } diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorEventsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorEventsTest.java index 989b0a52d5..26c33c2d64 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorEventsTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridEditorEventsTest.java @@ -17,7 +17,6 @@ package com.vaadin.tests.components.grid; import static org.junit.Assert.assertEquals; -import org.junit.Assert; import org.junit.Test; import org.openqa.selenium.WebElement; @@ -41,18 +40,14 @@ public class GridEditorEventsTest extends MultiBrowserTest { GridEditorElement editor = updateField(index, grid, "foo"); editor.save(); - assertEquals((index * 4 + 1) + ". editor is opened", - getLogRow(1)); - assertEquals((index * 4 + 2) + ". editor is saved", - getLogRow(0)); + assertEquals((index * 4 + 1) + ". editor is opened", getLogRow(1)); + assertEquals((index * 4 + 2) + ". editor is saved", getLogRow(0)); editor = updateField(index, grid, "bar"); editor.cancel(); - assertEquals((index * 4 + 3) + ". editor is opened", - getLogRow(1)); - assertEquals((index * 4 + 4) + ". editor is canceled", - getLogRow(0)); + assertEquals((index * 4 + 3) + ". editor is opened", getLogRow(1)); + assertEquals((index * 4 + 4) + ". editor is canceled", getLogRow(0)); } private GridEditorElement updateField(int index, GridElement grid, diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectionTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectionTest.java index 7a11b3375e..09d19ef81a 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectionTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectionTest.java @@ -6,7 +6,6 @@ import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.HashSet; -import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; import org.openqa.selenium.Keys; diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortWhenPossibleTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortWhenPossibleTest.java new file mode 100644 index 0000000000..aede22d367 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSortWhenPossibleTest.java @@ -0,0 +1,175 @@ +/* + * 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.tests.components.grid; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.CheckBoxElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridSortWhenPossibleTest extends MultiBrowserTest { + + @Test + public void inMemory() throws InterruptedException { + openTestURL(); + $(CheckBoxElement.class).first().click(); + $(ButtonElement.class).first().click(); + + GridElement grid = $(GridElement.class).first(); + assertRow(grid, 0, "a", "4", true); + assertRow(grid, 1, "b", "5", false); + assertRow(grid, 2, "c", "3", false); + assertRow(grid, 3, "a", "6", false); + assertRow(grid, 4, "a", "2", true); + assertRow(grid, 5, "c", "7", false); + assertRow(grid, 6, "b", "1", true); + + grid.getHeaderCell(0, 0).click(); + assertTrue("First column should be sorted ascending", + grid.getHeaderCell(0, 0).getAttribute("class") + .contains("sort-asc")); + assertFalse("Second column should not be sorted", grid + .getHeaderCell(0, 1).getAttribute("class").contains("sort-")); + assertFalse("Third column should not be sorted", grid + .getHeaderCell(0, 2).getAttribute("class").contains("sort-")); + + assertRow(grid, 0, "a", "4", true); + assertRow(grid, 1, "a", "6", false); + assertRow(grid, 2, "a", "2", true); + assertRow(grid, 3, "b", "5", false); + assertRow(grid, 4, "b", "1", true); + assertRow(grid, 5, "c", "3", false); + assertRow(grid, 6, "c", "7", false); + + grid.getHeaderCell(0, 1).click(); + + assertFalse("First column should not be sorted", grid + .getHeaderCell(0, 0).getAttribute("class").contains("sort-")); + assertTrue("Second column should be sorted ascending", + grid.getHeaderCell(0, 1).getAttribute("class") + .contains("sort-asc")); + assertFalse("Third column should not be sorted", grid + .getHeaderCell(0, 2).getAttribute("class").contains("sort-")); + + assertRow(grid, 0, "b", "1", true); + assertRow(grid, 1, "a", "2", true); + assertRow(grid, 2, "c", "3", false); + assertRow(grid, 3, "a", "4", true); + assertRow(grid, 4, "b", "5", false); + assertRow(grid, 5, "a", "6", false); + assertRow(grid, 6, "c", "7", false); + + grid.getHeaderCell(0, 2).click(); + + assertFalse("First column should not be sorted", grid + .getHeaderCell(0, 0).getAttribute("class").contains("sort-")); + assertFalse("Second column should not be sorted", grid + .getHeaderCell(0, 1).getAttribute("class").contains("sort-")); + assertTrue("Third column should be sorted ascending", + grid.getHeaderCell(0, 2).getAttribute("class") + .contains("sort-asc")); + + assertRow(grid, 0, "b", "5", false); + assertRow(grid, 1, "c", "3", false); + assertRow(grid, 2, "a", "6", false); + assertRow(grid, 3, "c", "7", false); + assertRow(grid, 4, "a", "4", true); + assertRow(grid, 5, "a", "2", true); + assertRow(grid, 6, "b", "1", true); + } + + @Test + public void lazyLoading() throws InterruptedException { + openTestURL(); + $(ButtonElement.class).first().click(); + + GridElement grid = $(GridElement.class).first(); + assertRow(grid, 0, "a", "4", true); + assertRow(grid, 1, "b", "5", false); + assertRow(grid, 2, "c", "3", false); + assertRow(grid, 3, "a", "6", false); + assertRow(grid, 4, "a", "2", true); + assertRow(grid, 5, "c", "7", false); + assertRow(grid, 6, "b", "1", true); + + grid.getHeaderCell(0, 0).click(); + + assertTrue("First column should be sorted ascending", + grid.getHeaderCell(0, 0).getAttribute("class") + .contains("sort-asc")); + assertFalse("Second column should not be sorted", grid + .getHeaderCell(0, 1).getAttribute("class").contains("sort-")); + assertFalse("Third column should not be sorted", grid + .getHeaderCell(0, 2).getAttribute("class").contains("sort-")); + + assertRow(grid, 0, "a", "4", true); + assertRow(grid, 1, "a", "6", false); + assertRow(grid, 2, "a", "2", true); + assertRow(grid, 3, "b", "5", false); + assertRow(grid, 4, "b", "1", true); + assertRow(grid, 5, "c", "3", false); + assertRow(grid, 6, "c", "7", false); + + grid.getHeaderCell(0, 1).click(); + + assertFalse("First column should not be sorted", grid + .getHeaderCell(0, 0).getAttribute("class").contains("sort-")); + assertTrue("Second column should be sorted ascending", + grid.getHeaderCell(0, 1).getAttribute("class") + .contains("sort-asc")); + assertFalse("Third column should not be sorted", grid + .getHeaderCell(0, 2).getAttribute("class").contains("sort-")); + + assertRow(grid, 0, "b", "1", true); + assertRow(grid, 1, "a", "2", true); + assertRow(grid, 2, "c", "3", false); + assertRow(grid, 3, "a", "4", true); + assertRow(grid, 4, "b", "5", false); + assertRow(grid, 5, "a", "6", false); + assertRow(grid, 6, "c", "7", false); + + grid.getHeaderCell(0, 2).click(); + + assertFalse("First column should not be sorted", grid + .getHeaderCell(0, 0).getAttribute("class").contains("sort-")); + assertTrue("Second column should be sorted ascending", + grid.getHeaderCell(0, 1).getAttribute("class") + .contains("sort-asc")); + assertFalse("Third column should not be sorted", grid + .getHeaderCell(0, 2).getAttribute("class").contains("sort-")); + + assertRow(grid, 0, "b", "1", true); + assertRow(grid, 1, "a", "2", true); + assertRow(grid, 2, "c", "3", false); + assertRow(grid, 3, "a", "4", true); + assertRow(grid, 4, "b", "5", false); + assertRow(grid, 5, "a", "6", false); + assertRow(grid, 6, "c", "7", false); + } + + private void assertRow(GridElement grid, int row, Object... values) { + for (int col = 0; col < values.length; col++) { + assertEquals(String.valueOf(values[col]), + grid.getCell(row, col).getText()); + } + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java index 614de92071..3cc1906dcc 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/basics/GridBasicDetailsTest.java @@ -6,7 +6,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.openqa.selenium.NoSuchElementException; @@ -23,20 +22,20 @@ public class GridBasicDetailsTest extends GridBasicsTest { * awkward with two scroll commands back to back. */ private static final int ALMOST_LAST_INDEX = 995; - private static final String[] OPEN_ALMOST_LAST_ITEM_DETAILS = { - "Component", "Details", "Open " + ALMOST_LAST_INDEX }; - private static final String[] OPEN_FIRST_ITEM_DETAILS = { - "Component", "Details", "Open First" }; - private static final String[] TOGGLE_FIRST_ITEM_DETAILS = { - "Component", "Details", "Toggle First" }; - private static final String[] DETAILS_GENERATOR_NULL = { - "Component", "Details", "Generators", "NULL" }; - private static final String[] DETAILS_GENERATOR_WATCHING = { - "Component", "Details", "Generators", "\"Watching\"" }; - private static final String[] DETAILS_GENERATOR_PERSISTING = { - "Component", "Details", "Generators", "Persisting" }; - private static final String[] CHANGE_HIERARCHY = { "Component", - "Details", "Generators", "- Change Component" }; + private static final String[] OPEN_ALMOST_LAST_ITEM_DETAILS = { "Component", + "Details", "Open " + ALMOST_LAST_INDEX }; + private static final String[] OPEN_FIRST_ITEM_DETAILS = { "Component", + "Details", "Open First" }; + private static final String[] TOGGLE_FIRST_ITEM_DETAILS = { "Component", + "Details", "Toggle First" }; + private static final String[] DETAILS_GENERATOR_NULL = { "Component", + "Details", "Generators", "NULL" }; + private static final String[] DETAILS_GENERATOR_WATCHING = { "Component", + "Details", "Generators", "\"Watching\"" }; + private static final String[] DETAILS_GENERATOR_PERSISTING = { "Component", + "Details", "Generators", "Persisting" }; + private static final String[] CHANGE_HIERARCHY = { "Component", "Details", + "Generators", "- Change Component" }; @Override @Before -- 2.39.5