fix: SlickCellExternalCopyManager should work w/hidden cols fixes #1634

packages/common/src/core/__tests__/slickGrid.spec.ts

@@ -4392,15 +4392,14 @@ describe('SlickGrid core file', () => {
         expect(result5).toEqual({ cell: -1, row: -1 });
       });
 
-      it('should skip a cell when column found at x/y coordinates is hidden (Gender) so cell will the last known visible column', () => {
+      it('should return hidden cells as well since skipping them would add an unwanted offset', () => {
         grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });
 
         const result1 = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 5) + 5, (DEFAULT_COLUMN_HEIGHT * 5) + 5);
         const result2 = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 6) + 5, (DEFAULT_COLUMN_HEIGHT * 6) + 5);
 
-        // last 2 columns are hidden and last visible column is cell:4
-        expect(result1).toEqual({ cell: 4, row: 5 });
-        expect(result2).toEqual({ cell: 4, row: 6 });
+        expect(result1).toEqual({ cell: 5, row: 5 });
+        expect(result2).toEqual({ cell: 6, row: 6 });
       });
     });
 

packages/common/src/core/slickGrid.ts

@@ -4150,7 +4150,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
 
       const d = this.getDataItem(row);
 
-      // TODO:  shorten this loop (index? heuristics? binary search?)
+      // TODO: shorten this loop (index? heuristics? binary search?)
       for (let i = 0, ii = this.columns.length; i < ii; i++) {
         if (!this.columns[i] || this.columns[i].hidden) { continue; }
 
@@ -5104,7 +5104,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
 
     let w = 0;
     for (let i = 0; i < this.columns.length && w <= x; i++) {
-      if (!this.columns[i] || this.columns[i].hidden) {
+      if (!this.columns[i]) {
         continue;
       }
       w += this.columns[i].width as number;

packages/common/src/extensions/__tests__/slickCellExternalCopyManager.spec.ts

@@ -35,7 +35,7 @@ const gridStub = {
   getColumns: jest.fn().mockReturnValue([
     { id: 'firstName', field: 'firstName', name: 'First Name', },
     { id: 'lastName', field: 'lastName', name: 'Last Name' },
-  ]),
+  ] as Column[]),
   getData: () => dataViewStub,
   getDataItem: jest.fn(),
   getDataLength: jest.fn(),
@@ -688,6 +688,53 @@ describe('CellExternalCopyManager', () => {
           done();
         }, 2);
       });
