Skip to content

Commit

Permalink
fix: getCellFromPoint() should return row/cell -1 outside grid canv…
Browse files Browse the repository at this point in the history 
…as (#1325)

* fix: `getCellFromPoint()` should return row/cell -1 outside grid canvas
  • Loading branch information
@ghiscoding
ghiscoding authored Jan 15, 2024
1 parent 74c47a1 commit b483e62
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 31 deletions.
72 changes: 48 additions & 24 deletions packages/common/src/core/__tests__/slickGrid.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2999,44 +2999,67 @@ describe('SlickGrid core file', () => {
];

describe('getCellFromPoint() method', () => {
it('should return { row:0, cell:-1 } when x/y coordinates are 0,0', () => {
grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });

const result = grid.getCellFromPoint(0, 0);

expect(result).toEqual({ row: 0, cell: -1 });
});

it('should return { row:2, cell:2 } when x/y coordinates are 2x times offset with small buffer', () => {
grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });

const result = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 2) + 5, (DEFAULT_COLUMN_HEIGHT * 2) + 5);

expect(result).toEqual({ row: 2, cell: 2 }); // OK: guessed the same
});
const columns = [
{ id: 'firstName', field: 'firstName', name: 'First Name' },
{ id: 'middleName', field: 'middleName', name: 'Middle Name' },
{ id: 'lastName', field: 'lastName', name: 'Last Name' },
{ id: 'age', field: 'age', name: 'Age' },
{ id: 'gender', field: 'gender', name: 'Gender' },
{ id: 'doorNumber', field: 'doorNumber', name: 'Door Number', hidden: true },
{ id: 'streetName', field: 'streetName', name: 'Street Name', hidden: true },
] as Column[];
const data = [
{ id: 0, firstName: 'John', lastName: 'Doe', age: 30, gender: 'male' },
{ id: 1, firstName: 'Jane', lastName: 'Doe', age: 28, gender: 'female' },
{ id: 2, firstName: 'Bob', lastName: 'Smith', age: 48, gender: 'male' },
{ id: 3, firstName: 'Arnold', lastName: 'Smith', age: 37, gender: 'male' },
];

it('should return { row:-2, cell:-1 } when x/y coordinates are both 2x times negative offset values with small buffer', () => {
it('should return correct row/cell when providing x,y coordinates', () => {
grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });

const result = grid.getCellFromPoint(-(DEFAULT_COLUMN_WIDTH * 2) + 5, -(DEFAULT_COLUMN_HEIGHT * 2) + 5);
const result1 = grid.getCellFromPoint(0, 0);
const result2 = grid.getCellFromPoint(0, DEFAULT_COLUMN_HEIGHT + 5);
const result3 = grid.getCellFromPoint(DEFAULT_COLUMN_WIDTH + 5, 0);
const result4 = grid.getCellFromPoint(DEFAULT_COLUMN_WIDTH + 5, DEFAULT_COLUMN_HEIGHT + 5);
const result5 = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 2) + 5, (DEFAULT_COLUMN_HEIGHT * 2) + 5);
const result6 = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 3) + 5, (DEFAULT_COLUMN_HEIGHT * 3) + 5);
const result7 = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 4) + 5, (DEFAULT_COLUMN_HEIGHT * 4) + 5);

expect(result).toEqual({ row: -2, cell: -1 });
expect(result1).toEqual({ cell: 0, row: 0 });
expect(result2).toEqual({ cell: 0, row: 1 });
expect(result3).toEqual({ cell: 1, row: 0 });
expect(result4).toEqual({ cell: 1, row: 1 });
expect(result5).toEqual({ cell: 2, row: 2 });
expect(result6).toEqual({ cell: 3, row: 3 });
expect(result7).toEqual({ cell: 4, row: 4 });
});

