Skip to content

Resize column via keyboard#3357

Closed
godon019 wants to merge 10 commits intomainfrom
column-resize-by-keyboard
Closed

Resize column via keyboard#3357
godon019 wants to merge 10 commits intomainfrom
column-resize-by-keyboard

Conversation

@godon019
Copy link
Copy Markdown
Contributor

@godon019 godon019 commented Oct 7, 2023

Closes #3320

@godon019 godon019 self-assigned this Oct 7, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.21%. Comparing base (fe98d87) to head (e14b606).
Report is 133 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3357   +/-   ##
=======================================
  Coverage   98.21%   98.21%           
=======================================
  Files          47       47           
  Lines        5087     5091    +4     
  Branches      727      727           
=======================================
+ Hits         4996     5000    +4     
  Misses         91       91           
Files with missing lines Coverage Δ
src/DataGrid.tsx 99.34% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/DataGrid.tsx Outdated
}
}

if (column.resizable && isCtrlKeyHeldDown(event) && event.shiftKey) {
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Collaborator

@amanmahajan7 amanmahajan7 Mar 4, 2024

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 🤔

Comment thread src/DataGrid.tsx

if (mode === 'EDIT') return;

if (onCellKeyDown && isRowIdxWithinViewportBounds(rowIdx)) {
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

@godon019 godon019 marked this pull request as ready for review October 17, 2023 18:43
@kycutler
Copy link
Copy Markdown
Contributor

Hi folks, thanks for working on this! Please let me know if I can help at all.

One bit of feedback on the changes so far: ideally resizeColumn and getColumnWidth would be accessible from anywhere and not limited to specific handlers like onCellKeyDown so consumers have flexibility in how it is exposed (for example, column widths could be rendered within the grid and made editable via input elements). Perhaps the methods could be present on the CalculatedColumns themselves, or exposed via imperative handle on the grid?

Thanks for all the work on this fantastic project!

@amanmahajan7
Copy link
Copy Markdown
Collaborator

Should we complete and merge this PR?

@matanAlltra
Copy link
Copy Markdown

please merge this this behavior is an accessibility requirement

@amanmahajan7
Copy link
Copy Markdown
Collaborator

Replaced by #3754

@amanmahajan7 amanmahajan7 deleted the column-resize-by-keyboard branch April 11, 2025 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Accessibility] Make columns resizable via keyboard

4 participants