[terra-worklist-data-grid] added keyboard support for column resizing #1813
Conversation
| const getTableHeight = () => { | ||
| if (columnHeaderCellRef.current) { | ||
| // Find parent table element | ||
| const parentTable = columnHeaderCellRef.current.closest('table'); | ||
|
|
||
| // Update resize handle height to match parent table height | ||
| if (parentTable) { | ||
| return `${parentTable.offsetHeight}px`; | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| }; |
There was a problem hiding this comment.
You should no longer need this logic with the ResizeObserver being in the DataGrid component. You should be able to accept the table height as a prop and use directly. It will also allow you to remove the polyfill.
There was a problem hiding this comment.
Oh i didn't commit those changes. Committed here: 82615a0
| const parentTable = resizeHandleRef.current.closest('table'); | ||
|
|
||
| // Update resize handle height to match parent table height | ||
| if (parentTable) { | ||
| resizeHandleRef.current.style.height = `${parentTable.offsetHeight}px`; | ||
| } |
| <div | ||
| ref={resizeHandleRef} | ||
| draggable | ||
| role="slider" | ||
| role={!isNavigationEnabled ? 'slider' : 'divider'} |
There was a problem hiding this comment.
Should we use isNavigationEnabled instead of "!isNavigationEnabled" to deal with the positive case in the comparison?
There was a problem hiding this comment.
reversed condition here: 04a321e
| outline: 0 !important; | ||
| } | ||
|
|
||
| &:hover { | ||
| border-color: var(--terra-data-grid-resize-handle-hover-border-color, rgba(34, 42, 46, 0.6)); | ||
| border-color: var(--terra-data-grid-resize-handle-hover-border-color, #909496); |
There was a problem hiding this comment.
There should not be a need to change from RGBA to HEX. Also, I am not sure that this conversion is correctly accounting for opacity.
There was a problem hiding this comment.
I thought it was odd that this was using RGBA vs HEX in the other stylesheets but you're right about the opacity. Reverting it for now.
packages/terra-data-grid/src/subcomponents/ColumnResizeHandle.jsx
Outdated
Show resolved
Hide resolved
packages/terra-data-grid/tests/jest/ColumnResizeHandle.test.jsx
Outdated
Show resolved
Hide resolved
packages/terra-data-grid/tests/jest/ColumnResizeHandle.test.jsx
Outdated
Show resolved
Hide resolved
| outline: 0 !important; | ||
| } | ||
|
|
||
| &:hover { | ||
| border-color: var(--terra-data-grid-resize-handle-hover-border-color, rgba(34, 42, 46, 0.6)); | ||
| } | ||
| } | ||
|
|
||
| .resize-handle-selected { |
There was a problem hiding this comment.
Should this class just be "selected" instead of "resize-handle-selected" since it is localized? Also, we should be able to add this under the .resize-handle style block.
There was a problem hiding this comment.
Updated here: 46c86b0
| @@ -86,6 +86,7 @@ describe('ColumnHeader', () => { | |||
|
|
|||
| // Validate ColumnHeaderCell React component | |||
| const columnHeaderCells = columnHeader.find(ColumnHeaderCell); | |||
| // console.log(columnHeaderCells.first().props()) | |||
There was a problem hiding this comment.
I am going to approve the review, but I think you are supposed to remove this line in the test.
There was a problem hiding this comment.
Oh good catch. Removed here: 429c450
sdadn
added
⭐ UX Reviewed
Summary
This PR adds keyboard column support for the datagrid. Users can now navigate column headers and resize them using keyboard shortcuts.
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
Note: due to the architecture changes to datagrid, this PR will not be merged into main. Instead, it will be used as a reference to integrate it into table & datagrid in a separate PR. This will still need review to finalize the design & implementation.
This PR resolves:
UXPLATFORM-9147
Thank you for contributing to Terra.
@cerner/terra