From 4b7292989aaaf30c1b9f9715b4ff72acb4c64187 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Wed, 16 Aug 2023 23:42:58 -0400 Subject: [PATCH] fix: copying multiple times only kept last undo CellExternalCopyManager - fix an issue where copying and pasting to multiple area only kept the last undo and reapplied it over and over and the cause was because the clipCommand was global to the class so it only kept the last undo, the fix is to make sure the clipCommand variable is local to the execution handler instead --- .../slickCellExternalCopyManager.spec.ts | 35 +++++++++----- .../slickCellExternalCopyManager.ts | 47 +++++++++---------- pnpm-lock.yaml | 2 +- 3 files changed, 46 insertions(+), 38 deletions(-) diff --git a/packages/common/src/extensions/__tests__/slickCellExternalCopyManager.spec.ts b/packages/common/src/extensions/__tests__/slickCellExternalCopyManager.spec.ts index d9569049b..f925dac18 100644 --- a/packages/common/src/extensions/__tests__/slickCellExternalCopyManager.spec.ts +++ b/packages/common/src/extensions/__tests__/slickCellExternalCopyManager.spec.ts @@ -289,9 +289,13 @@ describe('CellExternalCopyManager', () => { }); it('should Copy, Paste and run Execute clip command', (done) => { - jest.spyOn(gridStub.getSelectionModel(), 'getSelectedRanges').mockReturnValueOnce([{ fromRow: 0, fromCell: 1, toRow: 1, toCell: 2 }]).mockReturnValueOnce(null); - - plugin.init(gridStub, { clipboardPasteDelay: 1, clearCopySelectionDelay: 1, includeHeaderWhenCopying: true, }); + let clipCommand; + const clipboardCommandHandler = (cmd) => { + clipCommand = cmd; + cmd.execute(); + }; + jest.spyOn(gridStub.getSelectionModel() as SelectionModel, 'getSelectedRanges').mockReturnValueOnce([new Slick.Range(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 }); @@ -318,16 +322,21 @@ describe('CellExternalCopyManager', () => { expect(updateCellSpy).toHaveBeenCalledWith(1, 1); expect(onCellChangeSpy).toHaveBeenCalledWith({ row: 1, cell: 0, item: { firstName: 'John', lastName: 'serialized output' }, grid: gridStub, column: {} }); const getDataItemSpy = jest.spyOn(gridStub, 'getDataItem'); - plugin.clipCommand.undo(); + clipCommand.undo(); expect(getDataItemSpy).toHaveBeenCalled(); done(); }, 2); }); it('should Copy, Paste and run Execute clip command with only 1 cell to copy', (done) => { - jest.spyOn(gridStub.getSelectionModel(), 'getSelectedRanges').mockReturnValueOnce([{ fromRow: 0, fromCell: 1, toRow: 1, toCell: 2 }]).mockReturnValueOnce([{ fromRow: 0, fromCell: 1, toRow: 1, toCell: 2 }]); + jest.spyOn(gridStub.getSelectionModel() as SelectionModel, 'getSelectedRanges').mockReturnValueOnce([new Slick.Range(0, 1, 1, 2)]).mockReturnValueOnce([new Slick.Range(0, 1, 1, 2)]); + let clipCommand; + const clipboardCommandHandler = (cmd) => { + clipCommand = cmd; + cmd.execute(); + }; - plugin.init(gridStub, { clipboardPasteDelay: 1, clearCopySelectionDelay: 1, includeHeaderWhenCopying: true, }); + 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 }); @@ -358,7 +367,7 @@ describe('CellExternalCopyManager', () => { const updateCell2Spy = jest.spyOn(gridStub, 'updateCell'); const onCellChange2Spy = jest.spyOn(gridStub.onCellChange, 'notify'); const setDataItemValSpy = jest.spyOn(plugin, 'setDataItemValueForColumn'); - plugin.clipCommand.undo(); + clipCommand.undo(); expect(getDataItemSpy).toHaveBeenCalled(); expect(updateCell2Spy).toHaveBeenCalled(); expect(onCellChangeSpy).toHaveBeenCalled(); @@ -434,8 +443,12 @@ describe('CellExternalCopyManager', () => { const renderSpy = jest.spyOn(gridStub, 'render'); const setDataSpy = jest.spyOn(gridStub, 'setData'); jest.spyOn(gridStub.getSelectionModel(), 'getSelectedRanges').mockReturnValueOnce([{ fromRow: 0, fromCell: 1, toRow: 2, toCell: 2 }]).mockReturnValueOnce(null); - - plugin.init(gridStub, { clearCopySelectionDelay: 1, clipboardPasteDelay: 1, includeHeaderWhenCopying: true, newRowCreator: mockNewRowCreator, onPasteCells: mockOnPasteCells }); + let clipCommand; + const clipboardCommandHandler = (cmd) => { + clipCommand = cmd; + cmd.execute(); + }; + plugin.init(gridStub, { clearCopySelectionDelay: 1, clipboardPasteDelay: 1, includeHeaderWhenCopying: true, clipboardCommandHandler, newRowCreator: mockNewRowCreator, onPasteCells: mockOnPasteCells }); const keyDownCtrlCopyEvent = new Event('keydown'); Object.defineProperty(keyDownCtrlCopyEvent, 'ctrlKey', { writable: true, configurable: true, value: true }); @@ -452,7 +465,7 @@ describe('CellExternalCopyManager', () => { 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`; + document.querySelector('textarea')!.value = `Doe\tserialized output`; setTimeout(() => { expect(getActiveCellSpy).toHaveBeenCalled(); @@ -463,7 +476,7 @@ describe('CellExternalCopyManager', () => { const getDataItemSpy = jest.spyOn(gridStub, 'getDataItem'); const setData2Spy = jest.spyOn(gridStub, 'setData'); const render2Spy = jest.spyOn(gridStub, 'render'); - plugin.clipCommand.undo(); + clipCommand.undo(); expect(getDataItemSpy).toHaveBeenCalled(); expect(setData2Spy).toHaveBeenCalledWith([{ firstName: 'John', lastName: 'Doe', age: 30 }, { firstName: 'Jane', lastName: 'Doe' }]); expect(render2Spy).toHaveBeenCalled(); diff --git a/packages/common/src/extensions/slickCellExternalCopyManager.ts b/packages/common/src/extensions/slickCellExternalCopyManager.ts index 3d3d557f9..d3f4d9499 100644 --- a/packages/common/src/extensions/slickCellExternalCopyManager.ts +++ b/packages/common/src/extensions/slickCellExternalCopyManager.ts @@ -27,7 +27,6 @@ export class SlickCellExternalCopyManager { protected _addonOptions!: ExcelCopyBufferOption; protected _bodyElement = document.body; protected _clearCopyTI?: NodeJS.Timeout; - protected _clipCommand!: ExternalCopyClipCommand; protected _copiedCellStyle = 'copied'; protected _copiedCellStyleLayerKey = 'copy-manager'; protected _copiedRanges: CellRange[] | null = null; @@ -48,10 +47,6 @@ export class SlickCellExternalCopyManager { return this._addonOptions; } - get clipCommand(): ExternalCopyClipCommand { - return this._clipCommand; - } - get eventHandler(): SlickEventHandler { return this._eventHandler; } @@ -238,7 +233,7 @@ export class SlickCellExternalCopyManager { this._addonOptions.newRowCreator(newRowsNeeded); } - this._clipCommand = { + const clipCommand: ExternalCopyClipCommand = { isClipboardCommand: true, clippedRange, oldValues: [], @@ -257,20 +252,20 @@ export class SlickCellExternalCopyManager { w: 0, execute: () => { - this._clipCommand.h = 0; - for (let y = 0; y < this._clipCommand.destH; y++) { - this._clipCommand.oldValues[y] = []; - this._clipCommand.w = 0; - this._clipCommand.h++; - for (let x = 0; x < this._clipCommand.destW; x++) { - this._clipCommand.w++; + clipCommand.h = 0; + for (let y = 0; y < clipCommand.destH; y++) { + clipCommand.oldValues[y] = []; + clipCommand.w = 0; + clipCommand.h++; + for (let x = 0; x < clipCommand.destW; x++) { + clipCommand.w++; const desty = activeRow + y; const destx = activeCell + x; - if (desty < this._clipCommand.maxDestY && destx < this._clipCommand.maxDestX) { + if (desty < clipCommand.maxDestY && destx < clipCommand.maxDestX) { // const nd = this._grid.getCellNode(desty, destx); const dt = this._grid.getDataItem(desty); - this._clipCommand.oldValues[y][x] = dt[columns[destx]['field']]; + clipCommand.oldValues[y][x] = dt[columns[destx]['field']]; if (oneCellToMultiple) { this.setDataItemValueForColumn(dt, columns[destx], clippedRange[0][0]); } else { @@ -291,8 +286,8 @@ export class SlickCellExternalCopyManager { const bRange = { fromCell: activeCell, fromRow: activeRow, - toCell: activeCell + this._clipCommand.w - 1, - toRow: activeRow + this._clipCommand.h - 1 + toCell: activeCell + clipCommand.w - 1, + toRow: activeRow + clipCommand.h - 1 }; this.markCopySelection([bRange]); this._grid.getSelectionModel()?.setSelectedRanges([bRange]); @@ -300,18 +295,18 @@ export class SlickCellExternalCopyManager { }, undo: () => { - for (let y = 0; y < this._clipCommand.destH; y++) { - for (let x = 0; x < this._clipCommand.destW; x++) { + for (let y = 0; y < clipCommand.destH; y++) { + for (let x = 0; x < clipCommand.destW; x++) { const desty = activeRow + y; const destx = activeCell + x; - if (desty < this._clipCommand.maxDestY && destx < this._clipCommand.maxDestX) { + if (desty < clipCommand.maxDestY && destx < clipCommand.maxDestX) { // const nd = this._grid.getCellNode(desty, destx); const dt = this._grid.getDataItem(desty); if (oneCellToMultiple) { - this.setDataItemValueForColumn(dt, columns[destx], this._clipCommand.oldValues[0][0]); + this.setDataItemValueForColumn(dt, columns[destx], clipCommand.oldValues[0][0]); } else { - this.setDataItemValueForColumn(dt, columns[destx], this._clipCommand.oldValues[y][x]); + this.setDataItemValueForColumn(dt, columns[destx], clipCommand.oldValues[y][x]); } this._grid.updateCell(desty, destx); this._grid.onCellChange.notify({ @@ -328,8 +323,8 @@ export class SlickCellExternalCopyManager { const bRange = { fromCell: activeCell, fromRow: activeRow, - toCell: activeCell + this._clipCommand.w - 1, - toRow: activeRow + this._clipCommand.h - 1 + toCell: activeCell + clipCommand.w - 1, + toRow: activeRow + clipCommand.h - 1 }; this.markCopySelection([bRange]); @@ -351,9 +346,9 @@ export class SlickCellExternalCopyManager { }; if (this._addonOptions.clipboardCommandHandler) { - this._addonOptions.clipboardCommandHandler(this._clipCommand); + this._addonOptions.clipboardCommandHandler(clipCommand); } else { - this._clipCommand.execute(); + clipCommand.execute(); } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 279f01ae9..dd5cce7a9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1,4 +1,4 @@ -lockfileVersion: '6.0' +lockfileVersion: '6.1' settings: autoInstallPeers: true