Skip to content

Commit fa443ec

Browse files
pulashkirpulashkir
authored
Fix: Improve keyboard navigation and focus for BasicSelect (#127)
1 parent b1fc9a0 commit fa443ec

3 files changed

Lines changed: 160 additions & 45 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
"lint": "npx eslint",
1010
"format": "npx prettier --write .",
1111
"prisma:migrate": "npx prisma migrate dev",
12-
"test": "jest"
12+
"test": "npx jest"
1313
},
1414
"dependencies": {
1515
"@eslint/js": "^9.37.0",

‎src/__tests__/BasicSelect.test.tsx‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import { render, screen, fireEvent } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { BasicSelect } from '../components/common/BasicSelect';
4+
5+
describe('BasicSelect', () => {
6+
const mockItems = [
7+
{ label: 'Option 1', value: '1' },
8+
{ label: 'Option 2', value: '2' },
9+
{ label: 'Option 3', value: '3' },
10+
];
11+
12+
const mockOnChange = jest.fn();
13+
14+
beforeEach(() => {
15+
jest.clearAllMocks();
16+
});
17+
18+
it('renders with placeholder and opens on click', async () => {
19+
render(
20+
<BasicSelect
21+
placeholder="Select an option"
22+
items={mockItems}
23+
onChange={mockOnChange}
24+
/>
25+
);
26+
27+
expect(screen.getByText('Select an option')).toBeInTheDocument();
28+
29+
await userEvent.click(screen.getByRole('combobox'));
30+
expect(screen.getByText('Option 1')).toBeInTheDocument();
31+
expect(screen.getByText('Option 2')).toBeInTheDocument();
32+
expect(screen.getByText('Option 3')).toBeInTheDocument();
33+
});
34+
35+
it('allows selecting an option with click', async () => {
36+
render(
37+
<BasicSelect
38+
placeholder="Select an option"
39+
items={mockItems}
40+
onChange={mockOnChange}
41+
/>
42+
);
43+
44+
await userEvent.click(screen.getByRole('combobox'));
45+
await userEvent.click(screen.getByText('Option 2'));
46+
47+
expect(mockOnChange).toHaveBeenCalledWith('2');
48+
expect(screen.queryByText('Option 1')).not.toBeInTheDocument(); // Menu should close
49+
expect(screen.getByText('Option 2')).toBeInTheDocument(); // Selected value
50+
});
51+
52+
it('navigates options with arrow keys and selects with Enter', async () => {
53+
render(
54+
<BasicSelect
55+
placeholder="Select an option"
56+
items={mockItems}
57+
onChange={mockOnChange}
58+
/>
59+
);
60+
61+
const combobox = screen.getByRole('combobox');
62+
await userEvent.click(combobox); // Open the select
63+
64+
// Navigate down to Option 2
65+
fireEvent.keyDown(combobox, { key: 'ArrowDown' }); // Focus Option 1
66+
fireEvent.keyDown(combobox, { key: 'ArrowDown' }); // Focus Option 2
67+
68+
// Select Option 2 with Enter
69+
fireEvent.keyDown(combobox, { key: 'Enter' });
70+
71+
expect(mockOnChange).toHaveBeenCalledWith('2');
72+
expect(screen.queryByText('Option 1')).not.toBeInTheDocument(); // Menu should close
73+
expect(screen.getByText('Option 2')).toBeInTheDocument(); // Selected value
74+
});
75+
76+
it('closes the select with Escape key', async () => {
77+
render(
78+
<BasicSelect
79+
placeholder="Select an option"
80+
items={mockItems}
81+
onChange={mockOnChange}
82+
/>
83+
);
84+
85+
const combobox = screen.getByRole('combobox');
86+
await userEvent.click(combobox); // Open the select
87+
88+
expect(screen.getByText('Option 1')).toBeInTheDocument(); // Menu is open
89+
90+
fireEvent.keyDown(combobox, { key: 'Escape' });
91+
92+
expect(screen.queryByText('Option 1')).not.toBeInTheDocument(); // Menu should close
93+
});
94+
95+
it('should clear selection when allowEmpty is true and clear button is clicked', async () => {
96+
render(
97+
<BasicSelect
98+
placeholder="Select an option"
99+
items={mockItems}
100+
onChange={mockOnChange}
101+
value="1"
102+
allowEmpty={true}
103+
/>
104+
);
105+
106+
expect(screen.getByText('Option 1')).toBeInTheDocument();
107+
108+
const clearButton = screen.getByRole('button', { name: /clear/i });
109+
await userEvent.click(clearButton);
110+
111+
expect(mockOnChange).toHaveBeenCalledWith(null);
112+
expect(screen.getByText('Select an option')).toBeInTheDocument();
113+
});
114+
});

‎src/components/common/BasicSelect.tsx‎

Lines changed: 45 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -42,52 +42,53 @@ export function BasicSelect({
4242
)}
4343
{helperText && <FormDescription>{helperText}</FormDescription>}
4444
<div className='relative'>
45-
<Select
46-
disabled={disabled}
47-
onValueChange={onChange}
48-
value={value?.toString() ?? ''}
49-
onOpenChange={(isOpen) => {
50-
if (!isOpen && !hasValue) {
51-
onChange(null);
52-
}
53-
}}
54-
>
55-
<SelectTrigger
56-
className={cn(
57-
'bg-transparent hover:bg-transparent focus:ring-0 border-neutral-700/60 w-full dark:bg-neutral-900 dark:text-white dark:border-neutral-700',
58-
allowEmpty && hasValue && 'pr-12',
59-
className
60-
)}
61-
>
62-
<SelectValue placeholder={placeholder} />
63-
</SelectTrigger>
64-
<SelectContent className='dark:bg-neutral-900 dark:border-neutral-700'>
65-
<SelectGroup>
66-
{items.map(({ value, label, disabled }) => (
67-
<SelectItem
68-
key={value}
69-
value={value.toString()}
70-
disabled={disabled}
71-
className='dark:text-white dark:focus:bg-neutral-800 dark:focus:text-white'
72-
>
73-
{label}
74-
</SelectItem>
75-
))}
76-
</SelectGroup>
77-
</SelectContent>
78-
</Select>
79-
{allowEmpty && hasValue && (
80-
<button
81-
type='button'
82-
onClick={(e) => {
83-
e.stopPropagation();
84-
onChange(null);
45+
<div className="relative flex items-center">
46+
<Select
47+
disabled={disabled}
48+
onValueChange={onChange}
49+
value={value?.toString() ?? ''}
50+
onOpenChange={(isOpen) => {
51+
if (!isOpen && !hasValue) {
52+
onChange(null);
53+
}
8554
}}
86-
className='absolute right-2 top-1/2 -translate-y-1/2 hover:bg-neutral-100 dark:hover:bg-neutral-800 p-1.5 rounded-sm'
8755
>
88-
<X className='h-3.5 w-3.5 text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-neutral-50' />
89-
</button>
90-
)}
56+
<SelectTrigger
57+
className={cn(
58+
'bg-transparent hover:bg-transparent focus:ring-0 border-neutral-700/60 w-full dark:bg-neutral-900 dark:text-white dark:border-neutral-700',
59+
allowEmpty && hasValue && 'pr-12',
60+
className
61+
)}
62+
>
63+
<SelectValue placeholder={placeholder} />
64+
</SelectTrigger>
65+
<SelectContent className='dark:bg-neutral-900 dark:border-neutral-700'>
66+
<SelectGroup>
67+
{items.map(({ value, label, disabled }) => (
68+
<SelectItem
69+
key={value}
70+
value={value.toString()}
71+
disabled={disabled}
72+
className='dark:text-white dark:focus:bg-neutral-800 dark:focus:text-white'
73+
>
74+
{label}
75+
</SelectItem>
76+
))}
77+
</SelectGroup>
78+
</SelectContent>
79+
</Select>
80+
{allowEmpty && hasValue && (
81+
<button
82+
type='button'
83+
onClick={() => {
84+
onChange(null);
85+
}}
86+
className='absolute right-2 top-1/2 -translate-y-1/2 hover:bg-neutral-100 dark:hover:bg-neutral-800 p-1.5 rounded-sm'
87+
>
88+
<X className='h-3.5 w-3.5 text-neutral-500 hover:text-neutral-900 dark:text-neutral-400 dark:hover:text-neutral-50' />
89+
</button>
90+
)}
91+
</div>
9192
</div>
9293
</div>
9394
);

0 commit comments

Comments
 (0)