Skip to content

[composite] Collapse grid navigation identity transform#5027

Open
atomiks wants to merge 1 commit into
mui:masterfrom
atomiks:claude/composite-grid-navigation-collapse
Open

[composite] Collapse grid navigation identity transform#5027
atomiks wants to merge 1 commit into
mui:masterfrom
atomiks:claude/composite-grid-navigation-collapse

Conversation

@atomiks

@atomiks atomiks commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

The grid navigator injected into useListNavigation only ever operates on a uniform 1×1 grid (sizes are always 1×1, packing is never dense), so the cell-map machinery is an identity transform: createGridCellMap returns [0..n-1], all corners coincide, the recomputed min/max equal the values passed in, and the disabled-index expansion is equivalent to passing disabledIndices straight through.

Collapsing it to a single getGridNavigatedIndex call:

  • keeps createGridCellMap / getGridCellIndexOfCorner / getGridCellIndices out of grid-combobox bundles (they remain used by the real multi-cell composite grid navigator in internals/composite/root);
  • drops the per-keystroke O(n) getComputedStyle scan the old min/max recomputation ran on every key;
  • preserves the old cellMap[...] out-of-bounds behavior via isIndexOutOfListBounds, so a -1 sentinel still surfaces as "no navigation" rather than highlighting index -1.

Also renames a stale grid test describe block (sizes support is gone; the fixture is a uniform multi-column grid).

The useListNavigation and combobox suites pass; typecheck/eslint clean. Based on current master.

@atomiks atomiks added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. component: composite labels Jun 12, 2026
@pkg-pr-new

pkg-pr-new Bot commented Jun 12, 2026

Copy link
Copy Markdown

commit: 77ade07

@code-infra-dashboard

code-infra-dashboard Bot commented Jun 12, 2026

Copy link
Copy Markdown

Bundle size

Bundle Parsed size Gzip size
@base-ui/react ▼-885B(-0.19%) ▼-336B(-0.23%)

Details of bundle changes

Performance

Total duration: 1,245.03 ms -209.22 ms(-14.4%) | Renders: 50 (+0) | Paint: 1,891.14 ms -293.25 ms(-13.4%)

Test Duration Renders
Checkbox mount (500 instances) 70.52 ms ▼-43.57 ms(-38.2%) 1 (+0)
Select mount (200 instances) 136.86 ms ▼-37.66 ms(-21.6%) 3 (+0)
Popover mount (300 instances) 61.00 ms ▼-16.52 ms(-21.3%) 1 (+0)

9 tests within noise — details


Check out the code infra dashboard for more information about this PR.

@netlify

netlify Bot commented Jun 12, 2026

Copy link
Copy Markdown

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 77ade07
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/6a2ba9fcabf1050008fe5944
😎 Deploy Preview https://deploy-preview-5027--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@atomiks atomiks requested a review from Copilot June 12, 2026 06:21
@atomiks atomiks marked this pull request as ready for review June 12, 2026 06:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies the grid navigator injected into useListNavigation by removing the unused “cell map” indirection (which is an identity transform for the 1×1 uniform grid used there) and calling getGridNavigatedIndex directly, while preserving the prior out-of-bounds/-1 sentinel behavior.

Changes:

  • Refactor hooks/gridNavigation.ts to call getGridNavigatedIndex directly and translate out-of-bounds results to undefined.
  • Remove now-unused cell-map helpers/constants imports to improve tree-shaking for grid-combobox consumers.
  • Rename a useListNavigation grid test describe label to reflect the actual fixture scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/react/src/floating-ui-react/hooks/gridNavigation.ts Collapses identity cell-map logic into a direct getGridNavigatedIndex call and preserves sentinel behavior via isIndexOutOfListBounds.
packages/react/src/floating-ui-react/hooks/useListNavigation.test.tsx Updates a grid navigation describe block name for clarity/accuracy.

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

The injected grid navigator always runs on a uniform 1x1 grid, so the cell-map machinery (createGridCellMap/getGridCellIndexOfCorner/getGridCellIndices) is an identity transform. Collapse it to a direct getGridNavigatedIndex call: lets those helpers tree-shake out of grid-combobox bundles and drops the per-keystroke O(n) getComputedStyle scan. Also rename the stale 'different sizes' grid test (sizes support is gone; the fixture is a uniform multi-column grid).
@atomiks atomiks force-pushed the claude/composite-grid-navigation-collapse branch from ca44119 to 77ade07 Compare June 12, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: composite type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants