Skip to content

[refactor] rework cell navigation code#413

Merged
severo merged 8 commits intomasterfrom
react-rules
Feb 3, 2026
Merged

[refactor] rework cell navigation code#413
severo merged 8 commits intomasterfrom
react-rules

Conversation

@severo
Copy link
Copy Markdown
Contributor

@severo severo commented Feb 3, 2026

  • fix focus action on a cell (use an effect, not a ref callback, so that it is not called only on mount)
  • recreate the cell navigation context on new data (key) to simplify the code
  • handle the cell navigation implementation in cell navigation provider, instead of in the table keyboard event handler
  • simplify state changes during rendering (even if they are still problematic because they update parent components, which is not allowed in React rules)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the cell navigation code to use an action-based API instead of direct cell updates, and improves the component remounting mechanism.

Changes:

  • Replaced goToCell function with moveCell that accepts action objects (e.g., { type: 'NEXT_ROW' })
  • Changed from dataId prop to React's key prop for component remounting when data changes
  • Refactored cell focusing to use useEffect with refs instead of ref callbacks

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/providers/CellNavigationProvider.test.tsx Updated tests to use new moveCell API and removed dataId prop usage
src/providers/CellNavigationProvider.tsx Implemented moveCell with action-based navigation, removed dataId tracking, added numRowsPerPage prop
src/hooks/useCellFocus.ts Changed focusCellIfNeeded to focusIfNeeded using useMemo, updated to use moveCell
src/contexts/CellNavigationContext.ts Added MoveCellAction type definition and updated context interface
src/components/TableCorner/TableCorner.tsx Refactored to use useEffect for focusing instead of ref callback
src/components/RowHeader/RowHeader.tsx Refactored to use useEffect for focusing instead of ref callback
src/components/HighTable/Slice.tsx Updated keyboard navigation to use moveCell actions, removed numRowsPerPage parameter
src/components/HighTable/HighTable.tsx Changed to pass numRowsPerPage to CellNavigationProvider and use key={dataId}
src/components/ColumnHeader/ColumnHeader.tsx Refactored to use useEffect for focusing instead of ref callback
src/components/Cell/Cell.tsx Refactored to use useEffect for focusing instead of ref callback

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/providers/CellNavigationProvider.tsx
Comment thread src/providers/CellNavigationProvider.tsx
Comment thread src/providers/CellNavigationProvider.tsx Outdated
severo and others added 2 commits February 3, 2026 13:31
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@severo severo merged commit 538cb29 into master Feb 3, 2026
5 checks passed
@severo severo deleted the react-rules branch February 3, 2026 17:40
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