Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions packages/apollo-wind/src/components/ui/combobox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,13 @@ function MultiSelectCombobox() {
variant="outline"
role="combobox"
aria-expanded={open}
className="w-[320px] justify-between future:gap-4 future:rounded-xl future:border-0 future:bg-surface-overlay future:font-normal future:text-muted-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background"
className="w-[320px] justify-between future:gap-4 future:rounded-xl future:border-0 future:bg-surface-overlay future:font-normal future:text-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background"
>
{selected.length > 0 ? `${selected.length} selected` : 'Select frameworks...'}
{selected.length > 0 ? (
`${selected.length} selected`
) : (
<span className="text-foreground-muted">Select frameworks...</span>
)}
<ChevronDown className="opacity-50" />
</Button>
</PopoverTrigger>
Expand Down Expand Up @@ -182,15 +186,15 @@ function CustomDisplayCombobox() {
variant="outline"
role="combobox"
aria-expanded={open}
className="w-[320px] justify-between future:gap-4 future:rounded-xl future:border-0 future:bg-surface-overlay future:font-normal future:text-muted-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background"
className="w-[320px] justify-between future:gap-4 future:rounded-xl future:border-0 future:bg-surface-overlay future:font-normal future:text-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background"
>
{selected ? (
<span className="flex items-center gap-2">
<span>{selected.emoji}</span>
<span>{selected.label}</span>
</span>
) : (
'Select issue type...'
<span className="text-foreground-muted">Select issue type...</span>
)}
<ChevronDown className="opacity-50" />
</Button>
Expand Down Expand Up @@ -284,9 +288,13 @@ function AsyncCombobox() {
variant="outline"
role="combobox"
aria-expanded={open}
className="w-[280px] justify-between future:gap-4 future:rounded-xl future:border-0 future:bg-surface-overlay future:font-normal future:text-muted-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background"
className="w-[280px] justify-between future:gap-4 future:rounded-xl future:border-0 future:bg-surface-overlay future:font-normal future:text-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background"
>
{selected ? selected.label : 'Search libraries...'}
{selected ? (
selected.label
) : (
<span className="text-foreground-muted">Search libraries...</span>
)}
<ChevronDown className="opacity-50" />
</Button>
</PopoverTrigger>
Expand Down
25 changes: 25 additions & 0 deletions packages/apollo-wind/src/components/ui/combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,29 @@ describe('Combobox', () => {
});
});
});

describe('Value vs placeholder color', () => {
it('mutes the placeholder via its own span', () => {
render(<Combobox items={mockItems} placeholder="Select an option..." />);
const placeholder = screen.getByText('Select an option...');
expect(placeholder.tagName).toBe('SPAN');
expect(placeholder).toHaveClass('text-foreground-muted');
});

it('renders a selected value at full strength', () => {
render(<Combobox items={mockItems} value="apple" />);
const trigger = screen.getByRole('combobox');
expect(trigger).toHaveTextContent('Apple');
expect(trigger.querySelector('.text-foreground-muted')).toBeNull();
});

// The outline Button variant mutes its own text in Future themes, so the
// trigger has to override it rather than rely on removing the class.
it('overrides the outline variant so the trigger is not globally muted', () => {
render(<Combobox items={mockItems} />);
const trigger = screen.getByRole('combobox');
expect(trigger).toHaveClass('future:text-foreground');
expect(trigger).not.toHaveClass('future:text-muted-foreground');
});
});
});
8 changes: 6 additions & 2 deletions packages/apollo-wind/src/components/ui/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,16 @@ export const Combobox = React.forwardRef<HTMLButtonElement, ComboboxProps>(funct
aria-expanded={open}
aria-label={selectedItem ? selectedItem.label : placeholder}
className={cn(
'w-[280px] justify-between future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:hover:bg-surface-hover future:font-normal future:text-muted-foreground future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
'w-[280px] justify-between future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:hover:bg-surface-hover future:font-normal future:text-foreground future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
Comment thread
BenGSchulz marked this conversation as resolved.
className
)}
disabled={disabled}
>
{selectedItem ? selectedItem.label : placeholder}
{selectedItem ? (
selectedItem.label
) : (
<span className="text-foreground-muted">{placeholder}</span>
)}
<ChevronDown className="opacity-50" />
</Button>
</PopoverTrigger>
Expand Down
67 changes: 67 additions & 0 deletions packages/apollo-wind/src/components/ui/date-picker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,70 @@ describe('DateRangePicker', () => {
});
});
});

