Skip to content

Conversation

@TkDodo
Copy link
Collaborator

@TkDodo TkDodo commented Dec 18, 2025

and replace them with a dedicated trigger function that renders a <SelectTrigger.Button>

depends on:

closes: https://linear.app/getsentry/issue/DE-668/streamline-compactselect-triggers

Most of the changes were done automatically by a codemod, but I checked them all afterwards.

The biggest change that was migrated wrongly is that passing children: undefined or children: null in triggerProps before led to the default children being rendered, for example:

triggerProps={{
  prefix: t('Geo region'),
  children: value.length === 0 ? t('All') : undefined,
}}

The 1:1 migration would be:

trigger={triggerProps => (
  <SelectTrigger.Button {...triggerProps} prefix={t('Geo region')}>
    {value.length === 0 ? t('All') : undefined}
  </SelectTrigger.Button>
)}

But this won’t work because falling back to undefined means nothing gets rendered.
So the new pattern is falling back to triggerProps.children:

trigger={triggerProps => (
  <SelectTrigger.Button {...triggerProps} prefix={t('Geo region')}>
-    {value.length === 0 ? t('All') : undefined}
+    {value.length === 0 ? t('All') : triggerProps.children}
  </SelectTrigger.Button>
)}

As triggerProps contains the defaultLabel as children, but the expression inside the body of SelectTrigger.Button completely overrides that.

this is the commit where I made most of these changes: 33f22a5

To make this future-proof, I’ve disallowed undefined and null on type-level as children for for SelectTrigger

while we shouldn't provide `triggerProps` _and_ `trigger` to a `CompactSelect`, we do create an `id` that we label the `ul` by, so this id must be put onto the trigger.

This does happen for the default trigger, but not if you provide your own. This PR fixes this by passing the id to the props that will eventually get spread onto the trigger.
# Conflicts:
#	static/app/components/core/compactSelect/control.tsx
@linear
Copy link

linear bot commented Dec 18, 2025

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Dec 18, 2025
@codecov
Copy link

codecov bot commented Dec 18, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #105215   +/-   ##
========================================
  Coverage   80.62%    80.62%           
========================================
  Files        9428      9428           
  Lines      404023    404015    -8     
  Branches    25661     25659    -2     
========================================
- Hits       325724    325718    -6     
+ Misses      77830     77828    -2     
  Partials      469       469           

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants