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 @@ -34,6 +34,8 @@
- ⚠️ `getValidFilterValues`
- ⚠️ `onCellCopyPaste`
- ⚠️ `onSelectedCellRangeChange`
- ⚠️ `onCheckCellIsEditable`
- Use `column.editable` instead.
- ⚠️ `onGridKeyDown`
- ⚠️ `onGridKeyUp`
- ⚠️ `onRowDoubleClick`
Expand Down
5 changes: 0 additions & 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,
FormatterProps,
Expand Down Expand Up @@ -121,8 +120,6 @@ export interface DataGridProps<R, K extends keyof R, SR = unknown> {
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 @@ -180,7 +177,6 @@ function DataGrid<R, K extends keyof R, SR>({
onScroll,
onColumnResize,
onSelectedCellChange,
onCheckCellIsEditable,
// Toggles and modes
enableFilters = false,
enableCellAutoFocus = true,
Expand Down Expand Up @@ -453,7 +449,6 @@ function DataGrid<R, K extends keyof R, SR>({
scrollTop={scrollTop}
scrollToCell={scrollToCell}
editorPortalTarget={editorPortalTarget}
onCheckCellIsEditable={onCheckCellIsEditable}
onRowsUpdate={handleRowsUpdate}
onSelectedCellChange={onSelectedCellChange}
/>
Expand Down
5 changes: 0 additions & 5 deletions src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,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
4 changes: 1 addition & 3 deletions src/masks/InteractionMasks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import { DataGridProps } from '../DataGrid';

type SharedCanvasProps<R, SR> = Pick<DataGridProps<R, never, SR>,
| 'rows'
| 'onCheckCellIsEditable'
| 'onSelectedCellChange'
> & Pick<Required<DataGridProps<R, never, SR>>,
| 'rowHeight'
Expand Down Expand Up @@ -70,7 +69,6 @@ export default function InteractionMasks<R, SR>({
scrollLeft,
scrollTop,
onSelectedCellChange,
onCheckCellIsEditable,
onRowsUpdate,
scrollToCell
}: InteractionMasksProps<R, SR>) {
Expand Down Expand Up @@ -257,7 +255,7 @@ export default function InteractionMasks<R, SR>({

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

function selectCell(position: Position, enableEditor = false): void {
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 @@ -152,7 +152,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 @@ -109,7 +109,7 @@ export function canEdit<R, SR>(column: CalculatedColumn<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.editable);
return column.editable || column.editor != 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 @@ -30,14 +30,12 @@ interface IsSelectedCellEditableOpts<R, SR> {
selectedPosition: Position;
columns: readonly CalculatedColumn<R, SR>[];
rows: readonly R[];
onCheckCellIsEditable?: (arg: { row: R; column: CalculatedColumn<R, SR> } & Position) => boolean;
}

export function isSelectedCellEditable<R, SR>({ selectedPosition, columns, rows, onCheckCellIsEditable }: IsSelectedCellEditableOpts<R, SR>): boolean {
export function isSelectedCellEditable<R, SR>({ selectedPosition, columns, rows }: IsSelectedCellEditableOpts<R, SR>): boolean {
const column = columns[selectedPosition.idx];
const row = rows[selectedPosition.rowIdx];
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