describe('value vs placeholder color', () => {
it('mutes the DatePicker placeholder via its own span', () => {
render(<DatePicker placeholder="Pick a date" />);
const placeholder = screen.getByText('Pick a date');
expect(placeholder.tagName).toBe('SPAN');
expect(placeholder).toHaveClass('text-foreground-muted');
});

it('mutes the DateRangePicker placeholder via its own span', () => {
render(<DateRangePicker placeholder="Pick a date range" />);
const placeholder = screen.getByText('Pick a date range');
expect(placeholder.tagName).toBe('SPAN');
expect(placeholder).toHaveClass('text-foreground-muted');
});

it('renders a selected date at full strength', () => {
render(<DatePicker value={new Date(2024, 5, 15)} />);
const trigger = screen.getByRole('button');
expect(trigger).toHaveTextContent(/June 15/);
expect(trigger.querySelector('.text-foreground-muted')).toBeNull();
});

// A range with only `from` set still renders a value, so the trigger must not
// fall back to the placeholder colour.
it('renders a partial range at full strength', () => {
render(<DateRangePicker value={{ from: new Date(2024, 5, 15), to: undefined }} />);
const trigger = screen.getByRole('button');
expect(trigger).toHaveTextContent(/Jun 15/);
expect(trigger.querySelector('.text-foreground-muted')).toBeNull();
});

// The outline Button variant mutes its own text in Future themes, so each
// trigger has to override it rather than rely on removing the class.
it.each([
['DatePicker', <DatePicker key="d" />],
['DateRangePicker', <DateRangePicker key="r" />],
])('overrides the outline variant on %s', (_name, element) => {
render(element);
const trigger = screen.getByRole('button');
expect(trigger).toHaveClass('future:text-foreground');
expect(trigger).not.toHaveClass('future:text-muted-foreground');
});
});

describe('trigger icon color', () => {
it.each([
['DatePicker', <DatePicker key="d" />],
['DateRangePicker', <DateRangePicker key="r" />],
])('mutes the %s icon and brightens it on hover', (_name, element) => {
render(element);
const trigger = screen.getByRole('button');
expect(trigger).toHaveClass('[&>svg]:text-foreground-muted');
expect(trigger).toHaveClass('hover:[&>svg]:text-accent-foreground');
});

// The [&>svg] selector only matches a direct child, so wrapping the icon
// would silently drop both rules.
it.each([
['DatePicker', <DatePicker key="d" />],
['DateRangePicker', <DateRangePicker key="r" />],
])('keeps the %s icon a direct child of the trigger', (_name, element) => {
render(element);
const trigger = screen.getByRole('button');
expect(trigger.querySelector(':scope > svg')).not.toBeNull();
});
});
18 changes: 10 additions & 8 deletions packages/apollo-wind/src/components/ui/date-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,18 @@ export const DatePicker = React.forwardRef<HTMLButtonElement, DatePickerProps>(f
variant="outline"
aria-label={value ? `Selected date: ${format(value, 'PPP')}` : placeholder}
className={cn(
'w-full justify-start text-left font-normal',
'future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:text-muted-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
!value && 'text-muted-foreground',
'w-full justify-start text-left font-normal [&>svg]:text-foreground-muted hover:[&>svg]:text-accent-foreground',
'future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:text-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
className
)}
disabled={disabled}
>
<CalendarIcon />
{value ? format(value, 'PPP') : <span>{placeholder}</span>}
{value ? (
format(value, 'PPP')
) : (
<span className="text-foreground-muted">{placeholder}</span>
)}
</Button>
</PopoverTrigger>
<PopoverContent className="w-auto p-0">
Expand Down Expand Up @@ -94,9 +97,8 @@ export const DateRangePicker = React.forwardRef<HTMLButtonElement, DateRangePick
: placeholder
}
className={cn(
'w-[300px] justify-start text-left font-normal',
'future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:text-muted-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
!value && 'text-muted-foreground',
'w-[300px] justify-start text-left font-normal [&>svg]:text-foreground-muted hover:[&>svg]:text-accent-foreground',
'future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:text-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
className
)}
disabled={disabled}
Expand All @@ -111,7 +113,7 @@ export const DateRangePicker = React.forwardRef<HTMLButtonElement, DateRangePick
format(value.from, 'LLL dd, y')
)
) : (
<span>{placeholder}</span>
<span className="text-foreground-muted">{placeholder}</span>
)}
</Button>
</PopoverTrigger>
Expand Down
39 changes: 39 additions & 0 deletions packages/apollo-wind/src/components/ui/datetime-picker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,43 @@ describe('DateTimePicker', () => {
expect(screen.getByRole('button', { name: 'Done' })).toBeDisabled();
});
});

