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

Resize column via keyboard #3357

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

Resize column via keyboard #3357

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8448fdd
initial idea
Oct 7, 2023
a2c31e7
expose handler
Oct 7, 2023
2734915
add test
Oct 9, 2023
8065535
cleanup
Oct 10, 2023
67b7819
use handler instead
Oct 17, 2023
1d19772
cleanup
Oct 17, 2023
8b6403c
Merge branch 'main' into column-resize-by-keyboard
amanmahajan7 Oct 20, 2023
7fa97cb
Merge branch 'main' into column-resize-by-keyboard
amanmahajan7 Mar 4, 2024
1e7c8be
Merge branch 'main' into column-resize-by-keyboard
amanmahajan7 Mar 14, 2024
e14b606
Merge branch 'main' into column-resize-by-keyboard
godon019 Mar 18, 2024
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
16 changes: 15 additions & 1 deletion src/DataGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ function DataGrid<R, SR, K extends Key>(

function handleKeyDown(event: KeyboardEvent<HTMLDivElement>) {
const { idx, rowIdx, mode } = selectedPosition;
const column = columns[idx];
if (mode === 'EDIT') return;

if (onCellKeyDown && isRowIdxWithinViewportBounds(rowIdx)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why do we need to check isRowIdxWithinViewportBounds before call onCellKeyDown ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think because row would be undefined otherwise. Same for summary row. We may have to add a type HEADER | ROW | SUMMARY

Expand All @@ -534,7 +535,7 @@ function DataGrid<R, SR, K extends Key>(
{
mode: 'SELECT',
row,
column: columns[idx],
column,
rowIdx,
selectCell
},
Expand Down Expand Up @@ -568,6 +569,19 @@ function DataGrid<R, SR, K extends Key>(
}
}

if (column.resizable && isCtrlKeyHeldDown(event) && event.shiftKey) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

need some insights here @amanmahajan7 @nstepien

  1. should we provide 'resizeViaKeyboard' flag? so that user can turn it on and off?
  2. what is the best way to provide handler via API? not sure if I can use onCellKeyDown

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to go with using the onCellKeyDown API. but still not sure as this doesn't seem to be very intrinsic

Copy link
Contributor

@amanmahajan7 amanmahajan7 Mar 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think we can always allow keyboard resizing if the column can be resized. I don't think we need an api to disable it unless there is a use case. We allow keyboard navigation on the header cell and can allow resizing when the cell is focused.

  • Check if the cell is focused and column is resizable
  • Check if a particular key combination is pressed (Ctrl + arrow right/left?)

Questions:

  • Which key to use. Are there any grid examples we can find?
    - Do we need some sort of timer if the key combination is pressed and increment the width by 1px every few ms? Does not look like this is an issue. The example works great. May be we can decrease the step 🤔

const leftKey = isRtl ? 39 : 37;
const rightKey = isRtl ? 37 : 39;
if (keyCode === leftKey || keyCode === rightKey) {
const width = getColumnWidth(column);
const step = 10;
const isIncrease = keyCode === rightKey;
const newWidth = isIncrease ? Number(width) + step : Number(width) - step;
handleColumnResizeLatest(column, newWidth);
return;
}
}

switch (event.key) {
case 'Escape':
setCopiedCell(null);
Expand Down
21 changes: 21 additions & 0 deletions test/column/resizable.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { fireEvent } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import type { Column } from '../../src';
import { resizeHandleClassname } from '../../src/HeaderCell';
Expand Down Expand Up @@ -104,3 +105,23 @@ test('should use the minWidth if specified', () => {
resize({ column: col2, clientXStart: 295, clientXEnd: 100, rect: { right: 300, left: 100 } });
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 100px' });
});

test('should resize column via keyboard', async () => {
function resizeCell(left: boolean) {
// eslint-disable-next-line testing-library/prefer-user-event
fireEvent.keyDown(document.activeElement!, {
keyCode: left ? '37' : '39',
ctrlKey: true,
shiftKey: true
});
}

setup({ columns, rows: [] });
const [, col2] = getHeaderCells();
await userEvent.click(col2);
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
resizeCell(false);
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 210px' });
resizeCell(true);
expect(getGrid()).toHaveStyle({ gridTemplateColumns: '100px 200px' });
});
Loading