[Studio UI] Keep Tab navigation within grid bounds - #4060
Open
idaiv wants to merge 4 commits into
Open
Conversation
Tab/Shift+Tab now move between cells within the grid: Tab moves to the next cell (wrapping to the first cell of the next row), Shift+Tab to the previous. Tab only exits the grid at the very first or last cell, matching the WAI-ARIA grid pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Tab/Shift+Tab grid-cell navigation, but the implementation is incomplete.
Changes:
- Handles Tab alongside arrow-key navigation.
- Wraps focus across row boundaries.
- Extracts shared cell-focus logic.
Review contract:
- Claim: Keep Tab navigation within grid bounds.
- Root cause: Only partially fixed; sorted rows and custom cells remain incorrect.
- Call sites: Custom cell renderers are not covered.
- Boundary: Navigation should operate on the shared
GridCelllayer. - Compatibility: No public API break identified.
- Tests: No automated regression coverage was added.
- Docs: No documentation update required.
- Risk: Virtualized navigation remains unverified.
Suppressed comments (2)
assets/js/src/core/components/grid/keyboard-navigation/use-keyboard-navigation.ts:80
- This uses the source
row.indexas though it were the row's displayed position. With client-side sorting enabled, TanStack reorderstable.getRowModel().rowsbut preserves each row's original index; Tab from the last cell can therefore jump to a different visual row or exit from the first displayed row when its original index is last. Resolve the current row and its successor fromgetRowModel().rowsinstead; the reverse path needs the analogous treatment.
const totalRows = props.table.getRowCount()
if (row + 1 < totalRows) return { row: row + 1, column: 0 }
assets/js/src/core/components/grid/keyboard-navigation/use-keyboard-navigation.ts:87
- The previous-row wrap uses all top-level columns, while the body renders visible leaf columns and
column.getIndex()is in that visible set. IfinitialStatehides a column (or grouped columns are supplied), this computes adata-grid-columnvalue that is not rendered; the event has already been cancelled, so Shift+Tab gets stuck. Use the last visible leaf-column index here.
if (row - 1 >= 0) return { row: row - 1, column: props.table.getAllColumns().length - 1 }
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| function focusCell (rowId: number, columnId: number): void { | ||
| if (tableElement?.current === null) return | ||
| const cellElement = tableElement!.current.querySelector<HTMLDivElement>(`[data-grid-row="${rowId}"][data-grid-column="${columnId}"]`) |
Extract resolveTarget and resolveTabTarget from handleArrowNavigation to bring cognitive complexity from 16 to within the allowed 15. Uses switch/case for cleaner key dispatch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
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.



Summary
Test plan
🤖 Generated with Claude Code
Relates to https://github.com/pimcore/product-management/issues/372