+
+      it('should copy selection but skip hidden column and then use window.clipboard when exist and Paste is performed', (done) => {
+        let clipCommand;
+        const clipboardCommandHandler = (cmd) => {
+          clipCommand = cmd;
+          cmd.execute();
+        };
+        const mockColumns = [
+          { id: 'firstName', field: 'firstName', name: 'First Name', editor: { model: Editors.text }, editorClass: Editors.text },
+          { id: 'lastName', field: 'lastName', name: lastNameElm, hidden: true, },
+          { id: 'age', field: 'age', name: 'Age', editor: { model: Editors.text }, editorClass: Editors.text },
+          { id: 'gender', field: 'gender', name: 'Gender' },
+        ] as Column[];
+        jest.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
+        jest.spyOn(gridStub.getSelectionModel() as SelectionModel, 'getSelectedRanges').mockReturnValueOnce([new SlickRange(0, 1, 1, 2)]).mockReturnValueOnce(null as any);
+        plugin.init(gridStub, { clipboardPasteDelay: 1, clearCopySelectionDelay: 1, includeHeaderWhenCopying: true, clipboardCommandHandler });
+
+        const keyDownCtrlCopyEvent = new Event('keydown');
+        Object.defineProperty(keyDownCtrlCopyEvent, 'ctrlKey', { writable: true, configurable: true, value: true });
+        Object.defineProperty(keyDownCtrlCopyEvent, 'key', { writable: true, configurable: true, value: 'c' });
+        Object.defineProperty(keyDownCtrlCopyEvent, 'isPropagationStopped', { writable: true, configurable: true, value: jest.fn() });
+        Object.defineProperty(keyDownCtrlCopyEvent, 'isImmediatePropagationStopped', { writable: true, configurable: true, value: jest.fn() });
+        gridStub.onKeyDown.notify({ cell: 0, row: 0, grid: gridStub }, keyDownCtrlCopyEvent, gridStub);
+
+        const updateCellSpy = jest.spyOn(gridStub, 'updateCell');
+        const onCellChangeSpy = jest.spyOn(gridStub.onCellChange, 'notify');
+        const getActiveCellSpy = jest.spyOn(gridStub, 'getActiveCell').mockReturnValue({ cell: 0, row: 1 });
+        const keyDownCtrlPasteEvent = new Event('keydown');
+        jest.spyOn(gridStub, 'getColumns').mockReturnValue(mockColumns);
+        Object.defineProperty(keyDownCtrlPasteEvent, 'ctrlKey', { writable: true, configurable: true, value: true });
+        Object.defineProperty(keyDownCtrlPasteEvent, 'key', { writable: true, configurable: true, value: 'v' });
+        Object.defineProperty(keyDownCtrlPasteEvent, 'isPropagationStopped', { writable: true, configurable: true, value: jest.fn() });
+        Object.defineProperty(keyDownCtrlPasteEvent, 'isImmediatePropagationStopped', { writable: true, configurable: true, value: jest.fn() });
+        gridStub.onKeyDown.notify({ cell: 0, row: 0, grid: gridStub }, keyDownCtrlPasteEvent, gridStub);
+        document.querySelector('textarea')!.value = `Doe\tserialized output`;
+
+        setTimeout(() => {
+          expect(getActiveCellSpy).toHaveBeenCalled();
+          expect(updateCellSpy).toHaveBeenCalledWith(1, 0);
+          expect(updateCellSpy).not.toHaveBeenCalledWith(1, 1);
+          expect(onCellChangeSpy).toHaveBeenCalledWith({ row: 1, cell: 2, item: { firstName: 'John', lastName: 'Doe' }, grid: gridStub, column: {} });
+          const getDataItemSpy = jest.spyOn(gridStub, 'getDataItem');
+          clipCommand.undo();
+          expect(getDataItemSpy).toHaveBeenCalled();
+          done();
+        }, 2);
+      });
     });
   });
 });

packages/common/src/extensions/slickCellExternalCopyManager.ts

@@ -1,7 +1,7 @@
 import { createDomElement, getHtmlStringOutput, stripTags } from '@slickgrid-universal/utils';
 
 import { DataWrapperService } from '../services/dataWrapperService';
-import type { Column, Editor, EditorConstructor, ElementPosition, ExcelCopyBufferOption, ExternalCopyClipCommand, OnEventArgs } from '../interfaces/index';
+import type { Column, CssStyleHash, Editor, EditorConstructor, ElementPosition, ExcelCopyBufferOption, ExternalCopyClipCommand, OnEventArgs } from '../interfaces/index';
 import { type SlickDataView, SlickEvent, SlickEventData, SlickEventHandler, type SlickGrid, SlickRange, Utils as SlickUtils } from '../core/index';
 
 // using external SlickGrid JS libraries
