feat(grid): improve column reorder auto-scroll (#1286)

* feat(grid): improve column reorder auto-scroll

* chore: fix failing Cypress test

Author: ghiscoding

cypress/e2e/example-frozen-columns-reorder.cy.ts

@@ -147,46 +147,65 @@ describe('Example - Frozen Columns - Column Header Reorder (characterization)',
     cy.get(RIGHT_VIEWPORT).scrollTo(0, 0, { ensureScrollable: false });
   });
 
-  it('should auto-scroll the right viewport when a header drag starts beyond the right edge of the grid', () => {
+  it('should auto-scroll the right viewport when a header drag moves past the right edge of the grid', () => {
     cy.get(RIGHT_VIEWPORT).scrollTo(0, 0, { ensureScrollable: false });
     cy.wait(50);
     cy.get(RIGHT_VIEWPORT).should(($v) => expect($v[0].scrollLeft).to.eq(0));
 
-    // start a drag on "Finish" with the pointer already past the grid's right edge, then also fire a
-    // document-level `drag` event at those coordinates (that is what a real browser does continuously
-    // during a native drag, and what the future native reorder engine listens to for auto-scrolling)
+    // Start the drag inside the viewport first, then move past the grid's right edge via document-level
+    // drag events. This characterizes the live drag tracking behavior rather than a start-outside shortcut.
     cy.window().then((win: any) => {
       const finishHeader = getRightHeader(win, 'Finish');
       expect(finishHeader).to.exist;
       const rect = finishHeader.getBoundingClientRect();
+      const startX = rect.left + rect.width / 2;
       const sy = rect.top + rect.height / 2;
-      const dragX = (win.document.querySelector('#myGrid') as HTMLElement).clientWidth + 100;
       const dataTransfer = new DataTransfer();
 
-      pressPointer(finishHeader, rect.left + rect.width / 2, sy);
-      finishHeader.dispatchEvent(createDragLikeEvent('dragstart', dragX, sy, dataTransfer));
-      win.document.dispatchEvent(createDragLikeEvent('drag', dragX, sy, dataTransfer));
+      pressPointer(finishHeader, startX, sy);
+      finishHeader.dispatchEvent(createDragLikeEvent('dragstart', startX, sy, dataTransfer));
     });
 
-    cy.wait(250);
+    // SortableJS dispatches its start callback on the next macrotask. Yield so the
+    // grid can bind its document-level auto-scroll listeners before moving outside.
+    cy.wait(50);
     cy.window().then((win: any) => {
       const finishHeader = getRightHeader(win, 'Finish');
-      const sy = finishHeader.getBoundingClientRect().top;
-      const dragX = (win.document.querySelector('#myGrid') as HTMLElement).clientWidth + 100;
-      win.document.dispatchEvent(createDragLikeEvent('drag', dragX, sy, new DataTransfer()));
+      const rect = finishHeader.getBoundingClientRect();
+      const gridRect = (win.document.querySelector('#myGrid') as HTMLElement).getBoundingClientRect();
+      const sy = rect.top + rect.height / 2;
+      const dragX = gridRect.right + 100;
+      const dataTransfer = new DataTransfer();
+      win.document.dispatchEvent(createDragLikeEvent('drag', dragX, sy, dataTransfer));
+      win.document.dispatchEvent(createDragLikeEvent('mousemove', dragX, sy, dataTransfer));
     });
     cy.wait(250);
 
-    cy.get(RIGHT_VIEWPORT).should(($v) => expect($v[0].scrollLeft).to.be.greaterThan(10));
+    cy.window().then((win: any) => {
+      const finishHeader = getRightHeader(win, 'Finish');
+      const rect = finishHeader.getBoundingClientRect();
+      const sy = rect.top + rect.height / 2;
+      const safeX = rect.left + rect.width / 2;
+      const dataTransfer = new DataTransfer();
+      win.document.dispatchEvent(createDragLikeEvent('drag', safeX, sy, dataTransfer));
+      win.document.dispatchEvent(createDragLikeEvent('mousemove', safeX, sy, dataTransfer));
+    });
+
+    cy.get(RIGHT_VIEWPORT).then(($v) => {
+      expect($v[0].scrollLeft).to.be.greaterThan(10);
+      const scrollLeftAfterSafeZone = $v[0].scrollLeft;
+      cy.wait(250);
+      cy.get(RIGHT_VIEWPORT).should(($v2) => expect($v2[0].scrollLeft).to.eq(scrollLeftAfterSafeZone));
+    });
 
-    // end the drag on the source itself: no reorder, and the auto-scroll must stop
+    // end the drag on the source itself: no reorder, and the auto-scroll must remain stopped
     cy.window().then((win: any) => {
       const finishHeader = getRightHeader(win, 'Finish');
       const rect = finishHeader.getBoundingClientRect();
       const sy = rect.top + rect.height / 2;
-      const dragX = (win.document.querySelector('#myGrid') as HTMLElement).clientWidth + 100;
-      finishHeader.dispatchEvent(createDragLikeEvent('dragend', dragX, sy, new DataTransfer()));
-      releasePointer(finishHeader, dragX, sy);
+      const safeX = rect.left + rect.width / 2;
+      finishHeader.dispatchEvent(createDragLikeEvent('dragend', safeX, sy, new DataTransfer()));
+      releasePointer(finishHeader, safeX, sy);
     });
 
     cy.get(RIGHT_VIEWPORT).then(($v) => {

cypress/e2e/example14-column-reorder.cy.ts

@@ -137,7 +137,7 @@ describe('Example 14 - Column Header Reorder (characterization)', { retries: 1 }
   // get no header element rendered and the SortableJS toArray() read-back therefore omits them.
   // The native reorder engine (PR 2 of the SortableJS removal) fixes this with column-map reconciliation.
   // Enable this test when that engine lands.
-  it.skip('should keep hidden columns in the column set when reordering', () => {
+  it('should keep hidden columns in the column set when reordering', () => {
     cy.window().then((win: any) => {
       const cols = win.grid.getColumns();
       cols[2].hidden = true; // hide "CPU1"

src/slick.grid.ts

@@ -1951,13 +1951,41 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
     this.sortableSideLeftInstance?.destroy();
     this.sortableSideRightInstance?.destroy();
 
-    let columnScrollTimer: any = null;
-
-    const scrollColumnsRight = () => this._viewportScrollContainerX.scrollLeft = this._viewportScrollContainerX.scrollLeft + 10;
-    const scrollColumnsLeft = () => this._viewportScrollContainerX.scrollLeft = this._viewportScrollContainerX.scrollLeft - 10;
+    let columnScrollTimer: number | undefined;
+    let columnScrollDirection = 0;
     let prevColumnIds: Array<string | number> = [];
+    let hiddenColumns = new Map<string | number, C>();
+
+    const stopAutoScroll = () => {
+      if (columnScrollTimer) {
+        clearInterval(columnScrollTimer);
+        columnScrollTimer = undefined;
+      }
+      columnScrollDirection = 0;
+    };
+
+    const autoScrollHandler = (event: DragEvent | MouseEvent) => {
+      const { clientX, clientY, pageX } = event;
+      if (!clientX || !clientY) {
+        return;
+      }
+
+      const viewportLeft = Utils.offset(this._viewportScrollContainerX)!.left;
+      const containerRight = Utils.offset(this._container)!.left + this._container.clientWidth;
+      const direction = pageX > containerRight ? 1 : pageX < viewportLeft ? -1 : 0;
+
+      if (direction !== columnScrollDirection) {
+        stopAutoScroll();
+        columnScrollDirection = direction;
+
+        if (direction) {
+          columnScrollTimer = window.setInterval(() => {
+            this._viewportScrollContainerX.scrollLeft += direction * 10;
+          }, 30);
+        }
+      }
+    };
 
-    let canDragScroll = false;
     const sortableOptions = {
       animation: 50,
       direction: 'horizontal',
@@ -1975,42 +2003,38 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
       },
       onStart: (e: SortableEvent) => {
         e.item.classList.add('slick-header-column-active');
-        canDragScroll = !this.hasFrozenColumns() || Utils.offset(e.item)!.left > Utils.offset(this._viewportScrollContainerX)!.left;
+        this._bindingEventService.unbindAll('colreorder');
+        stopAutoScroll();
 
-        if (canDragScroll && e.originalEvent.pageX > this._container.clientWidth) {
-          if (!(columnScrollTimer)) {
-            columnScrollTimer = window.setInterval(scrollColumnsRight, 100);
-          }
-        } else if (canDragScroll && e.originalEvent.pageX < Utils.offset(this._viewportScrollContainerX)!.left) {
-          if (!(columnScrollTimer)) {
-            columnScrollTimer = window.setInterval(scrollColumnsLeft, 100);
-          }
-        } else {
-          window.clearInterval(columnScrollTimer);
-          columnScrollTimer = null;
+        if (!this.hasFrozenColumns() || this._headerR.contains(e.item)) {
+          this._bindingEventService.bind(document, 'drag', autoScrollHandler as EventListener, {}, 'colreorder');
+          this._bindingEventService.bind(document, 'mousemove', autoScrollHandler as EventListener, {}, 'colreorder');
         }
+
         prevColumnIds = this.columns.map((c) => c.id);
+        hiddenColumns = new Map(this.columns.filter((column) => column.hidden).map((column) => [column.id, column] as [string | number, C]));
       },
       onEnd: (e: SortableEvent) => {
         e.item.classList.remove('slick-header-column-active');
-        clearInterval(columnScrollTimer);
+        stopAutoScroll();
+        this._bindingEventService.unbindAll('colreorder');
         const prevScrollLeft = this.scrollLeft;
 
         if (!this.getEditorLock()?.commitCurrentEdit()) {
           return;
         }
 
-        let reorderedIds = this.sortableSideLeftInstance?.toArray() ?? [];
-        reorderedIds = reorderedIds.concat(this.sortableSideRightInstance?.toArray() ?? []);
-
-        const reorderedColumns: C[] = [];
-        for (let i = 0; i < reorderedIds.length; i++) {
-          reorderedColumns.push(this.columns[this.getColumnIndex(reorderedIds[i])]);
-        }
+        const reorderedIds = [
+          ...(this.sortableSideLeftInstance?.toArray() ?? []),
+          ...(this.sortableSideRightInstance?.toArray() ?? []),
+        ];
+        const reorderedColumns = reorderedIds.map((columnId) => this.columns[this.getColumnIndex(columnId)]);
+        let visibleColumnIdx = 0;
+        const finalColumns = this.columns.map((column) => hiddenColumns.get(column.id) ?? reorderedColumns[visibleColumnIdx++]);
 
         e.stopPropagation();
         if (!this.arrayEquals(prevColumnIds, reorderedIds)) {
-          this.setColumns(reorderedColumns);
+          this.setColumns(finalColumns);
           // reapply previous scroll position since it might move back to x=0 after calling `setColumns()` (especially when `frozenColumn` is set)
           this.scrollToX(prevScrollLeft);
           this.trigger(this.onColumnsReordered, { impactedColumns: this.columns, previousColumnOrder: prevColumnIds });