Add a prop to handle key down on a cell#153
Merged
Conversation
Note that we remove the unused top pending state
This reverts commit 1ca438c.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request adds keyboard navigation support to various table components by introducing a new prop to handle key down events on cells and updating the navigation context accordingly. Key changes include:
- Creation and integration of the CellsNavigationContext and corresponding hooks to manage focus and navigation.
- Updates to TableHeader, RowHeader, TableCorner, Cell, and ColumnHeader components to pass and utilize aria indices and keydown event handlers.
- Enhancements in HighTable and related tests to support keyboard interactions and assistive navigation.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/hooks/useCellsNavigation.tsx | New hook and context providing keyboard navigation support for table cells. |
| src/components/TableHeader/TableHeader.tsx & .test.tsx | Updated to include ariaRowIndex and correct aria indices for header cells. |
| src/components/TableCorner/TableCorner.tsx | Adjusted to require ariaRowIndex and integrate cell navigation on click. |
| src/components/RowHeader/RowHeader.tsx | Updated to require ariaRowIndex and use cell navigation hook. |
| src/components/HighTable/HighTable.tsx & .test.tsx | Integrated the navigation context; handled scroll focus and cell key down events. |
| src/components/ColumnResizer/ColumnResizer.tsx | Made the resizer keyboard accessible with new aria attributes and key event handling. |
| src/components/ColumnHeader/ColumnHeader.tsx & .test.tsx | Updated to include ariaRowIndex and utilize cell navigation for header clicks. |
| src/components/Cell/Cell.tsx | Added onKeyDown prop and integrated cell navigation for consistent focus behavior. |
Files not reviewed (1)
- src/components/HighTable/HighTable.module.css: Language not supported
Comments suppressed due to low confidence (1)
src/components/ColumnResizer/ColumnResizer.tsx:107
- [nitpick] Consider making the 'keyboardShiftWidth' value configurable via a component prop instead of being hardcoded. This would allow more flexibility for different use cases and improve maintainability.
if (e.key === 'ArrowRight') { setWidth?.(Math.max(minWidth, width + keyboardShiftWidth)) }
9 tasks
platypii
approved these changes
May 7, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #30
Note that this PR is against https://github.com/hyparam/hightable/tree/navigate-through-keyboard, ie #140