Skip to content

Commit

Permalink
Merge pull request #1651 from ghiscoding/bugfix/hidden-copy-cells
Browse files Browse the repository at this point in the history 
fix: SlickCellExternalCopyManager should work w/hidden cols fixes #1634
  • Loading branch information
@ghiscoding
ghiscoding authored Aug 24, 2024
2 parents f89a840 + 7b48596 commit 52e9805
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 28 deletions.
7 changes: 3 additions & 4 deletions packages/common/src/core/__tests__/slickGrid.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4445,15 +4445,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 });
});
});

Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/core/slickGrid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4157,7 +4157,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; }

Expand Down Expand Up @@ -5116,7 +5116,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;
Expand Down
49 changes: 48 additions & 1 deletion packages/common/src/extensions/__tests__/slickCellExternalCopyManager.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
});
});
});
});
52 changes: 32 additions & 20 deletions packages/common/src/extensions/slickCellExternalCopyManager.ts
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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({
Expand All @@ -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++) {
Expand Down Expand Up @@ -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'));
Expand Down Expand Up @@ -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] = {};
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/extensions/slickCellSelectionModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 52e9805

Please sign in to comment.