Skip to content

Comments

Combobox base-ui migration#691

Draft
kylemcd wants to merge 9 commits intomainfrom
cursor/combobox-base-ui-migration-4c3f
Draft

Combobox base-ui migration#691
kylemcd wants to merge 9 commits intomainfrom
cursor/combobox-base-ui-migration-4c3f

Conversation

@kylemcd
Copy link
Member

@kylemcd kylemcd commented Feb 21, 2026

Migrate the Combobox component to use Base UI while maintaining its existing public API and behavior, backed by hardened regression tests.

This migration was executed in two phases:

  1. Test Hardening: Enhanced the test suite with new contract tests covering edge behaviors (e.g., controlled open state, closeOnSelect, Create behavior, defaultScrollToValue) and assertions for critical DOM/data attributes to lock in the existing behavior.
  2. Base UI Integration: Replaced the internal implementation of Combobox.Root, Trigger, Content, Options, and Option with Base UI primitives. A compatibility layer was introduced to ensure the public API, compound components, styling, custom behaviors (like clear button, tag rendering, search-in-popup, legacy object values), and accessibility contract remain identical.

Open in Web Open in Cursor 

cursoragent and others added 8 commits February 21, 2026 08:45
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
@cursor
Copy link

cursor bot commented Feb 21, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@changeset-bot
Copy link

changeset-bot bot commented Feb 21, 2026

⚠️ No Changeset found

Latest commit: 10930e8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Feb 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
telegraph-storybook Ready Ready Preview, Comment Feb 21, 2026 9:42am

Request Review

@knock-eng-bot
Copy link
Contributor

knock-eng-bot commented Feb 21, 2026

Package size differences

The following packages have size differences greater than 20 KB

  • @telegraph/combobox: +1873.10 KB
All package size differences
  • @telegraph/appearance: 0.00 KB
  • @telegraph/button: 0.00 KB
  • @telegraph/combobox: +1873.10 KB
  • @telegraph/compose-refs: 0.00 KB
  • @telegraph/data-list: 0.00 KB
  • @telegraph/filter: 0.00 KB
  • @telegraph/helpers: 0.00 KB
  • @telegraph/icon: 0.00 KB
  • @telegraph/input: 0.00 KB
  • @telegraph/kbd: 0.00 KB
  • @telegraph/layout: 0.00 KB
  • @telegraph/menu: 0.00 KB
  • @telegraph/modal: 0.00 KB
  • @telegraph/nextjs: 0.00 KB
  • @telegraph/popover: 0.00 KB
  • @telegraph/postcss-config: 0.00 KB
  • @telegraph/prettier-config: 0.00 KB
  • @telegraph/radio: 0.00 KB
  • @telegraph/segmented-control: 0.00 KB
  • @telegraph/select: 0.00 KB
  • @telegraph/style-engine: 0.00 KB
  • @telegraph/tabs: 0.00 KB
  • @telegraph/tag: 0.00 KB
  • @telegraph/textarea: 0.00 KB
  • @telegraph/toggle: 0.00 KB
  • @telegraph/tokens: 0.00 KB
  • @telegraph/tooltip: 0.00 KB
  • @telegraph/truncate: 0.00 KB
  • @telegraph/typography: 0.00 KB
  • @telegraph/vite-config: 0.00 KB

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

}
onValueChange={handleBaseValueChange}
multiple={isMultipleSelectValue}
modal={modal && false}
Copy link

Choose a reason for hiding this comment

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

Modal prop always forced to false

Medium Severity

The expression modal={modal && false} always evaluates to a falsy value regardless of the modal prop's input. When modal is true (the default, set on line 119), the expression yields false. When modal is false or undefined, it yields that falsy value. This makes the modal prop on the public API completely inert — consumers passing modal={true} will silently get non-modal behavior. If the intent is to always disable modal mode, the expression can just be false with a comment explaining why.

Fix in Cursor Fix in Web

});

return () => observer.disconnect();
}, []);
Copy link

Choose a reason for hiding this comment

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

Global MutationObserver per combobox instance is wasteful

Medium Severity

Each Root instance creates a MutationObserver on document.body with childList: true, subtree: true, firing on every DOM addition/removal anywhere on the page for the component's entire lifetime. Multiple combobox instances multiply this cost. Additionally, the same focus-guard labeling logic is duplicated in the Content component (lines 610–619), making the Content effect redundant given the persistent observer already covers all mutations.

Additional Locations (1)

Fix in Cursor Fix in Web

Co-authored-by: Kyle McDonald <kylemcd@users.noreply.github.com>
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.

3 participants