it('should return { row:-3, cell:-1 } when x/y coordinates are both 3x times negative offset values with small buffer', () => {
it('should return negative row/cell when x,y coordinates is outside the grid canvas', () => {
grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });

const result = grid.getCellFromPoint(-(DEFAULT_COLUMN_WIDTH * 3) + 5, -(DEFAULT_COLUMN_HEIGHT * 3) + 5);
const result1 = grid.getCellFromPoint(0, -999);
const result2 = grid.getCellFromPoint(-777, 0);
const result3 = grid.getCellFromPoint(-(DEFAULT_COLUMN_WIDTH + 5), -(DEFAULT_COLUMN_HEIGHT + 5));
const result4 = grid.getCellFromPoint(-((DEFAULT_COLUMN_WIDTH * 2) + 5), -((DEFAULT_COLUMN_HEIGHT * 2) + 5));
const result5 = grid.getCellFromPoint(-((DEFAULT_COLUMN_WIDTH * 3) + 5), -((DEFAULT_COLUMN_HEIGHT * 3) + 5));

expect(result).toEqual({ row: -3, cell: -1 });
expect(result1).toEqual({ cell: 0, row: -1 });
expect(result2).toEqual({ cell: -1, row: 0 });
expect(result3).toEqual({ cell: -1, row: -1 });
expect(result4).toEqual({ cell: -1, row: -1 });
expect(result5).toEqual({ cell: -1, row: -1 });
});

it('should return { row: 4, cell: 2 } when column found at x/y coordinates is hidden (Gender) so cell will be -1 which is last known visible column', () => {
it('should skip a cell when column found at x/y coordinates is hidden (Gender) so cell will the last known visible column', () => {
grid = new SlickGrid<any, Column>(container, data, columns, { ...defaultOptions, enableCellNavigation: true });

const result = grid.getCellFromPoint((DEFAULT_COLUMN_WIDTH * 4) + 5, (DEFAULT_COLUMN_HEIGHT * 4) + 5);
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);

expect(result).toEqual({ row: 4, cell: 2 });
// 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 });
});
});

Expand Down Expand Up @@ -3132,6 +3155,7 @@ describe('SlickGrid core file', () => {
const secondRowSlickCells = container.querySelectorAll('.slick-row:nth-child(2) .slick-cell');
const event = new CustomEvent('click');
Object.defineProperty(event, 'target', { writable: true, value: secondRowSlickCells[1] });

const result = grid.getCellFromEvent(event); // not passing clientX/clientY will return NaN

expect(result).toBeNull();
Expand Down
20 changes: 13 additions & 7 deletions packages/common/src/core/slickGrid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4944,24 +4944,28 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
* @param y A y coordinate.
*/
getCellFromPoint(x: number, y: number) {
const row = this.getRowFromPosition(y);
let row = this.getRowFromPosition(y);
let cell = 0;

let w = 0;
for (let i = 0; i < this.columns.length && w < x; i++) {
for (let i = 0; i < this.columns.length && w <= x; i++) {
if (!this.columns[i] || this.columns[i].hidden) {
continue;
}
w += this.columns[i].width as number;
cell++;
}
cell -= 1;

/* istanbul ignore if - TODO: need to investigate, this is technically unreachable */
if (cell < 0) {
cell = 0;
// we'll return -1 when coordinate falls outside the grid canvas
if (row < -1) {
row = -1;
}
if (cell < -1) {
cell = -1;
}

return { row, cell: (cell - 1) };
return { row, cell };
}

protected getCellFromNode(cellNode: HTMLElement) {
Expand Down Expand Up @@ -5043,7 +5047,9 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
rowOffset = (this._options.frozenBottom) ? Utils.height(this._canvasTopL) as number : this.frozenRowsHeight;
}

row = this.getCellFromPoint(targetEvent.clientX - c!.left, targetEvent.clientY - c!.top + rowOffset + document.documentElement.scrollTop).row;
const x = targetEvent.clientX - c!.left;
const y = targetEvent.clientY - c!.top + rowOffset + document.documentElement.scrollTop;
row = this.getCellFromPoint(x, y).row;
}

const cell = this.getCellFromNode(cellNode as HTMLElement);
Expand Down

0 comments on commit b483e62

Please sign in to comment.