Preserve selection order for multi select value (#8109, #8787)

svn changeset:24030/svn branch:6.8

2 files changed, 20 insertions, 14 deletions

diff --git a/src/com/vaadin/ui/AbstractSelect.java b/src/com/vaadin/ui/AbstractSelect.java
index bb49626741..71624804fa 100644
--- a/src/com/vaadin/ui/AbstractSelect.java
+++ b/src/com/vaadin/ui/AbstractSelect.java
@@ -426,12 +426,15 @@ public abstract class AbstractSelect extends AbstractField implements
                 // (non-visible items can not be deselected)
                 final Collection<?> visible = getVisibleItemIds();
                 if (visible != null) {
+                    // Don't remove those that will be added to preserve order
+                    visible.removeAll(s);
+
                     @SuppressWarnings("unchecked")
                     Set<Object> newsel = (Set<Object>) getValue();
                     if (newsel == null) {
-                        newsel = new HashSet<Object>();
+                        newsel = new LinkedHashSet<Object>();
                     } else {
-                        newsel = new HashSet<Object>(newsel);
+                        newsel = new LinkedHashSet<Object>(newsel);
                     }
                     newsel.removeAll(visible);
                     newsel.addAll(s);
@@ -645,10 +648,10 @@ public abstract class AbstractSelect extends AbstractField implements
 
         if (isMultiSelect()) {
             if (newValue == null) {
-                super.setValue(new HashSet<Object>(), repaintIsNotNeeded);
+                super.setValue(new LinkedHashSet<Object>(), repaintIsNotNeeded);
             } else if (Collection.class.isAssignableFrom(newValue.getClass())) {
-                super.setValue(new HashSet<Object>((Collection<?>) newValue),
-                        repaintIsNotNeeded);
+                super.setValue(new LinkedHashSet<Object>(
+                        (Collection<?>) newValue), repaintIsNotNeeded);
             }
         } else if (newValue == null || items.containsId(newValue)) {
             super.setValue(newValue, repaintIsNotNeeded);
diff --git a/src/com/vaadin/ui/Table.java b/src/com/vaadin/ui/Table.java
index 60b6122270..c4d90d7790 100644
--- a/src/com/vaadin/ui/Table.java
+++ b/src/com/vaadin/ui/Table.java
@@ -2295,7 +2295,7 @@ public class Table extends AbstractSelect implements Action.Container,
      * @return
      */
     private Set<Object> getItemIdsInRange(Object itemId, final int length) {
-        HashSet<Object> ids = new HashSet<Object>();
+        HashSet<Object> ids = new LinkedHashSet<Object>();
         for (int i = 0; i < length; i++) {
             assert itemId != null; // should not be null unless client-server
                                    // are out of sync
@@ -2318,18 +2318,12 @@ public class Table extends AbstractSelect implements Action.Container,
         Set<Object> renderedItemIds = getCurrentlyRenderedItemIds();
 
         @SuppressWarnings("unchecked")
-        HashSet<Object> newValue = new HashSet<Object>(
+        HashSet<Object> newValue = new LinkedHashSet<Object>(
                 (Collection<Object>) getValue());
 
         if (variables.containsKey("clearSelections")) {
             // the client side has instructed to swipe all previous selections
             newValue.clear();
-        } else {
-            /*
-             * first clear all selections that are currently rendered rows (the
-             * ones that the client side counterpart is aware of)
-             */
-            newValue.removeAll(renderedItemIds);
         }
 
         /*
@@ -2346,6 +2340,7 @@ public class Table extends AbstractSelect implements Action.Container,
                 requestRepaint();
             } else if (id != null && containsId(id)) {
                 newValue.add(id);
+                renderedItemIds.remove(id);
             }
         }
 
@@ -2355,9 +2350,17 @@ public class Table extends AbstractSelect implements Action.Container,
                 String[] split = range.split("-");
                 Object startItemId = itemIdMapper.get(split[0]);
                 int length = Integer.valueOf(split[1]);
-                newValue.addAll(getItemIdsInRange(startItemId, length));
+                Set<Object> itemIdsInRange = getItemIdsInRange(startItemId,
+                        length);
+                newValue.addAll(itemIdsInRange);
+                renderedItemIds.removeAll(itemIdsInRange);
             }
         }
+        /*
+         * finally clear all currently rendered rows (the ones that the client
+         * side counterpart is aware of) that the client didn't send as selected
+         */
+        newValue.removeAll(renderedItemIds);
 
         if (!isNullSelectionAllowed() && newValue.isEmpty()) {
             // empty selection not allowed, keep old value