Add internal `useActivePosition` hook by nstepien · Pull Request #3990 · Comcast/react-data-grid

nstepien · 2026-03-05T20:23:25Z

No description provided.

nstepien · 2026-03-05T21:40:22Z

src/hooks/useActivePosition.ts

+const initialActivePosition: ActivePosition = {
+  idx: -1,
+  // use -Infinity to avoid issues when adding header rows or top summary rows
+  rowIdx: Number.NEGATIVE_INFINITY,


Found a bug and added a test.

nstepien · 2026-03-05T21:41:27Z

src/hooks/useActivePosition.ts

+  }
+
+  return {
+    activePosition: resolvedActivePosition,


We will now render DataGrid with the correct state one render early in some cases.

We could also just not do this to keep the implementation simpler.

nstepien · 2026-03-05T21:43:12Z

src/hooks/useActivePosition.ts

+
+import type { CalculatedColumn, Position, StateSetter } from '../types';
+
+interface ActivePosition extends Position {


Renamed this from ActiveCellState to ActivePosition, likewise for EditPosition below

nstepien · 2026-03-05T21:44:24Z

src/DataGrid.tsx

    if (!activePositionIsCellInViewport) return;
-    const { idx, rowIdx } = activePosition;
-    onCellCopy?.({ row: rows[rowIdx], column: columns[idx] }, event);
+    onCellCopy?.({ row: getActiveRow(), column: getActiveColumn() }, event);


WDYT about getActiveRow()/getActiveColumn()? Worth it?

Not sure. Should we return variables instead activeRow, activeColumn?

We'd have to check if they're undefined in many places

Could also add queryActive*() 🤔

nstepien · 2026-03-05T22:21:10Z

src/DataGrid.tsx


  function getDragHandle() {
-    if (onFill == null || activePosition.mode === 'EDIT' || !activePositionIsCellInViewport) {
+    if (onFill == null || activePosition.mode !== 'ACTIVE' || !activePositionIsCellInViewport) {


I've changed the mode check in case we ever add a third mode

nstepien · 2026-03-05T22:26:39Z

test/browser/keyboardNavigation.test.tsx

  await validateCellPosition(2, 0);
 });
+
+test('adding a top summary row when no rows or cells are active should not focus the summary row', async () => {


Not sure where to put this test, it's not only strictly about keyboard navigation, and it may not be limited to TreeDataGrid if we ever add row focus support in DataGrid.

TreeDataGrid tests also have some keyboard tests so we can move it there as well. I am okay with either
https://github.com/Comcast/react-data-grid/blob/main/test/browser/TreeDataGrid.test.tsx#L311

Moved the test

codecov-commenter · 2026-03-05T23:07:55Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 97.05882% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.41%. Comparing base (e533510) to head (159304e).

Files with missing lines	Patch %	Lines
src/hooks/useActivePosition.ts	95.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3990      +/-   ##
==========================================
- Coverage   97.51%   97.41%   -0.11%     
==========================================
  Files          36       37       +1     
  Lines        1449     1470      +21     
  Branches      463      471       +8     
==========================================
+ Hits         1413     1432      +19     
- Misses         36       38       +2

Files with missing lines	Coverage Δ
src/DataGrid.tsx	`98.91% <100.00%> (-0.05%)`	⬇️
src/hooks/useActivePosition.ts	`95.00% <95.00%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

src/hooks/useActivePosition.ts

amanmahajan7 · 2026-03-06T15:44:22Z

src/hooks/useActivePosition.ts

+export function useActivePosition<R, SR>({
+  columns,
+  rows,
+  validatePosition,


Would it make sense for this hook to return validatePosition?

amanmahajan7 · 2026-03-06T16:12:08Z

src/DataGrid.tsx

    if (!activePositionIsCellInViewport) return;
-    const { idx, rowIdx } = activePosition;
-    onCellCopy?.({ row: rows[rowIdx], column: columns[idx] }, event);
+    onCellCopy?.({ row: getActiveRow(), column: getActiveColumn() }, event);


Not sure. Should we return variables instead activeRow, activeColumn?

amanmahajan7 · 2026-03-06T16:16:04Z

test/browser/keyboardNavigation.test.tsx

  await validateCellPosition(2, 0);
 });
+
+test('adding a top summary row when no rows or cells are active should not focus the summary row', async () => {


TreeDataGrid tests also have some keyboard tests so we can move it there as well. I am okay with either
https://github.com/Comcast/react-data-grid/blob/main/test/browser/TreeDataGrid.test.tsx#L311

Add internal useActivePosition hook

b26bbfb

nstepien self-assigned this Mar 5, 2026

nstepien commented Mar 5, 2026

View reviewed changes

nstepien marked this pull request as ready for review March 5, 2026 22:26

nstepien requested a review from amanmahajan7 as a code owner March 5, 2026 22:26

amanmahajan7 reviewed Mar 6, 2026

View reviewed changes

nstepien added 2 commits March 6, 2026 16:44

move test

7dbe36b

move validatePosition

2c2d852

nstepien requested a review from amanmahajan7 March 6, 2026 16:48

Merge branch 'main' into useActivePosition

159304e


		import type { CalculatedColumn, Position, StateSetter } from '../types';

		interface ActivePosition extends Position {

Conversation

nstepien commented Mar 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Mar 5, 2026 •

edited

Loading