merge combobox trigger foundation

apps/design-system/content/docs/components/combobox.mdx

@@ -19,18 +19,21 @@ See installation instructions for the [Popover](/docs/components/popover#install
 ```tsx
 'use client'
 
-import { Check, ChevronsUpDown } from 'lucide-react'
+import { Check } from 'lucide-react'
 import * as React from 'react'
-
-import { Button } from '@/components/ui/button'
 import {
+  ComboboxTrigger,
   Command,
   CommandEmpty,
   CommandGroup,
   CommandInput,
   CommandItem,
-} from '@/components/ui/command'
-import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
+  CommandList,
+  Popover,
+  PopoverContent,
+  PopoverTrigger,
+} from 'ui'
+
 import { cn } from '@/lib/utils'
 
 const frameworks = [
@@ -63,17 +66,15 @@ export function ComboboxDemo() {
   return (
     <Popover open={open} onOpenChange={setOpen}>
       <PopoverTrigger asChild>
-        <Button
-          variant="outline"
-          role="combobox"
+        <ComboboxTrigger
           aria-expanded={open}
-          className="w-[200px] justify-between"
+          data-state={open ? 'open' : 'closed'}
+          className={cn('w-[200px]', !value && 'text-foreground-lighter')}
         >
           {value
             ? frameworks.find((framework) => framework.value === value)?.label
             : 'Select framework...'}
-          <ChevronsUpDown className="ml-2 h-4 w-4 shrink-0 opacity-50" />
-        </Button>
+        </ComboboxTrigger>
       </PopoverTrigger>
       <PopoverContent className="w-[200px] p-0">
         <Command>

apps/design-system/registry/default/example/combobox-demo.tsx

@@ -1,9 +1,9 @@
 'use client'
 
-import { Check, ChevronsUpDown } from 'lucide-react'
+import { Check } from 'lucide-react'
 import * as React from 'react'
 import {
-  Button,
+  ComboboxTrigger,
   Command,
   CommandEmpty,
   CommandGroup,
@@ -47,18 +47,15 @@ export default function ComboboxDemo() {
   return (
     <Popover open={open} onOpenChange={setOpen}>
       <PopoverTrigger asChild>
-        <Button
-          variant="default"
-          role="combobox"
-          size={'small'}
+        <ComboboxTrigger
           aria-expanded={open}
-          className="w-[200px] justify-between"
-          iconRight={<ChevronsUpDown className="ml-2 h-4 w-4 shrink-0 opacity-50" />}
+          data-state={open ? 'open' : 'closed'}
+          className={cn('w-[200px]', !value && 'text-foreground-lighter')}
         >
           {value
             ? frameworks.find((framework) => framework.value === value)?.label
             : 'Select framework...'}
-        </Button>
+        </ComboboxTrigger>
       </PopoverTrigger>
       <PopoverContent className="w-[200px] p-0">
         <Command>

packages/ui/src/components/Button/Button.test.tsx

@@ -1,4 +1,5 @@
 import { fireEvent, render, screen } from '@testing-library/react'
+import { ChevronsUpDown } from 'lucide-react'
 import React from 'react'
 import { describe, expect, it } from 'vitest'
 
@@ -90,4 +91,27 @@ describe('#Button', () => {
 
     expect(ref.current).toBeInstanceOf(HTMLButtonElement)
   })
+
+  it('renders combobox triggers with select styling when role is combobox', () => {
+    render(
+      <Button role="combobox" variant="default" iconRight={<ChevronsUpDown />}>
+        Select publication
+      </Button>
+    )
+
+    const trigger = screen.getByRole('combobox')
+    expect(trigger).toHaveTextContent('Select publication')
+    expect(trigger).toHaveClass('bg-control-raised', 'border-strong', 'text-left')
+    expect(trigger).not.toHaveClass('bg-background')
+  })
+
+  it('renders default buttons with ChevronsUpDown as combobox triggers', () => {
+    render(
+      <Button variant="default" iconRight={<ChevronsUpDown className="opacity-50" />}>
+        Choose schema
+      </Button>
+    )
+
+    expect(screen.getByRole('combobox')).toHaveTextContent('Choose schema')
+  })
 })

packages/ui/src/components/Button/Button.tsx

@@ -8,6 +8,11 @@ import { cloneElement, forwardRef, isValidElement, ReactNode } from 'react'
 import { SIZE_VARIANTS, SIZE_VARIANTS_DEFAULT } from '../../lib/constants'
 import { cn } from '../../lib/utils/cn'
 import { getExplicitTabIndex } from '../../lib/utils/getExplicitTabIndex'
+import {
+  ComboboxTrigger,
+  isChevronsUpDownIcon,
+  shouldUseComboboxTrigger,
+} from '../shadcn/ui/select-trigger'
 
 export type ButtonVariantProps = VariantProps<typeof buttonVariants>
 const buttonVariants = cva(
@@ -204,15 +209,51 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
     ref
   ) => {
     const Comp = asChild ? Slot.Slot : 'button'
-    const { className, tabIndex } = props
+    const { className, tabIndex, role, disabled: disabledProp, onClick, ...restProps } = props
     const showIcon = loading || icon
     // decrecating 'showIcon' for rightIcon
     const _iconLeft: React.ReactNode = icon ?? iconLeft
     // if loading, button is disabled
-    const disabled = loading === true || props.disabled
+    const disabled = loading === true || disabledProp
 
     const computedTabIndex = getExplicitTabIndex(tabIndex, disabled)
 
+    const useComboboxTrigger = shouldUseComboboxTrigger({
+      asChild,
+      role,
+      variant,
+      iconRight,
+    })
+
+    if (useComboboxTrigger) {
+      const trailingIcon = loading ? (
+        <Loader2 className={cn('h-4 w-4 animate-spin', loadingVariants({ loading, variant }))} />
+      ) : iconRight && !isChevronsUpDownIcon(iconRight) ? (
+        iconRight
+      ) : undefined
+
+      return (
+        <ComboboxTrigger
+          ref={ref}
+          size={size}
+          type={type}
+          role="combobox"
+          disabled={disabled}
+          tabIndex={computedTabIndex}
+          leadingIcon={_iconLeft ?? undefined}
+          icon={trailingIcon}
+          className={cn(block && 'w-full', className)}
+          onClick={(e) => {
+            if (disabled) return e.preventDefault()
+            onClick?.(e)
+          }}
+          {...restProps}
+        >
+          {children}
+        </ComboboxTrigger>
+      )
+    }
+
     const renderIconContainer = (content: ReactNode) => (
       <div aria-hidden className={cn(IconContainerVariants({ size, variant }))}>
         {content}
@@ -224,14 +265,15 @@ const Button = forwardRef<HTMLButtonElement, ButtonProps>(
         ref={ref}
         data-size={size}
         type={type}
-        {...props}
+        role={role}
+        {...restProps}
         disabled={disabled}
         tabIndex={computedTabIndex}
         className={cn(buttonVariants({ variant, size, disabled, block, rounded }), className)}
         onClick={(e) => {
           // [Joshen] Prevents redirecting if Button is used with a link-based child element
           if (disabled) return e.preventDefault()
-          else props?.onClick?.(e)
+          else onClick?.(e)
         }}
       >
         {asChild ? (

packages/ui/src/components/shadcn/ui/select-trigger.test.tsx

@@ -0,0 +1,57 @@
+import { ChevronsUpDown } from 'lucide-react'
+import { describe, expect, it } from 'vitest'
+
+import { isChevronsUpDownIcon, shouldUseComboboxTrigger } from './select-trigger'
+
+describe('select-trigger helpers', () => {
+  it('detects ChevronsUpDown icons', () => {
+    expect(isChevronsUpDownIcon(<ChevronsUpDown />)).toBe(true)
+    expect(isChevronsUpDownIcon(<ChevronsUpDown className="opacity-50" />)).toBe(true)
+    expect(isChevronsUpDownIcon(null)).toBe(false)
+  })
+
+  it('delegates when role is combobox', () => {
+    expect(
+      shouldUseComboboxTrigger({
+        role: 'combobox',
+        variant: 'default',
+      })
+    ).toBe(true)
+  })
+
+  it('does not delegate danger combobox buttons', () => {
+    expect(
+      shouldUseComboboxTrigger({
+        role: 'combobox',
+        variant: 'danger',
+      })
+    ).toBe(false)
+  })
+
+  it('delegates default buttons with ChevronsUpDown', () => {
+    expect(
+      shouldUseComboboxTrigger({
+        variant: 'default',
+        iconRight: <ChevronsUpDown />,
+      })
+    ).toBe(true)
+  })
+
+  it('does not delegate text buttons with ChevronsUpDown', () => {
+    expect(
+      shouldUseComboboxTrigger({
+        variant: 'text',
+        iconRight: <ChevronsUpDown />,
+      })
+    ).toBe(false)
+  })
+
+  it('does not delegate asChild buttons', () => {
+    expect(
+      shouldUseComboboxTrigger({
+        asChild: true,
+        role: 'combobox',
+      })
+    ).toBe(false)
+  })
+})

packages/ui/src/components/shadcn/ui/select-trigger.tsx

@@ -0,0 +1,80 @@
+'use client'
+
+import { cva, type VariantProps } from 'class-variance-authority'
+import { ChevronDown, ChevronsUpDown } from 'lucide-react'
+import { isValidElement, type ReactNode } from 'react'
+import * as React from 'react'
+
+import { SIZE_VARIANTS, SIZE_VARIANTS_DEFAULT } from '../../../lib/constants'
+import { cn } from '../../../lib/utils/cn'
+import { getExplicitTabIndex } from '../../../lib/utils/getExplicitTabIndex'
+
+export const selectTriggerVariants = cva(
+  'flex w-full cursor-pointer items-center justify-between rounded-md border border-strong hover:border-control-hover bg-control-raised text-xs data-[placeholder]:text-foreground-lighter ring-border-control focus-ring disabled:cursor-not-allowed disabled:opacity-50 transition-colors duration-200 data-[state=open]:border-control-hover gap-2 [&>span]:truncate text-left',
+  {
+    variants: {
+      size: {
+        ...SIZE_VARIANTS,
+      },
+    },
+    defaultVariants: {
+      size: SIZE_VARIANTS_DEFAULT,
+    },
+  }
+)
+
+export type SelectTriggerVariantProps = VariantProps<typeof selectTriggerVariants>
+
+export function isChevronsUpDownIcon(icon: ReactNode): boolean {
+  if (!isValidElement(icon)) return false
+  return icon.type === ChevronsUpDown
+}
+
+export function shouldUseComboboxTrigger({
+  asChild,
+  role,
+  variant,
+  iconRight,
+}: {
+  asChild?: boolean
+  role?: string
+  variant?: string | null
+  iconRight?: ReactNode
+}): boolean {
+  if (asChild) return false
+  if (variant !== 'default') return false
+  if (role === 'combobox') return true
+  return isChevronsUpDownIcon(iconRight)
+}
+
+const ComboboxTrigger = React.forwardRef<
+  HTMLButtonElement,
+  React.ButtonHTMLAttributes<HTMLButtonElement> &
+    SelectTriggerVariantProps & {
+      icon?: React.ReactNode
+      leadingIcon?: React.ReactNode
+    }
+>(({ className, children, disabled, icon, leadingIcon, size, tabIndex, ...props }, ref) => {
+  const computedTabIndex = getExplicitTabIndex(tabIndex, disabled)
+
+  return (
+    <button
+      ref={ref}
+      type="button"
+      role="combobox"
+      disabled={disabled}
+      className={cn(selectTriggerVariants({ size }), className)}
+      tabIndex={computedTabIndex}
+      {...props}
+    >
+      {leadingIcon ? <span className="shrink-0 text-foreground-lighter">{leadingIcon}</span> : null}
+      <span className="min-w-0 flex-1 truncate text-left">{children}</span>
+      {icon ?? (
+        <ChevronDown className="h-4 w-4 text-foreground-lighter shrink-0" strokeWidth={1.5} />
+      )}
+    </button>
+  )
+})
+ComboboxTrigger.displayName = 'ComboboxTrigger'
+
+export { ComboboxTrigger }

packages/ui/src/components/shadcn/ui/select.test.tsx

@@ -1,7 +1,7 @@
 import { render, screen } from '@testing-library/react'
 import { describe, expect, it } from 'vitest'
 
-import { ComboboxTrigger } from './select'
+import { ComboboxTrigger } from './select-trigger'
 
 describe('ComboboxTrigger', () => {
   it('matches the raised select trigger styling', () => {
@@ -9,7 +9,13 @@ describe('ComboboxTrigger', () => {
 
     const trigger = screen.getByRole('combobox')
     expect(trigger).toHaveTextContent('Select publication')
-    expect(trigger).toHaveClass('bg-control-raised', 'border-strong', 'focus-ring', 'text-left')
+    expect(trigger).toHaveClass(
+      'bg-control-raised',
+      'border-strong',
+      'cursor-pointer',
+      'focus-ring',
+      'text-left'
+    )
     expect(trigger).not.toHaveClass('bg-field')
   })
 })

packages/ui/src/components/shadcn/ui/select.tsx

@@ -1,27 +1,22 @@
 'use client'
 
-import { cva, VariantProps } from 'class-variance-authority'
+import { VariantProps } from 'class-variance-authority'
 import { Check, ChevronDown, ChevronUp } from 'lucide-react'
 import { Select as SelectPrimitive } from 'radix-ui'
 import * as React from 'react'
 
-import { SIZE_VARIANTS, SIZE_VARIANTS_DEFAULT } from '../../../lib/constants'
 import { cn } from '../../../lib/utils/cn'
-import { getExplicitTabIndex } from '../../../lib/utils/getExplicitTabIndex'
+import { ComboboxTrigger, selectTriggerVariants } from './select-trigger'
 
 const Select = SelectPrimitive.Root
 
 const SelectGroup = SelectPrimitive.Group
 
-const selectTriggerClassName =
-  'flex w-full items-center justify-between rounded-md border border-strong hover:border-control-hover bg-control-raised text-xs data-[placeholder]:text-foreground-lighter ring-border-control focus-ring disabled:cursor-not-allowed disabled:opacity-50 transition-colors duration-200 data-[state=open]:border-control-hover gap-2 [&>span]:truncate text-left'
-
 // If placeholder is a string, wrap it in a span. This is to avoid page crashes when using Google Translate.
 // https://github.com/radix-ui/primitives/issues/2578#issuecomment-1890801041 for more info.
 const SelectValue = React.forwardRef<
   React.ElementRef<typeof SelectPrimitive.Value>,
-  React.ComponentPropsWithoutRef<typeof SelectPrimitive.Value> &
-    VariantProps<typeof SelectTriggerVariants>
+  React.ComponentPropsWithoutRef<typeof SelectPrimitive.Value>
 >(({ placeholder, ...props }, ref) => (
   <SelectPrimitive.Value
     placeholder={typeof placeholder === 'string' ? <span>{placeholder}</span> : placeholder}
@@ -32,25 +27,17 @@ const SelectValue = React.forwardRef<
 
 SelectValue.displayName = SelectPrimitive.Value.displayName
 
-const SelectTriggerVariants = cva('', {
-  variants: {
-    size: {
-      ...SIZE_VARIANTS,
-    },
-  },
-  defaultVariants: {
-    size: SIZE_VARIANTS_DEFAULT,
-  },
-})
+type SelectTriggerSize = NonNullable<VariantProps<typeof selectTriggerVariants>['size']>
 
 const SelectTrigger = React.forwardRef<
   React.ElementRef<typeof SelectPrimitive.Trigger>,
-  React.ComponentPropsWithoutRef<typeof SelectPrimitive.Trigger> &
-    VariantProps<typeof SelectTriggerVariants>
+  React.ComponentPropsWithoutRef<typeof SelectPrimitive.Trigger> & {
+    size?: SelectTriggerSize
+  }
 >(({ className, children, size, ...props }, ref) => (
   <SelectPrimitive.Trigger
     ref={ref}
-    className={cn(selectTriggerClassName, SelectTriggerVariants({ size }), className)}
+    className={cn(selectTriggerVariants({ size }), className)}
     tabIndex={0}
     {...props}
   >
@@ -62,34 +49,6 @@ const SelectTrigger = React.forwardRef<
 ))
 SelectTrigger.displayName = SelectPrimitive.Trigger.displayName
 
-const ComboboxTrigger = React.forwardRef<
-  HTMLButtonElement,
-  React.ButtonHTMLAttributes<HTMLButtonElement> &
-    VariantProps<typeof SelectTriggerVariants> & {
-      icon?: React.ReactNode
-    }
->(({ className, children, disabled, icon, size, tabIndex, ...props }, ref) => {
-  const computedTabIndex = getExplicitTabIndex(tabIndex, disabled)
-
-  return (
-    <button
-      ref={ref}
-      type="button"
-      role="combobox"
-      disabled={disabled}
-      className={cn(selectTriggerClassName, SelectTriggerVariants({ size }), className)}
-      tabIndex={computedTabIndex}
-      {...props}
-    >
-      <span className="flex-1 truncate text-left">{children}</span>
-      {icon ?? (
-        <ChevronDown className="h-4 w-4 text-foreground-lighter shrink-0" strokeWidth={1.5} />
-      )}
-    </button>
-  )
-})
-ComboboxTrigger.displayName = 'ComboboxTrigger'
-
 const SelectScrollUpButton = React.forwardRef<
   React.ElementRef<typeof SelectPrimitive.ScrollUpButton>,
   React.ComponentPropsWithoutRef<typeof SelectPrimitive.ScrollUpButton>