Skip to content

[Studio UI] Keep Tab navigation within grid bounds - #4060

Open
idaiv wants to merge 4 commits into
2026.xfrom
fix/grid-tab-within-bounds
Open

[Studio UI] Keep Tab navigation within grid bounds#4060
idaiv wants to merge 4 commits into
2026.xfrom
fix/grid-tab-within-bounds

Conversation

@idaiv

@idaiv idaiv commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Tab/Shift+Tab now navigate between cells within the grid
  • Tab wraps to the first cell of the next row; Shift+Tab to the last cell of the previous row
  • Only exits the grid at the very first cell (Shift+Tab) or last cell (Tab)
  • Matches WAI-ARIA grid keyboard pattern

Test plan

  • Focus a grid cell, press Tab — moves to the next cell
  • At the last cell in a row, Tab wraps to the first cell of the next row
  • At the very last cell, Tab exits the grid to the next focusable element
  • Shift+Tab reverses the direction
  • Arrow keys still work as before

🤖 Generated with Claude Code

Relates to https://github.com/pimcore/product-management/issues/372

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>
Copilot AI balanced review requested due to automatic review settings September 1, 2026 14:14
@idaiv idaiv added the Skip Milestone Check Exempts this PR from the mandatory milestone requirement label Sep 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 GridCell layer.
  • 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.index as though it were the row's displayed position. With client-side sorting enabled, TanStack reorders table.getRowModel().rows but 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 from getRowModel().rows instead; 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. If initialState hides a column (or grouped columns are supplied), this computes a data-grid-column value 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}"]`)
idaiv and others added 2 commits September 1, 2026 14:20
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>
@idaiv
idaiv requested a review from vin0401 September 2, 2026 09:11
@sonarqubecloud

sonarqubecloud Bot commented Sep 2, 2026

Copy link
Copy Markdown

@idaiv idaiv added this to the 2026.3.0 milestone Sep 2, 2026
@idaiv
idaiv requested a review from martineiber September 2, 2026 10:28
@idaiv idaiv removed the Skip Milestone Check Exempts this PR from the mandatory milestone requirement label Sep 2, 2026
@idaiv
idaiv removed the request for review from martineiber September 2, 2026 10:36
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.

2 participants