From: Teemu Suo-Anttila Date: Tue, 13 Jan 2015 14:06:15 +0000 (+0200) Subject: Fix grid sorting on init and sorting with unused properties (#16192) X-Git-Tag: 7.4.0.beta3~4^2^2~19^2~12 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=67090d9229707af3246eecc2ea56ad68e138772f;p=vaadin-framework.git Fix grid sorting on init and sorting with unused properties (#16192) Change-Id: I247a981c6a38bf78936f81f087ce3d5e6d354149 --- diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 98562a871e..5a68bbf4b8 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -517,6 +517,11 @@ public class GridConnector extends AbstractHasComponentsConnector implements updateFooterFromState(getState().footer); } + if (stateChangeEvent.hasPropertyChanged("sortColumns") + || stateChangeEvent.hasPropertyChanged("sortDirs")) { + onSortStateChange(); + } + if (stateChangeEvent.hasPropertyChanged("editorEnabled")) { getWidget().setEditorEnabled(getState().editorEnabled); } @@ -837,7 +842,6 @@ public class GridConnector extends AbstractHasComponentsConnector implements } } - @OnStateChange({ "sortColumns", "sortDirs" }) private void onSortStateChange() { List sortOrder = new ArrayList(); diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java index 443e51fba3..0d74c01027 100644 --- a/server/src/com/vaadin/ui/Grid.java +++ b/server/src/com/vaadin/ui/Grid.java @@ -3638,9 +3638,18 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Sets the current sort order using the fluid Sort API. Read the * documentation for {@link Sort} for more information. + *

+ * Note: Sorting by a property that has no column in Grid will hide + * all possible sorting indicators. * * @param s * a sort instance + * + * @throws IllegalStateException + * if container is not sortable (does not implement + * Container.Sortable) + * @throws IllegalArgumentException + * if trying to sort by non-existing property */ public void sort(Sort s) { setSortOrder(s.build()); @@ -3648,9 +3657,18 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Sort this Grid in ascending order by a specified property. + *

+ * Note: Sorting by a property that has no column in Grid will hide + * all possible sorting indicators. * * @param propertyId * a property ID + * + * @throws IllegalStateException + * if container is not sortable (does not implement + * Container.Sortable) + * @throws IllegalArgumentException + * if trying to sort by non-existing property */ public void sort(Object propertyId) { sort(propertyId, SortDirection.ASCENDING); @@ -3658,11 +3676,20 @@ public class Grid extends AbstractComponent implements SelectionNotifier, /** * Sort this Grid in user-specified {@link SortOrder} by a property. + *

+ * Note: Sorting by a property that has no column in Grid will hide + * all possible sorting indicators. * * @param propertyId * a property ID * @param direction * a sort order value (ascending/descending) + * + * @throws IllegalStateException + * if container is not sortable (does not implement + * Container.Sortable) + * @throws IllegalArgumentException + * if trying to sort by non-existing property */ public void sort(Object propertyId, SortDirection direction) { sort(Sort.by(propertyId, direction)); @@ -3677,20 +3704,26 @@ public class Grid extends AbstractComponent implements SelectionNotifier, } /** - * Sets the sort order to use. This method throws - * {@link IllegalStateException} if the attached container is not a - * {@link Container.Sortable}, and {@link IllegalArgumentException} if a - * property in the list is not recognized by the container, or if the - * 'order' parameter is null. + * Sets the sort order to use. + *

+ * Note: Sorting by a property that has no column in Grid will hide + * all possible sorting indicators. * * @param order * a sort order list. + * + * @throws IllegalStateException + * if container is not sortable (does not implement + * Container.Sortable) + * @throws IllegalArgumentException + * if order is null or trying to sort by non-existing property */ public void setSortOrder(List order) { setSortOrder(order, false); } - private void setSortOrder(List order, boolean userOriginated) { + private void setSortOrder(List order, boolean userOriginated) + throws IllegalStateException, IllegalArgumentException { if (!(getContainerDataSource() instanceof Container.Sortable)) { throw new IllegalStateException( "Attached container is not sortable (does not implement Container.Sortable)"); @@ -3740,15 +3773,12 @@ public class Grid extends AbstractComponent implements SelectionNotifier, Object[] propertyIds = new Object[items]; boolean[] directions = new boolean[items]; - String[] columnKeys = new String[items]; SortDirection[] stateDirs = new SortDirection[items]; for (int i = 0; i < items; ++i) { SortOrder order = sortOrder.get(i); - columnKeys[i] = this.columnKeys.key(order.getPropertyId()); stateDirs[i] = order.getDirection(); - propertyIds[i] = order.getPropertyId(); switch (order.getDirection()) { case ASCENDING: @@ -3768,8 +3798,18 @@ public class Grid extends AbstractComponent implements SelectionNotifier, fireEvent(new SortEvent(this, new ArrayList(sortOrder), userOriginated)); - getState().sortColumns = columnKeys; - getState(false).sortDirs = stateDirs; + if (columns.keySet().containsAll(Arrays.asList(propertyIds))) { + String[] columnKeys = new String[items]; + for (int i = 0; i < items; ++i) { + columnKeys[i] = this.columnKeys.key(propertyIds[i]); + } + getState().sortColumns = columnKeys; + getState(false).sortDirs = stateDirs; + } else { + // Not all sorted properties are in Grid. Remove any indicators. + getState().sortColumns = new String[] {}; + getState(false).sortDirs = new SortDirection[] {}; + } } else { throw new IllegalStateException( "Container is not sortable (does not implement Container.Sortable)"); diff --git a/uitest/src/com/vaadin/tests/components/grid/GridGeneratedProperties.java b/uitest/src/com/vaadin/tests/components/grid/GridGeneratedProperties.java index 294c23ffe5..2782a5fc6c 100644 --- a/uitest/src/com/vaadin/tests/components/grid/GridGeneratedProperties.java +++ b/uitest/src/com/vaadin/tests/components/grid/GridGeneratedProperties.java @@ -27,6 +27,7 @@ import com.vaadin.data.util.PropertyValueGenerator; import com.vaadin.data.util.filter.Compare; import com.vaadin.data.util.filter.UnsupportedFilterException; import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.data.sort.SortDirection; import com.vaadin.tests.components.AbstractTestUI; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; @@ -130,6 +131,7 @@ public class GridGeneratedProperties extends AbstractTestUI { }); addComponent(filterButton); + grid.sort(Sort.by("km").then("bar", SortDirection.DESCENDING)); } private Indexed createContainer() { diff --git a/uitest/src/com/vaadin/tests/components/grid/GridGeneratedPropertiesTest.java b/uitest/src/com/vaadin/tests/components/grid/GridGeneratedPropertiesTest.java index c71dd80360..ffcd4c448f 100644 --- a/uitest/src/com/vaadin/tests/components/grid/GridGeneratedPropertiesTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/GridGeneratedPropertiesTest.java @@ -23,6 +23,7 @@ import org.junit.Test; import com.vaadin.testbench.elements.GridElement; import com.vaadin.testbench.elements.GridElement.GridCellElement; +import com.vaadin.testbench.elements.NotificationElement; import com.vaadin.tests.annotations.TestCategory; import com.vaadin.tests.tb3.MultiBrowserTest; @@ -69,4 +70,21 @@ public class GridGeneratedPropertiesTest extends MultiBrowserTest { assertTrue("Column baz was not sorted descending", bazHeader .getAttribute("class").contains("sort-desc")); } + + @Test + public void testInitialSorting() { + // Grid is sorted in this case by one visible and one nonexistent + // column. There should be no sort indicator. + setDebug(true); + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + + GridCellElement kmHeader = grid.getHeaderCell(0, 1); + assertFalse("Column km was unexpectedly sorted", + kmHeader.getAttribute("class").contains("sort-asc") + || kmHeader.getAttribute("class").contains("sort-desc")); + assertFalse("Unexpected client-side exception was visible", + isElementPresent(NotificationElement.class)); + } }