From c14c0090c8e61d7d010f137d69867677a07f7914 Mon Sep 17 00:00:00 2001 From: felicia-haggqvist Date: Tue, 16 Jul 2024 15:03:54 +0200 Subject: [PATCH 1/9] fix(combobox): add unit-tests --- packages/combobox/src/component.tsx | 2 +- tests/components/ComboBoxTest.tsx | 88 +++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 tests/components/ComboBoxTest.tsx diff --git a/packages/combobox/src/component.tsx b/packages/combobox/src/component.tsx index 26e6ad3a..efd19cd2 100644 --- a/packages/combobox/src/component.tsx +++ b/packages/combobox/src/component.tsx @@ -81,7 +81,7 @@ export const Combobox = forwardRef(({ id: pid, ); // eslint-disable-next-line - }, [options, disableStaticFiltering]); + }, [options, disableStaticFiltering, value]); useEffect(() => { if (disableStaticFiltering && currentOptions.length && currentOptions.length === 1 && !currentOptions.some((o) => o.value === value)) { diff --git a/tests/components/ComboBoxTest.tsx b/tests/components/ComboBoxTest.tsx new file mode 100644 index 00000000..fd7afe10 --- /dev/null +++ b/tests/components/ComboBoxTest.tsx @@ -0,0 +1,88 @@ +import React, { useState } from 'react'; + +import { render, screen, fireEvent } from '@testing-library/react'; +import { vi } from 'vitest'; + +import { Combobox } from '../../packages/combobox/src/component'; +import type { ComboboxProps } from '../../packages/combobox/src/props'; + +describe('Combobox', () => { + const defaultProps: ComboboxProps = { + id: 'combobox', + options: [{ value: 'Option 1' }, { value: 'Option 2' }, { value: 'Option 3' }], + value: '', + label: 'Test Combobox', + onChange: vi.fn(), + onSelect: vi.fn(), + }; + + const ComboboxWrapper = (props: Partial) => { + const [value, setValue] = useState(''); + + return ( + { + setValue(newValue); + if (props.onChange) props.onChange(newValue); + }} + /> + ); + }; + + it('renders without crashing', () => { + render(); + expect(screen.getByRole('combobox')).toBeInTheDocument(); + }); + + it('opens the options list on input focus', () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + }); + + it('filters options based on user input', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.change(input, { target: { value: 'Option 1' } }); + + const option1 = screen.queryByText('Option 1'); + const option2 = screen.queryByText('Option 2'); + console.log('Option 1:', option1); + console.log('Option 2:', option2); + expect(option1).toBeInTheDocument(); + expect(option2).toBeNull(); + }); + + it('selects an option on click', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + const option = await screen.findByText('Option 1'); + fireEvent.click(option); + expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1'); + }); + + it('navigates options with arrow keys', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(screen.getByText('Option 1')).toHaveClass('block cursor-pointer p-8 hover:s-bg-hover w-react-combobox-option'); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(screen.getByText('Option 2')).toHaveClass('block cursor-pointer p-8 hover:s-bg-hover w-react-combobox-option'); + fireEvent.keyDown(input, { key: 'ArrowUp' }); + expect(screen.getByText('Option 1')).toHaveClass('block cursor-pointer p-8 hover:s-bg-hover w-react-combobox-option'); + }); + + it('closes the options list on escape key press', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'Escape' }); + expect(screen.queryByRole('listbox')).toBeNull(); + }); +}); From 2e549554970bbb1a8c4e25162b1b3131cab678e3 Mon Sep 17 00:00:00 2001 From: felicia-haggqvist Date: Wed, 17 Jul 2024 11:57:24 +0200 Subject: [PATCH 2/9] fix(combobox): add more tests --- tests/components/ComboBoxTest.tsx | 115 ++++++++++++++++++++++++++++-- 1 file changed, 109 insertions(+), 6 deletions(-) diff --git a/tests/components/ComboBoxTest.tsx b/tests/components/ComboBoxTest.tsx index fd7afe10..4dbfa3e9 100644 --- a/tests/components/ComboBoxTest.tsx +++ b/tests/components/ComboBoxTest.tsx @@ -1,7 +1,7 @@ import React, { useState } from 'react'; -import { render, screen, fireEvent } from '@testing-library/react'; -import { vi } from 'vitest'; +import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; +import { beforeEach, vi } from 'vitest'; import { Combobox } from '../../packages/combobox/src/component'; import type { ComboboxProps } from '../../packages/combobox/src/props'; @@ -17,7 +17,7 @@ describe('Combobox', () => { }; const ComboboxWrapper = (props: Partial) => { - const [value, setValue] = useState(''); + const [value, setValue] = useState(props.value || ''); return ( { value={value} onChange={(newValue) => { setValue(newValue); + if (defaultProps?.onChange) defaultProps.onChange(newValue); if (props.onChange) props.onChange(newValue); }} + onSelect={(selectedValue) => { + if (defaultProps?.onSelect) defaultProps.onSelect(selectedValue); + if (props.onSelect) props.onSelect(selectedValue); + }} /> ); }; + beforeEach(() => { + cleanup(); + vi.clearAllMocks(); + }); + it('renders without crashing', () => { render(); expect(screen.getByRole('combobox')).toBeInTheDocument(); @@ -51,8 +61,6 @@ describe('Combobox', () => { const option1 = screen.queryByText('Option 1'); const option2 = screen.queryByText('Option 2'); - console.log('Option 1:', option1); - console.log('Option 2:', option2); expect(option1).toBeInTheDocument(); expect(option2).toBeNull(); }); @@ -63,7 +71,7 @@ describe('Combobox', () => { fireEvent.focus(input); const option = await screen.findByText('Option 1'); fireEvent.click(option); - expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1'); + await waitFor(() => expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1')); }); it('navigates options with arrow keys', async () => { @@ -85,4 +93,99 @@ describe('Combobox', () => { fireEvent.keyDown(input, { key: 'Escape' }); expect(screen.queryByRole('listbox')).toBeNull(); }); + + it('closes options list on blur', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.blur(input); + expect(screen.queryByRole('listbox')).toBeNull(); + }); + + it('does not close options list on blur if clicked inside', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + const option = await screen.findByText('Option 1'); + fireEvent.mouseDown(option); + setTimeout(() => { + fireEvent.blur(input); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + }, 0); + }); + + it('filters options based on input value when static filtering is disabled', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.change(input, { target: { value: 'Option 1' } }); + expect(screen.getByText('Option 1')).toBeInTheDocument(); + expect(screen.queryByText('Option 2')).toBeInTheDocument(); + }); + + it('navigates options with keyboard and selects an option', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'Enter' }); + expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1'); + }); + + it('navigates options with keyboard and closes list on Escape', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'Escape' }); + expect(screen.queryByRole('listbox')).toBeNull(); + }); + + it('handles special keys without breaking', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'PageDown' }); + fireEvent.keyDown(input, { key: 'Home' }); + fireEvent.keyDown(input, { key: 'End' }); + fireEvent.keyDown(input, { key: 'PageUp' }); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + }); + + it('handles onChange and onBlur correctly', async () => { + const onChange = vi.fn(); + const onBlur = vi.fn(); + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.change(input, { target: { value: 'Option 2' } }); + fireEvent.blur(input); + expect(onChange).toHaveBeenCalledWith('Option 2'); + expect(onBlur).toHaveBeenCalled(); + }); + + it('handles Backspace key', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.change(input, { target: { value: '' } }); + fireEvent.keyDown(input, { key: 'Backspace' }); + await waitFor(() => expect(defaultProps.onChange).toHaveBeenCalledWith('')); + }); + + it('handles Enter key without navigation option', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'Enter' }); + await waitFor(() => expect(defaultProps.onSelect).not.toHaveBeenCalled()); + }); + + it('handles input blur with selectOnBlur set to false', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.blur(input); + await waitFor(() => expect(defaultProps.onSelect).not.toHaveBeenCalled()); + }); }); From 8824a2d3bd8612ccde8ca57e188c7d8c478ec5fd Mon Sep 17 00:00:00 2001 From: felicia-haggqvist Date: Thu, 18 Jul 2024 11:31:22 +0200 Subject: [PATCH 3/9] fix(combobox): add more tests --- tests/components/ComboBoxTest.tsx | 65 +++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/tests/components/ComboBoxTest.tsx b/tests/components/ComboBoxTest.tsx index 4dbfa3e9..b2bfec02 100644 --- a/tests/components/ComboBoxTest.tsx +++ b/tests/components/ComboBoxTest.tsx @@ -5,6 +5,7 @@ import { beforeEach, vi } from 'vitest'; import { Combobox } from '../../packages/combobox/src/component'; import type { ComboboxProps } from '../../packages/combobox/src/props'; +import { Affix } from '../../packages/_helpers'; describe('Combobox', () => { const defaultProps: ComboboxProps = { @@ -188,4 +189,68 @@ describe('Combobox', () => { fireEvent.blur(input); await waitFor(() => expect(defaultProps.onSelect).not.toHaveBeenCalled()); }); + + it('sets the combobox open on navigation key if closed', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + }); + + it('dismisses the popover on Delete key press', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'Delete' }); + expect(screen.queryByRole('listbox')).toBeNull(); + }); + + it('sets empty options on select when disableStaticFiltering is true', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'Enter' }); + expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1'); + expect(screen.queryByText('Option 1')).toBeNull(); + }); + + it('renders TextField with Affix component correctly', async () => { + const ComboboxWithAffixWrapper = (props: Partial) => { + const [_, setValue] = useState(props.value || ''); + + return ( + + setValue('')} /> + + ); + }; + + render(); + + const clearButton = screen.getByLabelText('Clear text'); + expect(clearButton).toBeInTheDocument(); + fireEvent.click(clearButton); + expect(screen.getByRole('combobox')).toHaveValue(''); + }); + + it('matches text segments correctly', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(screen.getByRole('listbox')).toBeInTheDocument(); + fireEvent.change(input, { target: { value: 'Option 1' } }); + expect(screen.getByRole('listbox')).toHaveClass('w-react-combobox-match') + }); + + it('handles PageDown key correctly', async () => { + render(); + const input = screen.getByRole('combobox'); + fireEvent.focus(input); + fireEvent.keyDown(input, { key: 'PageDown' }); + expect(screen.getByText('Option 2')).toHaveClass('block cursor-pointer p-8 hover:s-bg-hover w-react-combobox-option'); + }); }); From 54103cbaed0d5cdb2d8ba6c4043f38d6fcd865e3 Mon Sep 17 00:00:00 2001 From: felicia-haggqvist Date: Thu, 18 Jul 2024 11:59:11 +0200 Subject: [PATCH 4/9] fix(combobox): refactor tests --- tests/components/ComboBoxTest.tsx | 29 ++++------------------------- 1 file changed, 4 insertions(+), 25 deletions(-) diff --git a/tests/components/ComboBoxTest.tsx b/tests/components/ComboBoxTest.tsx index b2bfec02..c17ac82e 100644 --- a/tests/components/ComboBoxTest.tsx +++ b/tests/components/ComboBoxTest.tsx @@ -103,18 +103,6 @@ describe('Combobox', () => { expect(screen.queryByRole('listbox')).toBeNull(); }); - it('does not close options list on blur if clicked inside', async () => { - render(); - const input = screen.getByRole('combobox'); - fireEvent.focus(input); - const option = await screen.findByText('Option 1'); - fireEvent.mouseDown(option); - setTimeout(() => { - fireEvent.blur(input); - expect(screen.getByRole('listbox')).toBeInTheDocument(); - }, 0); - }); - it('filters options based on input value when static filtering is disabled', async () => { render(); const input = screen.getByRole('combobox'); @@ -190,28 +178,19 @@ describe('Combobox', () => { await waitFor(() => expect(defaultProps.onSelect).not.toHaveBeenCalled()); }); - it('sets the combobox open on navigation key if closed', async () => { - render(); - const input = screen.getByRole('combobox'); - fireEvent.focus(input); - fireEvent.keyDown(input, { key: 'ArrowDown' }); - expect(screen.getByRole('listbox')).toBeInTheDocument(); - }); - - it('dismisses the popover on Delete key press', async () => { - render(); + it('dismisses the popover on Escape key press', async () => { + render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); - fireEvent.keyDown(input, { key: 'Delete' }); + fireEvent.keyDown(input, { key: 'Escape' }); expect(screen.queryByRole('listbox')).toBeNull(); }); it('sets empty options on select when disableStaticFiltering is true', async () => { - render(); + render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); fireEvent.keyDown(input, { key: 'ArrowDown' }); - fireEvent.keyDown(input, { key: 'ArrowDown' }); fireEvent.keyDown(input, { key: 'Enter' }); expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1'); expect(screen.queryByText('Option 1')).toBeNull(); From ac27b5080ce5f4f2bd810126ee2e5ff604683655 Mon Sep 17 00:00:00 2001 From: felicia-haggqvist Date: Thu, 18 Jul 2024 12:03:41 +0200 Subject: [PATCH 5/9] fix(combobox): fix lint warning and remove redundant async in tests --- tests/components/ComboBoxTest.tsx | 39 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/components/ComboBoxTest.tsx b/tests/components/ComboBoxTest.tsx index c17ac82e..fbb1656f 100644 --- a/tests/components/ComboBoxTest.tsx +++ b/tests/components/ComboBoxTest.tsx @@ -3,9 +3,9 @@ import React, { useState } from 'react'; import { render, screen, fireEvent, waitFor, cleanup } from '@testing-library/react'; import { beforeEach, vi } from 'vitest'; +import { Affix } from '../../packages/_helpers'; import { Combobox } from '../../packages/combobox/src/component'; import type { ComboboxProps } from '../../packages/combobox/src/props'; -import { Affix } from '../../packages/_helpers'; describe('Combobox', () => { const defaultProps: ComboboxProps = { @@ -55,7 +55,7 @@ describe('Combobox', () => { expect(screen.getByRole('listbox')).toBeInTheDocument(); }); - it('filters options based on user input', async () => { + it('filters options based on user input', () => { render(); const input = screen.getByRole('combobox'); fireEvent.change(input, { target: { value: 'Option 1' } }); @@ -75,7 +75,7 @@ describe('Combobox', () => { await waitFor(() => expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1')); }); - it('navigates options with arrow keys', async () => { + it('navigates options with arrow keys', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -87,7 +87,7 @@ describe('Combobox', () => { expect(screen.getByText('Option 1')).toHaveClass('block cursor-pointer p-8 hover:s-bg-hover w-react-combobox-option'); }); - it('closes the options list on escape key press', async () => { + it('closes the options list on escape key press', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -95,7 +95,7 @@ describe('Combobox', () => { expect(screen.queryByRole('listbox')).toBeNull(); }); - it('closes options list on blur', async () => { + it('closes options list on blur', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -103,7 +103,7 @@ describe('Combobox', () => { expect(screen.queryByRole('listbox')).toBeNull(); }); - it('filters options based on input value when static filtering is disabled', async () => { + it('filters options based on input value when static filtering is disabled', () => { render(); const input = screen.getByRole('combobox'); fireEvent.change(input, { target: { value: 'Option 1' } }); @@ -111,7 +111,7 @@ describe('Combobox', () => { expect(screen.queryByText('Option 2')).toBeInTheDocument(); }); - it('navigates options with keyboard and selects an option', async () => { + it('navigates options with keyboard and selects an option', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -121,7 +121,7 @@ describe('Combobox', () => { expect(defaultProps.onSelect).toHaveBeenCalledWith('Option 1'); }); - it('navigates options with keyboard and closes list on Escape', async () => { + it('navigates options with keyboard and closes list on Escape', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -130,7 +130,7 @@ describe('Combobox', () => { expect(screen.queryByRole('listbox')).toBeNull(); }); - it('handles special keys without breaking', async () => { + it('handles special keys without breaking', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -141,7 +141,7 @@ describe('Combobox', () => { expect(screen.getByRole('listbox')).toBeInTheDocument(); }); - it('handles onChange and onBlur correctly', async () => { + it('handles onChange and onBlur correctly', () => { const onChange = vi.fn(); const onBlur = vi.fn(); render(); @@ -178,7 +178,7 @@ describe('Combobox', () => { await waitFor(() => expect(defaultProps.onSelect).not.toHaveBeenCalled()); }); - it('dismisses the popover on Escape key press', async () => { + it('dismisses the popover on Escape key press', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); @@ -186,8 +186,8 @@ describe('Combobox', () => { expect(screen.queryByRole('listbox')).toBeNull(); }); - it('sets empty options on select when disableStaticFiltering is true', async () => { - render(); + it('sets empty options on select when disableStaticFiltering is true', () => { + render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); fireEvent.keyDown(input, { key: 'ArrowDown' }); @@ -196,12 +196,13 @@ describe('Combobox', () => { expect(screen.queryByText('Option 1')).toBeNull(); }); - it('renders TextField with Affix component correctly', async () => { + it('renders TextField with Affix component correctly', () => { const ComboboxWithAffixWrapper = (props: Partial) => { + // eslint-disable-next-line const [_, setValue] = useState(props.value || ''); - + return ( - + setValue('')} /> ); @@ -215,17 +216,17 @@ describe('Combobox', () => { expect(screen.getByRole('combobox')).toHaveValue(''); }); - it('matches text segments correctly', async () => { + it('matches text segments correctly', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); fireEvent.keyDown(input, { key: 'ArrowDown' }); expect(screen.getByRole('listbox')).toBeInTheDocument(); fireEvent.change(input, { target: { value: 'Option 1' } }); - expect(screen.getByRole('listbox')).toHaveClass('w-react-combobox-match') + expect(screen.getByRole('listbox')).toHaveClass('w-react-combobox-match'); }); - it('handles PageDown key correctly', async () => { + it('handles PageDown key correctly', () => { render(); const input = screen.getByRole('combobox'); fireEvent.focus(input); From 09f7a9bb12db834d19418ddc35a440f295f6c10f Mon Sep 17 00:00:00 2001 From: felicia-haggqvist Date: Thu, 18 Jul 2024 15:38:16 +0200 Subject: [PATCH 6/9] fix(combobox): move option classes to a function and rename classes --- packages/combobox/src/component.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/combobox/src/component.tsx b/packages/combobox/src/component.tsx index efd19cd2..115167fb 100644 --- a/packages/combobox/src/component.tsx +++ b/packages/combobox/src/component.tsx @@ -72,6 +72,12 @@ export const Combobox = forwardRef(({ id: pid, const navigationValueOrInputValue = navigationOption?.value || value; + const optionClasses = (option) => classNames( + ccCombobox.option, + OPTION_CLASS_NAME, + navigationOption?.id === option?.id ? ccCombobox.optionSelected : ccCombobox.optionUnselected + ) + // Set and filter available options based on user input useEffect(() => { setCurrentOptions( @@ -239,7 +245,7 @@ export const Combobox = forwardRef(({ id: pid,