From 6a39e36168009c0f9c99db98ae1d360e2c2ceb33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Myl=C3=A8ne?= <187286904+mleroy-pass@users.noreply.github.com> Date: Wed, 15 Jan 2025 17:13:15 +0100 Subject: [PATCH] (PC-33528)[PRO] feat: apply pr review --- .../ui-kit/MultiSelect/MultiSelectPanel.tsx | 2 - .../ui-kit/MultiSelect/MultiSelectTrigger.tsx | 1 + .../__specs__/MultiSelect.spec.tsx | 157 +++----- .../__specs__/MultiSelectPanel.spec.tsx | 305 +++++--------- .../__specs__/MultiSelectTrigger.spec.tsx | 377 +++--------------- 5 files changed, 231 insertions(+), 611 deletions(-) diff --git a/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx b/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx index 86625319fda..1a3843b308a 100644 --- a/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx +++ b/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx @@ -85,7 +85,6 @@ export const MultiSelectPanel = ({ labelClassName={styles['label']} inputClassName={styles['checkbox']} onChange={onSelectAll} - tabIndex={0} />
@@ -98,7 +97,6 @@ export const MultiSelectPanel = ({ label={option.label} checked={option.checked} onChange={() => onOptionSelect(option)} - tabIndex={0} /> ))} diff --git a/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx b/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx index b7aadcd35dc..f5085edc833 100644 --- a/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx +++ b/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx @@ -35,6 +35,7 @@ export const MultiSelectTrigger = ({ })} onClick={toggleDropdown} aria-haspopup="listbox" + aria-label={label} aria-expanded={isOpen} aria-controls={id} disabled={disabled} diff --git a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx index adcf1e948ae..c19011a284d 100644 --- a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx +++ b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx @@ -2,97 +2,90 @@ import { render, screen, waitFor } from '@testing-library/react' import { userEvent } from '@testing-library/user-event' import { axe } from 'vitest-axe' -import { MultiSelect, Option } from '../MultiSelect' +import { MultiSelect } from '../MultiSelect' describe('', () => { - const options: Option[] = [ + const options = [ { id: '1', label: 'Option 1' }, { id: '2', label: 'Option 2' }, { id: '3', label: 'Option 3' }, ] - const defaultOptions: Option[] = [{ id: '1', label: 'Option 1' }] + const renderMultiSelect = ({ + hasSelectAllOptions = false, + }: { + hasSelectAllOptions?: boolean + } = {}) => { + const defaultOptions = [{ id: '1', label: 'Option 1' }] - it('should render correctly', async () => { - render( + return render( ) + } - expect(await screen.findByText('Select Options')).toBeInTheDocument() + it('should render correctly', () => { + renderMultiSelect() + + expect( + screen.getByRole('button', { name: 'Select Options' }) + ).toBeInTheDocument() }) it('should not have accessibility violations', async () => { - const { container } = render( - - ) + const { container } = renderMultiSelect() expect(await axe(container)).toHaveNoViolations() - const toggleButton = screen.getByText('Select Options') + const toggleButton = screen.getByRole('button', { name: 'Select Options' }) await userEvent.click(toggleButton) expect(await axe(container)).toHaveNoViolations() }) it('renders the MultiSelect component with the correct initial selected options', () => { - render( - - ) + renderMultiSelect() + + const selectedTag = screen.getByRole('button', { + name: 'Supprimer Option 1', + }) - expect(screen.getByText('Option 1')).toBeInTheDocument() + expect(selectedTag).toBeInTheDocument() }) it('toggles the dropdown when the trigger is clicked', async () => { - render( - - ) - const toggleButton = screen.getByText('Select Options') + renderMultiSelect({ hasSelectAllOptions: true }) + + const toggleButton = screen.getByRole('button', { name: 'Select Options' }) await userEvent.click(toggleButton) - expect(screen.getByText(/Tout sélectionner/i)).toBeInTheDocument() + + const selectAllCheckbox = screen.getByRole('checkbox', { + name: 'Tout sélectionner', + }) + + expect(selectAllCheckbox).toBeInTheDocument() await userEvent.click(toggleButton) - expect(screen.queryByText(/Tout sélectionner/i)).not.toBeInTheDocument() + + expect(selectAllCheckbox).not.toBeInTheDocument() }) it('selects all options when "Select All" is clicked', async () => { - render( - - ) + renderMultiSelect({ hasSelectAllOptions: true }) - const toggleButton = screen.getByText('Select Options') + const toggleButton = screen.getByRole('button', { name: 'Select Options' }) await userEvent.click(toggleButton) - const selectAllCheckbox = screen.getByLabelText(/Tout sélectionner/i) + const selectAllCheckbox = screen.getByRole('checkbox', { + name: 'Tout sélectionner', + }) + await userEvent.click(selectAllCheckbox) options.forEach((option) => { @@ -101,73 +94,55 @@ describe('', () => { }) it('removes an option from the selected items when clicked in SelectedValuesTags', async () => { - render( - - ) + renderMultiSelect() + + const selectedTag = screen.getByRole('button', { + name: 'Supprimer Option 1', + }) - const selectedTag = screen.getByText('Option 1') await userEvent.click(selectedTag) - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() + + expect(selectedTag).not.toBeInTheDocument() }) it('closes the dropdown when clicked outside or when Escape key is pressed', async () => { - render( - - ) + renderMultiSelect() - const toggleButton = screen.getByText('Select Options') + const toggleButton = screen.getByRole('button', { name: 'Select Options' }) + toggleButton.focus() await userEvent.click(toggleButton) - expect(screen.queryByText('Option 1')).toBeInTheDocument() + + const optionCheckbox2 = screen.getByRole('checkbox', { name: 'Option 1' }) + + expect(optionCheckbox2).toBeInTheDocument() await userEvent.click(document.body) - await waitFor(() => - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() - ) + + await waitFor(() => expect(optionCheckbox2).not.toBeInTheDocument()) await userEvent.click(toggleButton) await userEvent.keyboard('[Escape]') - await waitFor(() => - expect(screen.queryByText('Option 1')).not.toBeInTheDocument() - ) + await waitFor(() => expect(optionCheckbox2).not.toBeInTheDocument()) }) it('should toggle dropdown with keyboard accessibility', async () => { - render( - - ) + renderMultiSelect({ hasSelectAllOptions: true }) const toggleButton = screen.getByRole('button', { name: 'Select Options' }) toggleButton.focus() await userEvent.keyboard('[Enter]') - await waitFor(() => - expect(screen.getByText(/Tout sélectionner/i)).toBeInTheDocument() - ) + const selectAllCheckbox = screen.getByRole('checkbox', { + name: 'Tout sélectionner', + }) + + await waitFor(() => expect(selectAllCheckbox).toBeInTheDocument()) await userEvent.keyboard('[Escape]') - await waitFor(() => - expect(screen.queryByText(/Tout sélectionner/i)).not.toBeInTheDocument() - ) + await waitFor(() => expect(selectAllCheckbox).not.toBeInTheDocument()) }) }) diff --git a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectPanel.spec.tsx b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectPanel.spec.tsx index cf2b12269c5..23c9ac410c0 100644 --- a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectPanel.spec.tsx +++ b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectPanel.spec.tsx @@ -15,18 +15,37 @@ describe('', () => { const onOptionSelect = vi.fn() const onSelectAll = vi.fn() - it('renders options with checkboxes', () => { - render( + const renderMultiSelectPanel = ({ + hasSelectAllOptions = false, + isAllChecked = false, + hasSearch = false, + searchLabel = '', + searchExample = '', + }: { + hasSelectAllOptions?: boolean + isAllChecked?: boolean + hasSearch?: boolean + searchLabel?: string + searchExample?: string + } = {}) => { + return render( ) + } + + it('renders options with checkboxes', () => { + renderMultiSelectPanel() expect(screen.getByLabelText('Option 1')).toBeInTheDocument() expect(screen.getByLabelText('Option 2')).toBeInTheDocument() @@ -34,37 +53,21 @@ describe('', () => { }) it('renders the search input if hasSearch is true', () => { - render( - - ) + renderMultiSelectPanel({ + hasSearch: true, + searchExample: 'Exemple: Nantes', + searchLabel: 'Search label', + }) expect(screen.getByText(/Exemple: Nantes/i)).toBeInTheDocument() }) - test('updates search value on input change', async () => { - render( - - ) + it('updates search value on input change', async () => { + renderMultiSelectPanel({ + hasSearch: true, + searchExample: 'Exemple: Nantes', + searchLabel: 'Search label', + }) const input = screen.getByRole('searchbox') @@ -73,54 +76,28 @@ describe('', () => { expect(input).toHaveValue('apple') }) - test('displays the search example text', () => { - render( - - ) + it('displays the search example text', () => { + renderMultiSelectPanel({ + hasSearch: true, + searchExample: 'Exemple: Nantes', + searchLabel: 'Search label', + }) expect(screen.getByText('Exemple: Nantes')).toBeInTheDocument() }) it('not renders the search input if hasSearch is false', () => { - render( - - ) + renderMultiSelectPanel() expect(screen.queryByText(/Exemple: Nantes/i)).not.toBeInTheDocument() }) it('should filter options based on the search input', async () => { - render( - - ) + renderMultiSelectPanel({ + hasSearch: true, + searchExample: 'Exemple: Nantes', + searchLabel: 'Search label', + }) expect(screen.getByText('Option 1')).toBeInTheDocument() expect(screen.getByText('Option 2')).toBeInTheDocument() @@ -146,19 +123,11 @@ describe('', () => { }) it('should show "No results found" when no options match the search', async () => { - render( - - ) + renderMultiSelectPanel({ + hasSearch: true, + searchExample: 'Exemple: Nantes', + searchLabel: 'Search label', + }) const searchInput = screen.getByRole('searchbox') @@ -172,130 +141,78 @@ describe('', () => { }) it('should not have accessibility violations', async () => { - const { container } = render( - - ) + const { container } = renderMultiSelectPanel() expect(await axe(container)).toHaveNoViolations() }) it('selects and deselects individual options', async () => { - render( - - ) + renderMultiSelectPanel() + + const option2Checkbox = screen.getByRole('checkbox', { name: 'Option 2' }) - const option2Checkbox = screen.getByLabelText(/Option 2/i) await userEvent.click(option2Checkbox) + expect(onOptionSelect).toHaveBeenCalledWith(options[1]) await userEvent.click(option2Checkbox) + expect(onOptionSelect).toHaveBeenCalledWith(options[1]) }) - test('renders "Select All" checkbox when hasSelectAllOptions is true', () => { - render( - - ) + it('renders "Select All" checkbox when hasSelectAllOptions is true', () => { + renderMultiSelectPanel({ + hasSelectAllOptions: true, + }) - expect(screen.getByLabelText('Tout sélectionner')).toBeInTheDocument() + expect( + screen.getByRole('checkbox', { + name: 'Tout sélectionner', + }) + ).toBeInTheDocument() }) - test('triggers onSelectAll when "Select All" checkbox is clicked', async () => { - render( - - ) + it('triggers onSelectAll when "Select All" checkbox is clicked', async () => { + renderMultiSelectPanel({ + hasSelectAllOptions: true, + }) + + const selectAllCheckbox = screen.getByRole('checkbox', { + name: 'Tout sélectionner', + }) - const selectAllCheckbox = screen.getByLabelText('Tout sélectionner') await userEvent.click(selectAllCheckbox) expect(onSelectAll).toHaveBeenCalledTimes(1) }) - test('reflects isAllChecked state in "Select All" checkbox', () => { - render( - - ) + it('reflects isAllChecked state in "Select All" checkbox', () => { + renderMultiSelectPanel({ + hasSelectAllOptions: true, + isAllChecked: true, + }) + + const selectAllCheckbox = screen.getByRole('checkbox', { + name: 'Tout sélectionner', + }) - const selectAllCheckbox = screen.getByLabelText('Tout sélectionner') expect(selectAllCheckbox).toBeChecked() }) - test('does not render "Select All" checkbox when hasSelectAllOptions is false', () => { - render( - - ) + it('does not render "Select All" checkbox when hasSelectAllOptions is false', () => { + renderMultiSelectPanel() + + const selectAllCheckbox = screen.queryByText('Tout sélectionner') - const selectAllCheckbox = screen.queryByLabelText('Tout sélectionner') expect(selectAllCheckbox).not.toBeInTheDocument() }) - test('calls onOptionSelect when Enter key is pressed on an option', async () => { - render( - - ) + it('calls onOptionSelect when Enter key is pressed on an option', async () => { + renderMultiSelectPanel({ + hasSelectAllOptions: true, + }) - const optionCheckbox = screen.getByLabelText('Option 1') + const optionCheckbox = screen.getByRole('checkbox', { name: 'Option 1' }) optionCheckbox.focus() @@ -308,21 +225,12 @@ describe('', () => { }) }) - test('calls onOptionSelect when Space key is pressed on an option', async () => { - render( - - ) + it('calls onOptionSelect when Space key is pressed on an option', async () => { + renderMultiSelectPanel({ + hasSelectAllOptions: true, + }) - const optionCheckbox = screen.getByLabelText('Option 2') + const optionCheckbox = screen.getByRole('checkbox', { name: 'Option 2' }) optionCheckbox.focus() @@ -335,21 +243,12 @@ describe('', () => { }) }) - test('does not call onOptionSelect when other keys are pressed', async () => { - render( - - ) + it('does not call onOptionSelect when other keys are pressed', async () => { + renderMultiSelectPanel({ + hasSelectAllOptions: true, + }) - const optionCheckbox = screen.getByLabelText('Option 3') + const optionCheckbox = screen.getByRole('checkbox', { name: 'Option 3' }) optionCheckbox.focus() diff --git a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectTrigger.spec.tsx b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectTrigger.spec.tsx index cf2b12269c5..bf26948b35b 100644 --- a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectTrigger.spec.tsx +++ b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectTrigger.spec.tsx @@ -1,360 +1,107 @@ -import { render, screen, waitFor } from '@testing-library/react' +import { render, screen } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { axe } from 'vitest-axe' -import { Option } from '../MultiSelect' -import { MultiSelectPanel } from '../MultiSelectPanel' +import { MultiSelectTrigger } from '../MultiSelectTrigger' -describe('', () => { - const options: (Option & { checked: boolean })[] = [ - { id: '1', label: 'Option 1', checked: false }, - { id: '2', label: 'Option 2', checked: false }, - { id: '3', label: 'Option 3', checked: false }, - ] +describe('', () => { + const mockToggleDropdown = vi.fn() - const onOptionSelect = vi.fn() - const onSelectAll = vi.fn() - - it('renders options with checkboxes', () => { - render( - { + return render( + ) + } - expect(screen.getByLabelText('Option 1')).toBeInTheDocument() - expect(screen.getByLabelText('Option 2')).toBeInTheDocument() - expect(screen.getByLabelText('Option 3')).toBeInTheDocument() - }) - - it('renders the search input if hasSearch is true', () => { - render( - - ) + it('should render correctly', () => { + renderMultiSelectPanel({ selectedCount: 2 }) - expect(screen.getByText(/Exemple: Nantes/i)).toBeInTheDocument() + expect(screen.getByText('Options Label')).toBeInTheDocument() + expect(screen.getByText('2')).toBeInTheDocument() }) - test('updates search value on input change', async () => { - render( - - ) - - const input = screen.getByRole('searchbox') - - await userEvent.type(input, 'apple') + it('should have no accessibility violations', async () => { + const { container } = renderMultiSelectPanel() - expect(input).toHaveValue('apple') + const results = await axe(container) + expect(results).toHaveNoViolations() }) - test('displays the search example text', () => { - render( - - ) - - expect(screen.getByText('Exemple: Nantes')).toBeInTheDocument() - }) + it('should disable the button when disabled prop is passed', () => { + renderMultiSelectPanel({ disabled: true }) - it('not renders the search input if hasSearch is false', () => { - render( - - ) + const button = screen.getByRole('button') - expect(screen.queryByText(/Exemple: Nantes/i)).not.toBeInTheDocument() + expect(button).toBeDisabled() }) - it('should filter options based on the search input', async () => { - render( - - ) - - expect(screen.getByText('Option 1')).toBeInTheDocument() - expect(screen.getByText('Option 2')).toBeInTheDocument() - expect(screen.getByText('Option 3')).toBeInTheDocument() - - const searchInput = screen.getByRole('searchbox') - - await userEvent.type(searchInput, 'Option 1') - - await waitFor(() => { - expect(screen.queryByText('Option 2')).not.toBeInTheDocument() - expect(screen.queryByText('Option 3')).not.toBeInTheDocument() - expect(screen.getByText('Option 1')).toBeInTheDocument() + it('should display the chevron open icon if panel is opened', () => { + const { container } = renderMultiSelectPanel({ + disabled: true, + isOpen: true, }) - await userEvent.clear(searchInput) - - await waitFor(() => { - expect(screen.getByText('Option 1')).toBeInTheDocument() - expect(screen.getByText('Option 2')).toBeInTheDocument() - expect(screen.getByText('Option 3')).toBeInTheDocument() - }) - }) - - it('should show "No results found" when no options match the search', async () => { - render( - - ) - - const searchInput = screen.getByRole('searchbox') - - await userEvent.type(searchInput, 'Non-matching option') - - await waitFor(() => - expect( - screen.getByText('Aucun résultat trouvé pour votre recherche.') - ).toBeInTheDocument() - ) - }) - - it('should not have accessibility violations', async () => { - const { container } = render( - - ) - - expect(await axe(container)).toHaveNoViolations() - }) + let chevronIcon = container.querySelector('svg') - it('selects and deselects individual options', async () => { - render( - - ) - - const option2Checkbox = screen.getByLabelText(/Option 2/i) - await userEvent.click(option2Checkbox) - expect(onOptionSelect).toHaveBeenCalledWith(options[1]) - - await userEvent.click(option2Checkbox) - expect(onOptionSelect).toHaveBeenCalledWith(options[1]) + expect(chevronIcon).toHaveClass('chevron chevronOpen') }) - test('renders "Select All" checkbox when hasSelectAllOptions is true', () => { - render( - - ) + it('should not display the chevron icon open if panel is closed', () => { + const { container } = renderMultiSelectPanel() - expect(screen.getByLabelText('Tout sélectionner')).toBeInTheDocument() - }) - - test('triggers onSelectAll when "Select All" checkbox is clicked', async () => { - render( - - ) + let chevronIcon = container.querySelector('svg') - const selectAllCheckbox = screen.getByLabelText('Tout sélectionner') - await userEvent.click(selectAllCheckbox) + chevronIcon = container.querySelector('svg') - expect(onSelectAll).toHaveBeenCalledTimes(1) + expect(chevronIcon).not.toHaveClass('chevron chevronOpen') }) - test('reflects isAllChecked state in "Select All" checkbox', () => { - render( - - ) + it('should render badge with correct count when options are selected', () => { + renderMultiSelectPanel({ selectedCount: 3 }) - const selectAllCheckbox = screen.getByLabelText('Tout sélectionner') - expect(selectAllCheckbox).toBeChecked() + expect(screen.getByText('3')).toBeInTheDocument() }) - test('does not render "Select All" checkbox when hasSelectAllOptions is false', () => { - render( - - ) - - const selectAllCheckbox = screen.queryByLabelText('Tout sélectionner') - expect(selectAllCheckbox).not.toBeInTheDocument() - }) - - test('calls onOptionSelect when Enter key is pressed on an option', async () => { - render( - - ) - - const optionCheckbox = screen.getByLabelText('Option 1') - - optionCheckbox.focus() - - await userEvent.keyboard('[Space]') + it('should not render badge when no options are selected', () => { + renderMultiSelectPanel({ selectedCount: 0 }) - expect(onOptionSelect).toHaveBeenCalledWith({ - id: '1', - label: 'Option 1', - checked: false, - }) + const badge = screen.queryByText('0') + expect(badge).toBeNull() }) - test('calls onOptionSelect when Space key is pressed on an option', async () => { - render( - - ) - - const optionCheckbox = screen.getByLabelText('Option 2') + it('should update badge count when selecting and deselecting options', () => { + const { rerender } = renderMultiSelectPanel({ selectedCount: 2 }) - optionCheckbox.focus() + expect(screen.getByText('2')).toBeInTheDocument() - await userEvent.keyboard('[Space]') + rerender(5) - expect(onOptionSelect).toHaveBeenCalledWith({ - id: '2', - label: 'Option 2', - checked: false, - }) + expect(screen.getByText('5')).toBeInTheDocument() }) - test('does not call onOptionSelect when other keys are pressed', async () => { - render( - - ) - - const optionCheckbox = screen.getByLabelText('Option 3') + it('should call toggleDropdown when the button is clicked', async () => { + renderMultiSelectPanel() - optionCheckbox.focus() + const button = screen.getByRole('button') - await userEvent.keyboard('[ArrowDown]') + await userEvent.click(button) - expect(onOptionSelect).not.toHaveBeenCalled() + expect(mockToggleDropdown).toHaveBeenCalledTimes(1) }) })