fix(table): keep cell content's keystrokes, and scope row scrolling to the focused grid - #20
Open
Taleef7 wants to merge 2 commits into
Open
Conversation
added 2 commits
July 29, 2026 14:02
handleKeyDown ran for every keydown that reached the grid, and calls preventDefault before deciding whether the key was meant for it. Anything interactive inside a cell is therefore unusable from the keyboard: Enter on a link activates the row instead of following the link, and because the nav binds bare letters, typing "j" or "k" in an in-cell input moves the row selection instead of typing a character. Space, Home and End are taken the same way. Return early when the event originates inside an anchor, button, input, select, textarea or contenteditable, so that content keeps its own keys. Mouse users never saw this, which is what makes it specifically an accessibility bug.
scrollActiveRowIntoView looked up [data-row-num="N"] against document. Row numbers restart at 0 in every grid, so on a page with more than one grid the lookup finds the first match in document order rather than the grid being navigated — arrow keys scroll a different table than the one with focus. useKeyboardNav now takes an optional containerRef and queries within it, falling back to document when none is supplied so existing callers are unaffected. PlainTable passes the tableRef it already holds.
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.
Fixes #19.
Two independent fixes, one commit each.
Interactive cell content keeps its own keys
handleKeyDownreturns early when the event originates inside an anchor, button, input, select, textarea, or contenteditable. Without it, the grid's bare-letter bindings (j/k) make an in-cell input impossible to type into, and Enter on an in-cell link activates the row instead of following the link.preventDefault()was being called before anything decided the key was meant for the grid.Row scrolling is scoped to the grid that owns the event
useKeyboardNavtakes an optionalcontainerRefand queries within it, falling back todocumentwhen none is passed — so existing callers are unaffected and the signature change is not breaking.PlainTablepasses thetableRefit already holds. Row numbers restart at 0 in every grid, so the previous document-wide lookup found the first match on the page rather than the grid being navigated.Verification
npm run typecheck,npm run lint(eslint + i18n), andnpm testall pass — 87 tests.No test included, and I would rather say why than leave it unexplained. Both behaviors are DOM-level. The unit suite has no jsdom or testing-library and covers pure helpers only, and the e2e harness has no fixture that renders interactive content inside a cell — so testing this properly means adding a harness fixture, which felt like more than a two-line behavioral fix should drag in. If you tell me where you would want that fixture to live, I will add it along with an e2e case.
Context
Both of these turned up using the grid through
@mieweb/ui/datavisin a compliance dashboard at MIE, where cells link through to employee and case detail and one page renders three grids. I have been running these two patches against a vendored copy for several weeks.