describe('value vs placeholder color', () => {
it('mutes the placeholder via its own span', () => {
render(<DateTimePicker placeholder="Pick a date and time" />);
const placeholder = screen.getByText('Pick a date and time');
expect(placeholder.tagName).toBe('SPAN');
expect(placeholder).toHaveClass('text-foreground-muted');
});

it('renders a selected value at full strength', () => {
const { container } = render(<DateTimePicker value={new Date(2024, 5, 15, 14, 30)} />);
expect(screen.getByRole('button')).toHaveTextContent(/June 15/);
expect(container.querySelector('.text-foreground-muted')).toBeNull();
});

it('overrides the outline variant so the trigger is not globally muted', () => {
render(<DateTimePicker />);
const trigger = screen.getByRole('button');
expect(trigger).toHaveClass('future:text-foreground');
expect(trigger).not.toHaveClass('future:text-muted-foreground');
});
});

describe('trigger icon color', () => {
it('mutes the icon and brightens it on hover', () => {
render(<DateTimePicker />);
const trigger = screen.getByRole('button');
expect(trigger).toHaveClass('[&>svg]:text-foreground-muted');
expect(trigger).toHaveClass('hover:[&>svg]:text-accent-foreground');
});

// The [&>svg] selector only matches a direct child, so wrapping the icon
// would silently drop both rules.
it('keeps the icon a direct child of the trigger', () => {
render(<DateTimePicker />);
const trigger = screen.getByRole('button');
expect(trigger.querySelector(':scope > svg')).not.toBeNull();
});
});
});
14 changes: 9 additions & 5 deletions packages/apollo-wind/src/components/ui/datetime-picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ export const DateTimePicker = React.forwardRef<HTMLButtonElement, DateTimePicker
};

