]> source.dussan.org Git - vaadin-framework.git/commitdiff
Fix and unify multi-column sorting behavior (#13334)
authorPatrik Lindström <patrik@vaadin.com>
Wed, 3 Sep 2014 11:50:47 +0000 (14:50 +0300)
committerVaadin Code Review <review@vaadin.com>
Tue, 9 Sep 2014 10:34:49 +0000 (10:34 +0000)
Change-Id: Idc5b66395eb132a3a0a177593f5d91a165a925de

client/src/com/vaadin/client/ui/grid/Grid.java
uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridSortingTest.java

index 478e7b4a9d136b51104cefedbb08cbe3f6d38ad8..cd9b615c4bf5d84d3291e284b3be728982059f7e 100644 (file)
@@ -597,80 +597,107 @@ public class Grid<T> extends Composite implements
     }
 
     /**
-     * Class for sorting at a later time
-     */
-    private class LazySorter extends Timer {
+     * Helper class for performing sorting through the user interface. Controls
+     * the sort() method, reporting USER as the event originator. This is a
+     * completely internal class, and is, as such, safe to re-name should a more
+     * descriptive name come to mind.
+     */
+    private final class UserSorter {
+
+        private final Timer timer;
+        private Cell scheduledCell;
+        private boolean scheduledMultisort;
+
+        private UserSorter() {
+            timer = new Timer() {
+                @Override
+                public void run() {
+                    UserSorter.this.sort(scheduledCell, scheduledMultisort);
+                }
+            };
+        }
 
-        private Cell cell;
+        /**
+         * Toggle sorting for a cell. If the multisort parameter is set to true,
+         * the cell's sort order is modified as a natural part of a multi-sort
+         * chain. If false, the sorting order is set to ASCENDING for that
+         * cell's column. If that column was already the only sorted column in
+         * the Grid, the sort direction is flipped.
+         * 
+         * @param cell
+         *            a valid cell reference
+         * @param multisort
+         *            whether the sort command should act as a multi-sort stack
+         *            or not
+         */
+        public void sort(Cell cell, boolean multisort) {
 
-        private boolean multisort;
+            final GridColumn<?, T> column = getColumnFromVisibleIndex(cell
+                    .getColumn());
+            final SortOrder so = getSortOrder(column);
 
-        @Override
-        public void run() {
-            SortOrder sortingOrder = getSortOrder(getColumnFromVisibleIndex(cell
-                    .getColumn()));
-            if (sortingOrder == null) {
-                /*
-                 * No previous sorting, sort Ascending
-                 */
-                sort(cell, SortDirection.ASCENDING, multisort);
+            if (multisort) {
+
+                // If the sort order exists, replace existing value with its
+                // opposite
+                if (so != null) {
+                    final int idx = sortOrder.indexOf(so);
+                    sortOrder.set(idx, so.getOpposite());
+                } else {
+                    // If it doesn't, just add a new sort order to the end of
+                    // the list
+                    sortOrder.add(new SortOrder(column));
+                }
 
             } else {
-                // Toggle sorting
-                SortDirection direction = sortingOrder.getDirection();
-                if (direction == SortDirection.ASCENDING) {
-                    sort(cell, SortDirection.DESCENDING, multisort);
+
+                // Since we're doing single column sorting, first clear the
+                // list. Then, if the sort order existed, add its opposite,
+                // otherwise just add a new sort value
+
+                int items = sortOrder.size();
+                sortOrder.clear();
+                if (so != null && items == 1) {
+                    sortOrder.add(so.getOpposite());
                 } else {
-                    sort(cell, SortDirection.ASCENDING, multisort);
+                    sortOrder.add(new SortOrder(column));
                 }
             }
+
+            // sortOrder has been changed; tell the Grid to re-sort itself by
+            // user request.
+            Grid.this.sort(SortEventOriginator.USER);
         }
 
         /**
-         * Set the cell reference to the primary cell that sorting should be
-         * done for.
-         * 
-         * @param cell
+         * Perform a sort after a delay.
          * 
+         * @param delay
+         *            delay, in milliseconds
          */
-        public void setCellReference(Cell cell) {
-            this.cell = cell;
+        public void sortAfterDelay(int delay, Cell cell, boolean multisort) {
+            scheduledCell = cell;
+            scheduledMultisort = multisort;
+            timer.schedule(delay);
         }
 
         /**
-         * Is multiple column sorting is enabled/disabled
+         * Check if a delayed sort command has been issued but not yet carried
+         * out.
          * 
-         * @param multisort
-         *            true if multiple column sorting is enabled
+         * @return a boolean value
          */
-        public void setMultisort(boolean multisort) {
-            this.multisort = multisort;
+        public boolean isDelayedSortScheduled() {
+            return timer.isRunning();
         }
 
         /**
-         * Sorts the column in a direction
+         * Cancel a scheduled sort.
          */
-        private void sort(Cell cell, SortDirection direction, boolean multisort) {
-            TableCellElement th = TableCellElement.as(cell.getElement());
-
-            // Apply primary sorting on clicked column
-            GridColumn<?, ?> columnInstance = getColumnFromVisibleIndex(cell
-                    .getColumn());
-            Sort sorting = Sort.by(columnInstance, direction);
-
-            // Re-apply old sorting to the sort order
-            if (multisort) {
-                for (SortOrder order : getSortOrder()) {
-                    if (order.getColumn() != columnInstance) {
-                        sorting = sorting.then(order.getColumn(),
-                                order.getDirection());
-                    }
-                }
-            }
-
-            // Perform sorting; indicate originator as user
-            Grid.this.setSortOrder(sorting.build(), SortEventOriginator.USER);
+        public void cancelDelayedSort() {
+            timer.cancel();
         }
+
     }
 
     /**
@@ -726,7 +753,7 @@ public class Grid<T> extends Composite implements
 
     protected final ActiveCellHandler activeCellHandler;
 
-    private final LazySorter lazySorter = new LazySorter();
+    private final UserSorter sorter = new UserSorter();
 
     private final EditorRow<T> editorRow = GWT.create(EditorRow.class);
 
@@ -1327,39 +1354,7 @@ public class Grid<T> extends Composite implements
                     return;
                 }
 
-                final Cell cell = event.getActiveCell();
-                final GridColumn<?, T> column = columns.get(cell.getColumn());
-
-                // If SHIFT is down, we modify multi-sorting order
-                if (event.isShiftKeyDown() && sortOrder != null) {
-
-                    final SortOrder so = getSortOrder(column);
-
-                    if (so != null) {
-                        // Flip sort direction in-place
-                        final int idx = sortOrder.indexOf(so);
-                        sortOrder.set(idx, so.getOpposite());
-                    } else {
-                        // Add a new sort rule to the end of the list
-                        sortOrder.add(new SortOrder(column));
-                    }
-
-                } else {
-                    if (sortOrder.size() == 1
-                            && sortOrder.get(0).getColumn() == column) {
-
-                        // Reverse the sort order and re-sort
-                        sortOrder.set(0, sortOrder.get(0).getOpposite());
-                    } else {
-
-                        // Manually re-set the sorting order
-                        sortOrder.clear();
-                        sortOrder.add(new SortOrder(column));
-                    }
-                }
-
-                // We've modified the sort order, re-sort it now.
-                setSortOrder(sortOrder, SortEventOriginator.USER);
+                sorter.sort(event.getActiveCell(), event.isShiftKeyDown());
             }
         });
     }
@@ -2240,9 +2235,7 @@ public class Grid<T> extends Composite implements
             rowEventTouchStartingPoint = new Point(touch.getClientX(),
                     touch.getClientY());
 
-            lazySorter.setCellReference(cell);
-            lazySorter.setMultisort(true);
-            lazySorter.schedule(GridConstants.LONG_TAP_DELAY);
+            sorter.sortAfterDelay(GridConstants.LONG_TAP_DELAY, cell, true);
 
             return true;
 
@@ -2263,7 +2256,7 @@ public class Grid<T> extends Composite implements
             // starting point
             if (diffX > GridConstants.LONG_TAP_THRESHOLD
                     || diffY > GridConstants.LONG_TAP_THRESHOLD) {
-                lazySorter.cancel();
+                sorter.cancelDelayedSort();
             }
 
             return true;
@@ -2273,11 +2266,10 @@ public class Grid<T> extends Composite implements
                 return false;
             }
 
-            if (lazySorter.isRunning()) {
+            if (sorter.isDelayedSortScheduled()) {
                 // Not a long tap yet, perform single sort
-                lazySorter.cancel();
-                lazySorter.setMultisort(false);
-                lazySorter.run();
+                sorter.cancelDelayedSort();
+                sorter.sort(cell, false);
             }
 
             return true;
@@ -2287,14 +2279,13 @@ public class Grid<T> extends Composite implements
                 return false;
             }
 
-            lazySorter.cancel();
+            sorter.cancelDelayedSort();
 
             return true;
 
         } else if (BrowserEvents.CLICK.equals(event.getType())) {
-            lazySorter.setCellReference(cell);
-            lazySorter.setMultisort(event.getShiftKey());
-            lazySorter.run();
+
+            sorter.sort(cell, event.getShiftKey());
 
             // Click events should go onward to active cell logic
             return false;
index 74a5c6ed9566bf5e38a60b2003f00b856a7cb2a5..acc5bfe51aa0e3f833dc190be0e15d302c41f2b9 100644 (file)
@@ -208,7 +208,7 @@ public class GridSortingTest extends GridBasicFeaturesTest {
     }
 
     @Test
-    public void testKeyboardMultiColumnSorting() throws InterruptedException {
+    public void testKeyboardSorting() {
         openTestURL();
 
         //
@@ -254,10 +254,17 @@ public class GridSortingTest extends GridBasicFeaturesTest {
         // Move back to the third column
         sendKeys(Keys.RIGHT);
 
-        // Reset sorting to third column, ASCENDING
+        // Set sorting to third column, ASCENDING
         sendKeys(Keys.ENTER);
         assertLog("10. Sort order: [Column 2 ASCENDING] by USER");
 
+        // Move to the fourth column
+        sendKeys(Keys.RIGHT);
+
+        // Make sure that single-column sorting also works as expected
+        sendKeys(Keys.ENTER);
+        assertLog("12. Sort order: [Column 3 ASCENDING] by USER");
+
     }
 
     private void sortBy(String column) {