Skip to content

Commit 1dc8b76

Browse files
ghiscodingghiscoding
authored
fix(common): Excel copy cell ranges shouldn't lose its cell focus (#901)
* fix(common): Excel copy cell ranges shouldn't lose its cell focus - when using Excel copy buffer to copy cell ranges, the cell loses its focus after the copy execution, so we need to reapply the focus on the active cell that the user clicked
1 parent 66c8b4d commit 1dc8b76

File tree

10 files changed

+55
-75
lines changed

10 files changed

+55
-75
lines changed

‎examples/webpack-demo-vanilla-bundle/src/examples/example13.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {
22
BindingEventService,
33
Column,
4+
Editors,
5+
FieldType,
46
GridOption,
57
} from '@slickgrid-universal/common';
68
import { ExcelExportService } from '@slickgrid-universal/excel-export';
@@ -59,6 +61,9 @@ export class Example13 {
5961
enableAutoResize: true,
6062
enableHeaderButton: true,
6163
enableHeaderMenu: false,
64+
autoCommitEdit: true,
65+
autoEdit: true,
66+
editable: true,
6267
autoResize: {
6368
container: '.demo-container',
6469
},
@@ -124,9 +129,11 @@ export class Example13 {
124129
id: i,
125130
name: 'Column ' + String.fromCharCode('A'.charCodeAt(0) + i),
126131
field: i + '',
127-
width: i === 0 ? 70 : 100, // have the 2 first columns wider
132+
width: i === 0 ? 70 : 100, // make the first 2 columns wider
128133
filterable: true,
129134
sortable: true,
135+
type: FieldType.number,
136+
editor: { model: Editors.integer },
130137
formatter: (_row, _cell, value, columnDef) => {
131138
if (gridNo === 1 && columns1WithHighlightingById[columnDef.id] && value < 0) {
132139
return `<div style="color:red; font-weight:bold;">${value}</div>`;

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
"rimraf": "^3.0.2",
7676
"rxjs": "^7.5.7",
7777
"servor": "^4.0.2",
78-
"slickgrid": "^3.0.3",
78+
"slickgrid": "^3.0.4",
7979
"sortablejs": "^1.15.0",
8080
"ts-jest": "^29.0.5",
8181
"ts-node": "^10.9.1",

‎packages/common/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@
8282
"jquery": "^3.6.3",
8383
"moment-mini": "^2.29.4",
8484
"multiple-select-modified": "^1.3.17",
85-
"slickgrid": "^3.0.3",
85+
"slickgrid": "^3.0.4",
8686
"sortablejs": "^1.15.0",
8787
"un-flatten-tree": "^2.0.12"
8888
},

‎packages/common/src/services/__tests__/gridEvent.service.spec.ts

Lines changed: 5 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -179,46 +179,22 @@ describe('GridEventService', () => {
179179
});
180180
});
181181

182-
it('should execute the column "onCellClick" callback method and "setActiveCell" cell navigation is enabled but column is not editable', () => {
182+
it('should execute the column "onCellClick" callback method and "setActiveCell" only when enableExcelCopyBuffer is enabled', () => {
183183
gridStub.getOptions = jest.fn();
184-
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableCellNavigation: true, editable: false });
184+
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableCellNavigation: true, enableExcelCopyBuffer: true });
185185
const spyActive = jest.spyOn(gridStub, 'setActiveCell');
186186
const spyGetCols = jest.spyOn(gridStub, 'getColumns').mockReturnValue([mockColumn]);
187187
const spyGetData = jest.spyOn(gridStub, 'getDataItem').mockReturnValue(mockRowData);
188188
const spyOnChange = jest.spyOn(mockColumn, 'onCellClick');
189189

190190
service.bindOnClick(gridStub);
191-
gridStub.onClick.notify({ cell: 0, row: 0, grid: gridStub }, new Slick.EventData(), gridStub);
191+
gridStub.onClick.notify({ cell: 0, row: 2, grid: gridStub }, new Slick.EventData(), gridStub);
192192

193-
expect(spyActive).toHaveBeenCalled();
193+
expect(spyActive).toHaveBeenCalledWith(2, 0);
194194
expect(spyGetCols).toHaveBeenCalled();
195195
expect(spyGetData).toHaveBeenCalled();
196196
expect(spyOnChange).toHaveBeenCalledWith(expect.anything(), {
197-
row: 0,
198-
cell: 0,
199-
dataView: dataViewStub,
200-
grid: gridStub,
201-
columnDef: mockColumn,
202-
dataContext: mockRowData
203-
});
204-
});
205-
206-
it('should execute the column "onCellClick" callback method and "setActiveCell" when cell is editable and autoCommitEdit', () => {
207-
gridStub.getOptions = jest.fn();
208-
jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableCellNavigation: true, editable: true, autoCommitEdit: true });
209-
const spyActive = jest.spyOn(gridStub, 'setActiveCell');
210-
const spyGetCols = jest.spyOn(gridStub, 'getColumns').mockReturnValue([mockColumn]);
211-
const spyGetData = jest.spyOn(gridStub, 'getDataItem').mockReturnValue(mockRowData);
212-
const spyOnChange = jest.spyOn(mockColumn, 'onCellClick');
213-
214-
service.bindOnClick(gridStub);
215-
gridStub.onClick.notify({ cell: 0, row: 0, grid: gridStub }, new Slick.EventData(), gridStub);
216-
217-
expect(spyActive).toHaveBeenCalled();
218-
expect(spyGetCols).toHaveBeenCalled();
219-
expect(spyGetData).toHaveBeenCalled();
220-
expect(spyOnChange).toHaveBeenCalledWith(expect.anything(), {
221-
row: 0,
197+
row: 2,
222198
cell: 0,
223199
dataView: dataViewStub,
224200
grid: gridStub,

‎packages/common/src/services/gridEvent.service.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class GridEventService {
2828

2929
/* OnCellChange Event */
3030
bindOnBeforeEditCell(grid: SlickGrid) {
31-
const dataView = grid?.getData && grid.getData() as SlickDataView;
31+
const dataView = grid?.getData?.() as SlickDataView;
3232

3333
// subscribe to this Slickgrid event of onBeforeEditCell
3434
this._eventHandler.subscribe(grid.onBeforeEditCell, (e, args) => {
@@ -57,7 +57,7 @@ export class GridEventService {
5757

5858
/* OnCellChange Event */
5959
bindOnCellChange(grid: SlickGrid) {
60-
const dataView = grid?.getData && grid.getData() as SlickDataView;
60+
const dataView = grid?.getData?.() as SlickDataView;
6161

6262
// subscribe to this Slickgrid event of onCellChange
6363
this._eventHandler.subscribe(grid.onCellChange, (e, args) => {
@@ -86,22 +86,19 @@ export class GridEventService {
8686

8787
/* OnClick Event */
8888
bindOnClick(grid: SlickGrid) {
89-
const dataView = grid?.getData && grid.getData() as SlickDataView;
89+
const dataView = grid?.getData?.() as SlickDataView;
9090

9191
this._eventHandler.subscribe(grid.onClick, (e, args) => {
9292
if (!e || !args || !grid || args.cell === undefined || !grid.getColumns || !grid.getDataItem) {
9393
return;
9494
}
95-
const column: Column = grid && grid.getColumns && grid.getColumns()[args.cell];
96-
const gridOptions: GridOption = grid && grid.getOptions && grid.getOptions() || {};
97-
98-
// only when the grid option "autoCommitEdit" is enabled, we will make the cell active (in focus) when clicked
99-
// setting the cell as active as a side effect and if "autoCommitEdit" is set to false then the Editors won't save correctly
100-
if (gridOptions.enableCellNavigation && (!gridOptions.editable || (gridOptions.editable && gridOptions.autoCommitEdit))) {
101-
try {
102-
grid.setActiveCell(args.row, args.cell, false, false, true);
103-
// eslint-disable-next-line @typescript-eslint/no-shadow, no-empty
104-
} catch(e) {}
95+
const column: Column = grid.getColumns?.()[args.cell];
96+
const gridOptions: GridOption = grid.getOptions?.() || {};
97+
98+
// when using Excel copy buffer to copy cell ranges, the cell loses its focus after the copy execution
99+
// so we need to reapply the focus on the active cell that the user clicked
100+
if (gridOptions.enableCellNavigation && gridOptions.enableExcelCopyBuffer) {
101+
grid.setActiveCell(args.row, args.cell);
105102
}
106103

107104
// if the column definition has a onCellClick property (a callback function), then run it

‎packages/vanilla-bundle/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
"dequal": "^2.0.3",
6767
"flatpickr": "^4.6.13",
6868
"jquery": "^3.6.3",
69-
"slickgrid": "^3.0.3",
69+
"slickgrid": "^3.0.4",
7070
"sortablejs": "^1.15.0",
7171
"whatwg-fetch": "^3.6.2"
7272
},

‎packages/vanilla-force-bundle/webpack.config.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,5 @@ module.exports = ({ production } = {}) => ({
4141
options: { loader: 'ts', target: 'es2018' }
4242
},
4343
],
44-
},
45-
watchOptions: {
46-
ignored: '**/node_modules',
47-
poll: 1000, // Check for changes every second
4844
}
4945
});

‎pnpm-lock.yaml

Lines changed: 8 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/cypress/e2e/example11.cy.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,17 @@ describe('Example 11 - Batch Editing', { retries: 1 }, () => {
2121
cy.get('h3').should('contain', 'Example 11 - Batch Editing');
2222
});
2323

24+
it('should click on "Clear Local Storage" and expect to be back to original grid with all the columns', () => {
25+
cy.get('[data-test="clear-storage-btn"]')
26+
.click();
2427

25-
it('should have exact Column Titles in the grid', () => {
2628
cy.get('.grid11')
2729
.find('.slick-header-columns')
2830
.children()
2931
.each(($child, index) => expect($child.text()).to.eq(fullTitles[index]));
3032
});
3133

32-
it('should click on "Clear Local Storage" and expect to be back to original grid with all the columns', () => {
33-
cy.get('[data-test="clear-storage-btn"]')
34-
.click();
35-
34+
it('should have exact Column Titles in the grid', () => {
3635
cy.get('.grid11')
3736
.find('.slick-header-columns')
3837
.children()
@@ -56,36 +55,41 @@ describe('Example 11 - Batch Editing', { retries: 1 }, () => {
5655
.should('contain', '0 day')
5756
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
5857

59-
cy.get('.editor-duration').type('1').type('{enter}', { force: true });
58+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(2)`).click().type('1{enter}');
6059
cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(2)`).should('contain', '1 day')
6160
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
6261

63-
cy.get('.editor-duration').type('2').type('{enter}', { force: true });
62+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(2)`).click().type('2{enter}');
6463
cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(2)`).should('contain', '2 days')
6564
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
6665

67-
cy.get('.editor-duration').type('3').type('{enter}', { force: true });
66+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(2)`).click().type('3{enter}');
6867
cy.get(`[style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(2)`).should('contain', '3 days')
6968
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
70-
cy.get('.editor-duration').type('{esc}');
69+
70+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(2)`).click().type('{esc}');
7171
cy.get('.editor-duration').should('not.exist');
7272
});
7373

7474
it('should be able to change "Title" values of row indexes 1-3', () => {
7575
// change title
7676
cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(1)`).should('contain', 'TASK 1').click();
77-
cy.get('.editor-title').type('task 1111').type('{enter}', { force: true });
77+
cy.get('.editor-title').type('task 1111').type('{enter}');
7878
cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(1)`).should('contain', 'TASK 1111')
7979
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
8080

81-
cy.get('.editor-title').type('task 2222').type('{enter}', { force: true });
81+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(1)`).click()
82+
.type('task 2222{enter}');
8283
cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(1)`).should('contain', 'TASK 2222')
8384
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
8485

85-
cy.get('.editor-title').type('task 3333').type('{enter}', { force: true });
86+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(1)`).click()
87+
.type('task 3333').type('{enter}');
8688
cy.get(`[style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(1)`).should('contain', 'TASK 3333')
8789
.should('have.css', 'background-color').and('eq', UNSAVED_RGB_COLOR);
88-
cy.get('.editor-title').type('{esc}');
90+
91+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 3}px"] > .slick-cell:nth(1)`).click()
92+
.type('{esc}');
8993
cy.get('.editor-title').should('not.exist');
9094

9195
cy.get('.slick-viewport.slick-viewport-top.slick-viewport-left')

‎test/cypress/e2e/example12.cy.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { changeTimezone, zeroPadding } from '../plugins/utilities';
22

3-
describe('Example 12 - Composite Editor Modal', { retries: 1 }, () => {
3+
describe('Example 12 - Composite Editor Modal', { retries: 0 }, () => {
44
const fullPreTitles = ['', 'Common Factor', 'Analysis', 'Period', 'Item', ''];
55
const fullTitles = ['', ' Title', 'Duration', 'Cost', '% Complete', 'Complexity', 'Start', 'Completed', 'Finish', 'Product', 'Country of Origin', 'Action'];
66

@@ -43,20 +43,20 @@ describe('Example 12 - Composite Editor Modal', { retries: 1 }, () => {
4343
it('should be able to change "Duration" values of first 4 rows', () => {
4444
// change duration
4545
cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(2)`).should('contain', 'days').click();
46-
cy.get('.editor-duration').type('0').type('{enter}', { force: true });
46+
cy.get('.editor-duration').type('0{enter}');
4747
cy.get(`[style="top:${GRID_ROW_HEIGHT * 0}px"] > .slick-cell:nth(2)`)
4848
.should('contain', '0 day')
4949
.get('.editing-field')
5050
.should('have.css', 'border')
5151
.and('contain', `solid ${UNSAVED_RGB_COLOR}`);
5252

53-
cy.get('.editor-duration').type('1').type('{enter}', { force: true });
53+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(2)`).click().type('1{enter}');
5454
cy.get(`[style="top:${GRID_ROW_HEIGHT * 1}px"] > .slick-cell:nth(2)`).should('contain', '1 day')
5555
.get('.editing-field')
5656
.should('have.css', 'border')
5757
.and('contain', `solid ${UNSAVED_RGB_COLOR}`);
5858

59-
cy.get('.editor-duration').type('2').type('{enter}', { force: true });
59+
cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(2)`).click().type('2{enter}');
6060
cy.get(`[style="top:${GRID_ROW_HEIGHT * 2}px"] > .slick-cell:nth(2)`).should('contain', '2 days')
6161
.get('.editing-field')
6262
.should('have.css', 'border')

0 commit comments

Comments
 (0)