fix: set columns order correctly with more than 10 columns (#2297)

vaadin · Aug 5, 2021 · 7f2408b · 7f2408b
1 parent 19bf4e7
commit 7f2408b
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 4 deletions.
diff --git a/packages/vaadin-grid/src/vaadin-grid-column-group.js b/packages/vaadin-grid/src/vaadin-grid-column-group.js
@@ -7,6 +7,7 @@ import { PolymerElement } from '@polymer/polymer/polymer-element.js';
 import { microTask } from '@polymer/polymer/lib/utils/async.js';
 import { FlattenedNodesObserver } from '@polymer/polymer/lib/utils/flattened-nodes-observer.js';
 import { ColumnBaseMixin } from './vaadin-grid-column.js';
+import { updateColumnOrders } from './vaadin-grid-helpers.js';
 
 /**
  * A `<vaadin-grid-column-group>` is used to make groups of columns in `<vaadin-grid>` and
@@ -156,16 +157,17 @@ class GridColumnGroupElement extends ColumnBaseMixin(PolymerElement) {
 
       // In an unlikely situation where a group has more than 9 child columns,
       // the child scope must have 1 digit less...
-      const childCountDigits = ~~(Math.log(rootColumns.length) / Math.log(Math.LN10)) + 1;
+      // Log^a_b = Ln(a)/Ln(b)
+      // Number of digits of a number is equal to floor(Log(number)_10) + 1
+      const childCountDigits = ~~(Math.log(rootColumns.length) / Math.LN10) + 1;
 
       // Final scope for the child columns needs to mind both factors.
       const scope = Math.pow(10, trailingZeros - childCountDigits);
 
       if (_rootColumns[0] && _rootColumns[0]._order) {
         _rootColumns.sort((a, b) => a._order - b._order);
       }
-
-      _rootColumns.forEach((column, index) => (column._order = order + (index + 1) * scope));
+      updateColumnOrders(_rootColumns, scope, order);
     }
   }
 

diff --git a/packages/vaadin-grid/src/vaadin-grid-column-reordering-mixin.js b/packages/vaadin-grid/src/vaadin-grid-column-reordering-mixin.js
@@ -5,6 +5,7 @@
  */
 import { GestureEventListeners } from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
 import { addListener } from '@polymer/polymer/lib/utils/gestures.js';
+import { updateColumnOrders } from './vaadin-grid-helpers.js';
 
 /**
  * @polymerMixin
@@ -275,7 +276,7 @@ export const ColumnReorderingMixin = (superClass) =>
       // Reset all column orders
       columnTree[0].forEach((column) => (column._order = 0));
       // Set order numbers to top-level columns
-      columnTree[0].forEach((column, index) => (column._order = (index + 1) * this._orderBaseScope));
+      updateColumnOrders(columnTree[0], this._orderBaseScope, 0);
     }
 
     /**

diff --git a/packages/vaadin-grid/src/vaadin-grid-helpers.js b/packages/vaadin-grid/src/vaadin-grid-helpers.js
@@ -0,0 +1,23 @@
+/**
+ * @license
+ * Copyright (c) 2021 Vaadin Ltd.
+ * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
+ */
+
+/**
+ * @param {Array<Object>} columns array of columns to be modified
+ * @param {number} scope multiplier added to base order for each column
+ * @param {number} baseOrder base number used for order
+ */
+export function updateColumnOrders(columns, scope, baseOrder) {
+  let c = 1;
+  columns.forEach((column) => {
+    // avoid multiples of 10 because they introduce and extra zero and
+    // causes the underlying calculations for child order goes wrong
+    if (c % 10 === 0) {
+      c++;
+    }
+    column._order = baseOrder + c * scope;
+    c++;
+  });
+}
diff --git a/packages/vaadin-grid/test/column-groups.test.js b/packages/vaadin-grid/test/column-groups.test.js
@@ -440,4 +440,63 @@ describe('column groups', () => {
       expect(columns[1].hidden).not.to.be.true;
     });
   });
+
+  describe('Large nested groups', () => {
+    let grid;
+
+    beforeEach(() => {
+      grid = fixtureSync(`
+        <vaadin-grid>
+          <vaadin-grid-column-group header="groupA">
+            ${Array.from(Array(13).keys())
+              .slice(1)
+              .map((group) => {
+                const col = (group - 1) * 2;
+                return `
+                  <vaadin-grid-column-group header="group${group - 1}">
+                    <vaadin-grid-column id="col${col}" header="Col ${col}"></vaadin-grid-column>
+                    <vaadin-grid-column id="col${col + 1}" header="Col ${col + 1}"></vaadin-grid-column>
+                  </vaadin-grid-column-group>
+                `;
+              })
+              .join('')}
+          </vaadin-grid-column-group>
+        </vaadin-grid>
+      `);
+    });
+
+    it('should correctly set column order', async () => {
+      let col19 = grid.querySelector('#col19');
+      let col20 = grid.querySelector('#col20');
+      await nextFrame();
+      expect(col19._order).to.be.lessThan(col20._order);
+    });
+  });
+
+  describe('More than 100 columns', () => {
+    let grid;
+
+    beforeEach(() => {
+      grid = fixtureSync(`
+        <vaadin-grid>
+          ${Array.from(Array(101).keys())
+            .map(
+              (col) => `
+              <vaadin-grid-column-group>
+                <vaadin-grid-column id="col${col}" header="Col ${col}"></vaadin-grid-column>
+              </vaadin-grid-column-group>
+              `
+            )
+            .join('')}
+        </vaadin-grid>
+      `);
+    });
+
+    it('should correctly set column order when we have more than 100 columns', async () => {
+      let col99 = grid.querySelector('#col99');
+      let col100 = grid.querySelector('#col100');
+      await nextFrame();
+      expect(col99._order).to.be.lessThan(col100._order);
+    });
+  });
 });