const formatDisplayValue = () => {
if (!selectedDate) return placeholder;
// Placeholder is rendered by the caller; this only narrows for format().
if (!selectedDate) return null;
Comment thread
BenGSchulz marked this conversation as resolved.
const datePart = format(selectedDate, 'PPP');
const timePart = format(selectedDate, use12Hour ? 'hh:mm a' : 'HH:mm');
return `${datePart} at ${timePart}`;
Expand All @@ -78,15 +79,18 @@ export const DateTimePicker = React.forwardRef<HTMLButtonElement, DateTimePicker
ref={ref}
variant="outline"
className={cn(
'w-full justify-start text-left font-normal',
'future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:text-muted-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
!selectedDate && 'text-muted-foreground',
'w-full justify-start text-left font-normal [&>svg]:text-foreground-muted hover:[&>svg]:text-accent-foreground',
'future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:text-foreground future:hover:bg-surface-hover future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
className
)}
disabled={disabled}
>
<CalendarIcon className="mr-2 h-4 w-4" />
{formatDisplayValue()}
{selectedDate ? (
formatDisplayValue()
) : (
<span className="text-foreground-muted">{placeholder}</span>
)}
</Button>
</PopoverTrigger>
<PopoverContent className="w-auto p-0" align="start">
Expand Down
23 changes: 23 additions & 0 deletions packages/apollo-wind/src/components/ui/multi-select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -236,4 +236,27 @@ describe('MultiSelect', () => {
render(<MultiSelect ref={ref} options={mockOptions} selected={[]} onChange={onChange} />);
expect(ref.current).toBeInstanceOf(HTMLDivElement);
});

describe('value vs placeholder color', () => {
it('mutes the placeholder via its own span', () => {
render(
<MultiSelect
options={mockOptions}
selected={[]}
onChange={vi.fn()}
placeholder="Select frameworks..."
/>
);
const placeholder = screen.getByText('Select frameworks...');
expect(placeholder.tagName).toBe('SPAN');
expect(placeholder).toHaveClass('text-foreground-muted');
});

it('overrides the outline variant so the trigger is not globally muted', () => {
render(<MultiSelect options={mockOptions} selected={[]} onChange={vi.fn()} />);
const trigger = screen.getByRole('combobox');
expect(trigger).toHaveClass('future:text-foreground');
expect(trigger).not.toHaveClass('future:text-muted-foreground');
});
});
});
6 changes: 2 additions & 4 deletions packages/apollo-wind/src/components/ui/multi-select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,14 @@ const MultiSelect = React.forwardRef<HTMLDivElement, MultiSelectProps>(
: placeholder
}
className={cn(
'w-full justify-between future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:hover:bg-surface-hover future:font-normal future:text-muted-foreground future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
'w-full justify-between future:rounded-xl future:border-0 future:bg-surface-overlay future:px-4 future:gap-4 future:hover:bg-surface-hover future:font-normal future:text-foreground future:focus-visible:ring-offset-2 future:focus-visible:ring-offset-background',
selected.length > 0 ? 'h-auto min-h-10' : 'h-10'
)}
disabled={disabled}
>
<div className="flex flex-wrap gap-1 flex-1">
{selected.length === 0 ? (
<span className="text-muted-foreground future:text-foreground-muted">
{placeholder}
</span>
<span className="text-foreground-muted">{placeholder}</span>
) : (
selected.map((value) => {
const option = options.find((opt) => opt.value === value);
Expand Down
22 changes: 22 additions & 0 deletions packages/apollo-wind/src/components/ui/select.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -313,4 +313,26 @@ describe('Select', () => {
});
});
});

// SelectTrigger is not Button-based: Radix's data-placeholder already mutes the
// empty state in every theme. A future:text-foreground here would outrank it.
describe('value vs placeholder color', () => {
it('mutes the empty state through data-placeholder, not a blanket class', () => {
render(<SelectExample />);
const trigger = screen.getByRole('combobox');
expect(trigger).toHaveAttribute('data-placeholder');
expect(trigger).toHaveClass('data-[placeholder]:text-muted-foreground');
expect(trigger).not.toHaveClass('future:text-muted-foreground');
expect(trigger).not.toHaveClass('future:text-foreground');
});

it('drops data-placeholder once a value is selected', async () => {
const user = userEvent.setup();
render(<SelectExample />);
const trigger = screen.getByRole('combobox');
await user.click(trigger);
await user.click(await screen.findByRole('option', { name: 'Apple' }));
await waitFor(() => expect(trigger).not.toHaveAttribute('data-placeholder'));
});
Comment thread
Copilot marked this conversation as resolved.
});
});
2 changes: 1 addition & 1 deletion packages/apollo-wind/src/components/ui/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const SelectTrigger = React.forwardRef<
ref={ref}
data-slot="select-trigger"
className={cn(
'flex h-9 w-full cursor-pointer items-center justify-between rounded-md border border-input bg-transparent px-3 py-1 text-base transition-colors data-[placeholder]:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-ring focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50 md:text-sm [&>span]:line-clamp-1 future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:hover:bg-surface-hover future:px-4 future:gap-4 future:font-normal future:text-muted-foreground',
'flex h-9 w-full cursor-pointer items-center justify-between rounded-md border border-input bg-transparent px-3 py-1 text-base transition-colors data-[placeholder]:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-ring focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50 md:text-sm [&>span]:line-clamp-1 future:h-10 future:rounded-xl future:border-0 future:bg-surface-overlay future:hover:bg-surface-hover future:px-4 future:gap-4 future:font-normal',
className
)}
{...props}
Expand Down
Loading