diff --git a/apps/design-system/content/docs/components/combobox.mdx b/apps/design-system/content/docs/components/combobox.mdx index 21d55d94ae8..3ad33b0124e 100644 --- a/apps/design-system/content/docs/components/combobox.mdx +++ b/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 ( - + diff --git a/apps/design-system/registry/default/example/combobox-demo.tsx b/apps/design-system/registry/default/example/combobox-demo.tsx index 5216f8b4ada..280d9b772b5 100644 --- a/apps/design-system/registry/default/example/combobox-demo.tsx +++ b/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 ( - + diff --git a/packages/ui/src/components/Button/Button.test.tsx b/packages/ui/src/components/Button/Button.test.tsx index 9eaed6e17f1..16a35b839ee 100644 --- a/packages/ui/src/components/Button/Button.test.tsx +++ b/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( + + ) + + 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( + + ) + + expect(screen.getByRole('combobox')).toHaveTextContent('Choose schema') + }) }) diff --git a/packages/ui/src/components/Button/Button.tsx b/packages/ui/src/components/Button/Button.tsx index ff326fb121d..c2bef8f0e44 100644 --- a/packages/ui/src/components/Button/Button.tsx +++ b/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 const buttonVariants = cva( @@ -204,15 +209,51 @@ const Button = forwardRef( 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 ? ( + + ) : iconRight && !isChevronsUpDownIcon(iconRight) ? ( + iconRight + ) : undefined + + return ( + { + if (disabled) return e.preventDefault() + onClick?.(e) + }} + {...restProps} + > + {children} + + ) + } + const renderIconContainer = (content: ReactNode) => (
{content} @@ -224,14 +265,15 @@ const Button = forwardRef( 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 ? ( diff --git a/packages/ui/src/components/shadcn/ui/select-trigger.test.tsx b/packages/ui/src/components/shadcn/ui/select-trigger.test.tsx new file mode 100644 index 00000000000..37064b6f724 --- /dev/null +++ b/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()).toBe(true) + expect(isChevronsUpDownIcon()).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: , + }) + ).toBe(true) + }) + + it('does not delegate text buttons with ChevronsUpDown', () => { + expect( + shouldUseComboboxTrigger({ + variant: 'text', + iconRight: , + }) + ).toBe(false) + }) + + it('does not delegate asChild buttons', () => { + expect( + shouldUseComboboxTrigger({ + asChild: true, + role: 'combobox', + }) + ).toBe(false) + }) +}) diff --git a/packages/ui/src/components/shadcn/ui/select-trigger.tsx b/packages/ui/src/components/shadcn/ui/select-trigger.tsx new file mode 100644 index 00000000000..cb73d909421 --- /dev/null +++ b/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 + +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 & + SelectTriggerVariantProps & { + icon?: React.ReactNode + leadingIcon?: React.ReactNode + } +>(({ className, children, disabled, icon, leadingIcon, size, tabIndex, ...props }, ref) => { + const computedTabIndex = getExplicitTabIndex(tabIndex, disabled) + + return ( + + ) +}) +ComboboxTrigger.displayName = 'ComboboxTrigger' + +export { ComboboxTrigger } diff --git a/packages/ui/src/components/shadcn/ui/select.test.tsx b/packages/ui/src/components/shadcn/ui/select.test.tsx index 2eb308a0773..8f119f8560f 100644 --- a/packages/ui/src/components/shadcn/ui/select.test.tsx +++ b/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') }) }) diff --git a/packages/ui/src/components/shadcn/ui/select.tsx b/packages/ui/src/components/shadcn/ui/select.tsx index f691fcc3cdb..a209050df8f 100644 --- a/packages/ui/src/components/shadcn/ui/select.tsx +++ b/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, - React.ComponentPropsWithoutRef & - VariantProps + React.ComponentPropsWithoutRef >(({ placeholder, ...props }, ref) => ( {placeholder} : 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['size']> const SelectTrigger = React.forwardRef< React.ElementRef, - React.ComponentPropsWithoutRef & - VariantProps + React.ComponentPropsWithoutRef & { + size?: SelectTriggerSize + } >(({ className, children, size, ...props }, ref) => ( @@ -62,34 +49,6 @@ const SelectTrigger = React.forwardRef< )) SelectTrigger.displayName = SelectPrimitive.Trigger.displayName -const ComboboxTrigger = React.forwardRef< - HTMLButtonElement, - React.ButtonHTMLAttributes & - VariantProps & { - icon?: React.ReactNode - } ->(({ className, children, disabled, icon, size, tabIndex, ...props }, ref) => { - const computedTabIndex = getExplicitTabIndex(tabIndex, disabled) - - return ( - - ) -}) -ComboboxTrigger.displayName = 'ComboboxTrigger' - const SelectScrollUpButton = React.forwardRef< React.ElementRef, React.ComponentPropsWithoutRef