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.

Sign up for GitHub

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

Jump to bottom

External copy paste handlers #2981

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

External copy paste handlers #2981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d706c0
add external copy and paste handlers
adam-kuhn Aug 6, 2022
7193e14
add tests for external copy and paste handlers
adam-kuhn Aug 6, 2022
b0c1cb3
add external copy and paste handlers to readme
adam-kuhn Aug 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,14 @@ A number defining the height of summary rows.

###### `onPaste?: Maybe<(event: PasteEvent<R>) => R>`

###### `handleCopyExternally?: Maybe<() => void>`

Handle the copy event outside of react data grid

###### `handlePasteExternally?: Maybe<() => void>`

Handle the paste event outside of react data grid

###### `onRowClick?: Maybe<(row: R, column: CalculatedColumn<R, SR>) => void>`

###### `onRowDoubleClick?: Maybe<(row: R, column: CalculatedColumn<R, SR>) => void>`
Expand Down
14 changes: 11 additions & 3 deletions src/DataGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ export interface DataGridProps<R, SR = unknown, K extends Key = Key> extends Sha
onFill?: Maybe<(event: FillEvent<R>) => R>;
onCopy?: Maybe<(event: CopyEvent<R>) => void>;
onPaste?: Maybe<(event: PasteEvent<R>) => R>;
handleCopyExternally?: Maybe<() => void>;
handlePasteExternally?: Maybe<() => void>;

/**
* Event props
Expand Down Expand Up @@ -216,6 +218,8 @@ function DataGrid<R, SR, K extends Key>(
onFill,
onCopy,
onPaste,
handleCopyExternally,
handlePasteExternally,
// Toggles and modes
cellNavigationMode: rawCellNavigationMode,
enableVirtualization,
Expand Down Expand Up @@ -547,10 +551,12 @@ function DataGrid<R, SR, K extends Key>(

const { key, keyCode } = event;
const { rowIdx } = selectedPosition;
const hasCopyHandler = handleCopyExternally ?? onCopy;
const hasPasteHandler = handlePasteExternally ?? onPaste;

if (
selectedCellIsWithinViewportBounds &&
(onPaste != null || onCopy != null) &&
(hasPasteHandler != null || hasCopyHandler != null) &&
isCtrlKeyHeldDown(event) &&
!isGroupRow(rows[rowIdx]) &&
selectedPosition.mode === 'SELECT'
Expand All @@ -560,11 +566,13 @@ function DataGrid<R, SR, K extends Key>(
const cKey = 67;
const vKey = 86;
if (keyCode === cKey) {
handleCopy();
const copyHandler = handleCopyExternally ?? handleCopy
copyHandler();
return;
}
if (keyCode === vKey) {
handlePaste();
const pastehandler = handlePasteExternally ?? handlePaste
pastehandler();
return;
}
}
Expand Down
33 changes: 30 additions & 3 deletions test/copyPaste.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,19 @@ const summaryRows: readonly Row[] = [
const copyCellClassName = 'rdg-cell-copied';
const onPasteSpy = jest.fn();
const onCopySpy = jest.fn();
const externalPasteSpy = jest.fn();
const externalCopySpy = jest.fn();

function CopyPasteTest({
onPasteCallback = true,
onCopyCallback = false
onCopyCallback = false,
externalCopy = false,
externalPaste = false,
}: {
onPasteCallback?: boolean;
onCopyCallback?: boolean;
externalCopy?: boolean;
externalPaste?: boolean;
Copy link

Choose a reason for hiding this comment

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

I would try to keep the "on" convention they have in the previous callers, just to keep aligned with their code style.

Code looks good and thanks for this improvement, I am looking forward to this change too.

}) {
const [rows, setRows] = useState(initialRows);

Expand All @@ -68,16 +74,21 @@ function CopyPasteTest({
onRowsChange={setRows}
onPaste={onPasteCallback ? onPaste : undefined}
onCopy={onCopyCallback ? onCopySpy : undefined}
handlePasteExternally={externalPaste ? externalPasteSpy : undefined}
handleCopyExternally={externalCopy ? externalCopySpy : undefined}
/>
);
}

function setup(onPasteCallback = true, onCopyCallback = false) {
function setup(onPasteCallback = true, onCopyCallback = false, externalCopy = false, externalPaste = false) {
onPasteSpy.mockReset();
onCopySpy.mockReset();
externalPasteSpy.mockReset();
externalCopySpy.mockReset();

render(
<StrictMode>
<CopyPasteTest onPasteCallback={onPasteCallback} onCopyCallback={onCopyCallback} />
<CopyPasteTest onPasteCallback={onPasteCallback} onCopyCallback={onCopyCallback} externalCopy={externalCopy} externalPaste={externalPaste}/>
</StrictMode>
);
}
Expand Down Expand Up @@ -199,3 +210,19 @@ test('should not allow paste on header or summary cells', async () => {
expect(getSelectedCell()).toHaveTextContent('s1');
expect(onPasteSpy).not.toHaveBeenCalled();
});

test('when provided external copy, it is used over the internal copy method', async () => {
setup(true, true, true);
await userEvent.click(getCellsAtRowIndex(0)[0]);
copySelectedCell();
expect(externalCopySpy).toHaveBeenCalled();
expect(onCopySpy).not.toHaveBeenCalled();
})

test('when provided external paste, it is used over the internal paste method', async () => {
setup(true, true, true, true);
await userEvent.click(getCellsAtRowIndex(0)[0]);
pasteSelectedCell();
expect(externalPasteSpy).toHaveBeenCalled();
expect(onPasteSpy).not.toHaveBeenCalled();
})