fix: swap columns iteratively on reorder (#2220)

* fix: swap columns iteratively on reorder

Cherry-pick of vaadin/web-components#4534

* replace usages of findIndex, includes

* try reduce column size

* Update src/vaadin-grid-column-reordering-mixin.html

Co-authored-by: Tomi Virkki <[email protected]>

Co-authored-by: Tomi Virkki <[email protected]>

Author: sissbruecker

src/vaadin-grid-column-reordering-mixin.html

@@ -167,7 +167,23 @@
       const targetColumn = this._getTargetColumn(targetCell, this._draggedColumn);
       if (this._isSwapAllowed(this._draggedColumn, targetColumn) &&
         this._isSwappableByPosition(targetColumn, e.detail.x)) {
-        this._swapColumnOrders(this._draggedColumn, targetColumn);
+        // Get the header level of the target column (and the dragged column)
+        const columnTreeLevel = this.__getColumnTreeLevel(targetColumn);
+        // Get the columns on that level in visual order
+        const levelColumnsInOrder = this._getColumnsInOrder(columnTreeLevel);
+
+        // Index of the column being dragged
+        const startIndex = levelColumnsInOrder.indexOf(this._draggedColumn);
+        // Index of the column being dragged over
+        const endIndex = levelColumnsInOrder.indexOf(targetColumn);
+
+        // Direction of iteration
+        const direction = startIndex < endIndex ? 1 : -1;
+
+        // Iteratively swap all the columns from the dragged column to the target column
+        for (let i = startIndex; i !== endIndex; i += direction) {
+          this._swapColumnOrders(this._draggedColumn, levelColumnsInOrder[i + direction]);
+        }
       }
 
       this._updateGhostPosition(e.detail.x, this._touchDevice ? e.detail.y - 50 : e.detail.y);
@@ -194,13 +210,17 @@
     }
 
     /**
+     * Returns the columns (or column groups) on the specified header level in visual order.
+     * By default, the bottom level is used.
+     *
      * @return {!Array<!GridColumnElement>}
      * @protected
      */
-    _getColumnsInOrder() {
-      return this._columnTree.slice(0).pop()
-        .filter(c => !c.hidden)
-        .sort((b, a) => (b._order - a._order));
+    _getColumnsInOrder(headerLevel) {
+      if (!headerLevel && headerLevel !== 0) {
+        headerLevel = this._columnTree.length - 1;
+      }
+      return this._columnTree[headerLevel].filter((c) => !c.hidden).sort((b, a) => b._order - a._order);
     }
 
     /**
@@ -403,6 +423,16 @@
       }
     }
 
+    /** @private */
+    __getColumnTreeLevel(column) {
+      for (let level = 0; level < this._columnTree.length; level++) {
+        if (this._columnTree[level].indexOf(column) >= 0) {
+          return level;
+        }
+      }
+      return -1;
+    }
+
     /**
      * Fired when the columns in the grid are reordered.
      *

test/column-reordering.html

@@ -41,15 +41,15 @@
 
   <test-fixture id="groups">
     <template>
-      <vaadin-grid style="width: 400px; height: 200px;" size="1" column-reordering-allowed>
-        <dom-repeat is="dom-repeat" items="[1, 2]" as="colgroup">
+      <vaadin-grid style="width: 800px; height: 200px;" size="1" column-reordering-allowed>
+        <dom-repeat is="dom-repeat" items="[1, 2, 3]" as="colgroup">
           <template>
             <vaadin-grid-column-group>
               <template class="header">[[colgroup]]</template>
               <template class="footer">[[colgroup]]</template>
               <dom-repeat is="dom-repeat" items="[1, 2]" as="col">
                 <template>
-                  <vaadin-grid-column>
+                  <vaadin-grid-column width="50px" flex-grow="0">
                     <template class="header">[[colgroup]][[col]]</template>
                     <template>[[colgroup]][[col]]</template>
                     <template class="footer">[[colgroup]][[col]]</template>
@@ -358,10 +358,32 @@
         });
 
         it('should reorder multiple columns while dragging', () => {
+          // Start column order [1, 2, 3, 4]
+
+          // Drag column 1 on top of column 2. Expected order [2, 1, 3, 4]
           dragOver(headerContent[0], headerContent[1]);
+          // Keep dragging the column all the way over to the last column. Expected order [2, 3, 4, 1]
           dragOver(headerContent[0], headerContent[3]);
           flushGrid(grid);
-          expectVisualOrder(grid, [2, 4, 3, 1]);
+          expectVisualOrder(grid, [2, 3, 4, 1]);
+        });
+
+        it('should shift columns in between when dragged as last', () => {
+          // Start column order [1, 2, 3, 4]
+
+          // Drag column 2 all the way over to the last column. Expected order [1, 3, 4, 2]
+          dragOver(headerContent[1], headerContent[3]);
+          flushGrid(grid);
+          expectVisualOrder(grid, [1, 3, 4, 2]);
+        });
+
+        it('should shift columns in between when dragged as first', () => {
+          // Start column order [1, 2, 3, 4]
+
+          // Drag column 3 over the very first column. Expected order [3, 1, 2, 4]
+          dragOver(headerContent[2], headerContent[0]);
+          flushGrid(grid);
+          expectVisualOrder(grid, [3, 1, 2, 4]);
         });
 
         it('should update first-column attribute', () => {
@@ -516,6 +538,20 @@
         expectVisualOrder(grid, [21, 22, 11, 12]);
       });
 
+      it('should reorder all the groups', () => {
+        // Start order
+        // [1,      2,      3     ] <- groups
+        // [11, 12, 21, 22, 31, 32] <- columns
+
+        // Drag the first group over the last group
+        dragAndDropOver(getVisualHeaderCellContent(grid, 0, 0), getVisualHeaderCellContent(grid, 0, 5));
+
+        // Expected order
+        // [2,      3,      1     ] <- groups
+        // [21, 22, 31, 32, 11, 12] <- columns
+        expectVisualOrder(grid, [21, 22, 31, 32, 11, 12]);
+      });
+
       it('should allow dropping group over other groups column header', () => {
         dragAndDropOver(
           getVisualHeaderCellContent(grid, 0, 0),