@@ -311,31 +311,40 @@ export class SlickCellExternalCopyManager {
       maxDestX: this._grid.getColumns().length,
       h: 0,
       w: 0,
-
       execute: () => {
         clipCommand.h = 0;
         for (let y = 0; y < clipCommand.destH; y++) {
           clipCommand.oldValues[y] = [];
           clipCommand.w = 0;
           clipCommand.h++;
+          let xOffset = 0; // the x offset for hidden col
+
           for (let x = 0; x < clipCommand.destW; x++) {
-            clipCommand.w++;
             const desty = activeRow + y;
             const destx = activeCell + x;
+            const column = columns[destx];
+
+            // paste on hidden column will be skipped, but we need to paste 1 cell further on X axis
+            // we'll increase our X and increase the offset`
+            if (column.hidden) {
+              clipCommand.destW++;
+              xOffset++;
+              continue;
+            }
+            clipCommand.w++;
 
             if (desty < clipCommand.maxDestY && destx < clipCommand.maxDestX) {
-              // const nd = this._grid.getCellNode(desty, destx);
               const dt = this._dataWrapper.getDataItem(desty);
 
-              if (this._grid.triggerEvent(this.onBeforePasteCell, { row: desty, cell: destx, dt, column: columns[destx], target: 'grid' }).getReturnValue() === false) {
+              if (this._grid.triggerEvent(this.onBeforePasteCell, { row: desty, cell: destx, dt, column, target: 'grid' }).getReturnValue() === false) {
                 continue;
               }
 
-              clipCommand.oldValues[y][x] = dt[columns[destx]['field']];
+              clipCommand.oldValues[y][x - xOffset] = dt[column['field']];
               if (oneCellToMultiple) {
-                this.setDataItemValueForColumn(dt, columns[destx], clippedRange[0][0]);
+                this.setDataItemValueForColumn(dt, column, clippedRange[0][0]);
               } else {
-                this.setDataItemValueForColumn(dt, columns[destx], clippedRange[y] ? clippedRange[y][x] : '');
+                this.setDataItemValueForColumn(dt, column, clippedRange[y] ? clippedRange[y][x - xOffset] : '');
               }
               this._grid.updateCell(desty, destx);
               this._grid.onCellChange.notify({
@@ -359,7 +368,6 @@ export class SlickCellExternalCopyManager {
         this._grid.getSelectionModel()?.setSelectedRanges([bRange]);
         this.onPasteCells.notify({ ranges: [bRange] });
       },
-
       undo: () => {
         for (let y = 0; y < clipCommand.destH; y++) {
           for (let x = 0; x < clipCommand.destW; x++) {
@@ -459,22 +467,26 @@ export class SlickCellExternalCopyManager {
               if (clipTextRows.length === 0 && this._addonOptions.includeHeaderWhenCopying) {
                 const clipTextHeaders: string[] = [];
                 for (let j = range.fromCell; j < range.toCell + 1; j++) {
-                  const colName: string = columns[j].name instanceof HTMLElement
-                    ? stripTags((columns[j].name as HTMLElement).innerHTML)
-                    : columns[j].name as string;
-                  if (colName.length > 0 && !columns[j].hidden) {
-                    clipTextHeaders.push(this.getHeaderValueForColumn(columns[j]));
+                  if (columns[j]) {
+                    const colName: string = columns[j].name instanceof HTMLElement
+                      ? stripTags((columns[j].name as HTMLElement).innerHTML)
+                      : columns[j].name as string;
+                    if (colName.length > 0 && !columns[j].hidden) {
+                      clipTextHeaders.push(this.getHeaderValueForColumn(columns[j]));
+                    }
                   }
                 }
                 clipTextRows.push(clipTextHeaders.join('\t'));
               }
 
               for (let j = range.fromCell; j < range.toCell + 1; j++) {
-                const colName: string = columns[j].name instanceof HTMLElement
-                  ? stripTags((columns[j].name as HTMLElement).innerHTML)
-                  : columns[j].name as string;
-                if (colName.length > 0 && !columns[j].hidden) {
-                  clipTextCells.push(this.getDataItemValueForColumn(dt, columns[j], i, j, e));
+                if (columns[j]) {
+                  const colName: string = columns[j].name instanceof HTMLElement
+                    ? stripTags((columns[j].name as HTMLElement).innerHTML)
+                    : columns[j].name as string;
+                  if (colName.length > 0 && !columns[j].hidden) {
+                    clipTextCells.push(this.getDataItemValueForColumn(dt, columns[j], i, j, e));
+                  }
                 }
               }
               clipTextRows.push(clipTextCells.join('\t'));
@@ -522,7 +534,7 @@ export class SlickCellExternalCopyManager {
     this.clearCopySelection();
 
     const columns = this._grid.getColumns();
-    const hash: any = {};
+    const hash: CssStyleHash = {};
     for (const range of ranges) {
       for (let j = range.fromRow; j <= range.toRow; j++) {
         hash[j] = {};

packages/common/src/extensions/slickCellSelectionModel.ts

@@ -33,7 +33,7 @@ export class SlickCellSelectionModel implements SelectionModel {
 
     this._selector = (options === undefined || options.cellRangeSelector === undefined)
       ? new SlickCellRangeSelector({ selectionCss: { border: '2px solid black' } as CSSStyleDeclaration })
-      : this._selector = options.cellRangeSelector;
+      : options.cellRangeSelector;
 
     this._addonOptions = options;
   }