Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove onCheckCellIsEditable prop #2016

Merged
merged 8 commits into from
Oct 29, 2020
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
- ⚠️ `getValidFilterValues`
- ⚠️ `onCellCopyPaste`
- ⚠️ `onSelectedCellRangeChange`
- ⚠️ `onCheckCellIsEditable`
- Use `column.editable` instead.
- ⚠️ `onGridKeyDown`
- ⚠️ `onGridKeyUp`
- ⚠️ `onRowDoubleClick`
Expand Down
6 changes: 1 addition & 5 deletions src/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {

import {
CalculatedColumn,
CheckCellIsEditableEvent,
Column,
Filters,
Position,
Expand Down Expand Up @@ -149,8 +148,6 @@ export interface DataGridProps<R, K extends keyof R, SR = unknown> extends Share
onColumnResize?: (idx: number, width: number) => void;
/** Function called whenever selected cell is changed */
onSelectedCellChange?: (position: Position) => void;
/** called before cell is set active, returns a boolean to determine whether cell is editable */
onCheckCellIsEditable?: (event: CheckCellIsEditableEvent<R, SR>) => boolean;

/**
* Toggles and modes
Expand Down Expand Up @@ -209,7 +206,6 @@ function DataGrid<R, K extends keyof R, SR>({
onScroll,
onColumnResize,
onSelectedCellChange,
onCheckCellIsEditable,
// Toggles and modes
enableFilters = false,
enableCellCopyPaste = false,
Expand Down Expand Up @@ -639,7 +635,7 @@ function DataGrid<R, K extends keyof R, SR>({

function isCellEditable(position: Position): boolean {
return isCellWithinBounds(position)
&& isSelectedCellEditable<R, SR>({ columns, rows, selectedPosition: position, onCheckCellIsEditable, isGroupRow });
&& isSelectedCellEditable<R, SR>({ columns, rows, selectedPosition: position, isGroupRow });
}

function selectCell(position: Position, enableEditor = false): void {
Expand Down
5 changes: 0 additions & 5 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,6 @@ export interface RowsUpdateEvent<TUpdatedValue = never> {
fromCellKey?: string;
}

export interface CheckCellIsEditableEvent<TRow, TSummaryRow> extends Position {
row: TRow;
column: CalculatedColumn<TRow, TSummaryRow>;
}

export interface SelectRowEvent {
rowIdx: number;
checked: boolean;
Expand Down
2 changes: 1 addition & 1 deletion src/utils/columnUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ describe('canEdit', () => {
expect(canEdit({ ...column, editable: false }, row)).toBe(false);
expect(canEdit({ ...column, editable: true }, row)).toBe(true);
expect(canEdit({ ...column, editor }, row)).toBe(true);
expect(canEdit({ ...column, editor, editable: false }, row)).toBe(true);
expect(canEdit({ ...column, editor, editable: false }, row)).toBe(false);
expect(canEdit({ ...column, editor, editable: true }, row)).toBe(true);
});
});
2 changes: 1 addition & 1 deletion src/utils/columnUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export function canEdit<R, SR>(column: Column<R, SR>, row: R): boolean {
if (typeof column.editable === 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should move canEdit to selectedCellUtils.ts. It is only used once

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

return column.editable(row);
}
return Boolean(column.editor ?? column.editor2 ?? column.editable);
return column.editable || (column.editor != null || column.editor2 != null) && column.editable !== false;
nstepien marked this conversation as resolved.
Show resolved Hide resolved
}

export function getColumnScrollPosition<R, SR>(columns: readonly CalculatedColumn<R, SR>[], idx: number, currentScrollLeft: number, currentClientWidth: number): number {
Expand Down
6 changes: 2 additions & 4 deletions src/utils/selectedCellUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,14 @@ interface IsSelectedCellEditableOpts<R, SR> {
selectedPosition: Position;
columns: readonly CalculatedColumn<R, SR>[];
rows: readonly (R | GroupRow<R>)[];
onCheckCellIsEditable?: (arg: { row: R; column: CalculatedColumn<R, SR> } & Position) => boolean;
isGroupRow: (row: R | GroupRow<R>) => row is GroupRow<R>;
}

export function isSelectedCellEditable<R, SR>({ selectedPosition, columns, rows, onCheckCellIsEditable, isGroupRow }: IsSelectedCellEditableOpts<R, SR>): boolean {
export function isSelectedCellEditable<R, SR>({ selectedPosition, columns, rows, isGroupRow }: IsSelectedCellEditableOpts<R, SR>): boolean {
const column = columns[selectedPosition.idx];
const row = rows[selectedPosition.rowIdx];
if (column.rowGroup === true || isGroupRow(row)) return false;
const isCellEditable = onCheckCellIsEditable ? onCheckCellIsEditable({ row, column, ...selectedPosition }) : true;
return isCellEditable && canEdit<R, SR>(column, row);
return canEdit<R, SR>(column, row);
}

interface GetNextSelectedCellPositionOpts<R, SR> {
Expand Down