From 51e3823eb251d9049c68f79bea2eee9996c3f1df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Myl=C3=A8ne?= <187286904+mleroy-pass@users.noreply.github.com> Date: Fri, 10 Jan 2025 10:18:53 +0100 Subject: [PATCH] (PC-33528)[PRO] feat: add keyboard nav --- .../MultiSelect/MultiSelect.module.scss | 2 +- pro/src/ui-kit/MultiSelect/MultiSelect.tsx | 91 +++++++++---------- .../ui-kit/MultiSelect/MultiSelectPanel.tsx | 44 +++++---- .../ui-kit/MultiSelect/MultiSelectTrigger.tsx | 3 - pro/src/ui-kit/MultiSelect/TODO.md | 2 +- .../__specs__/MultiSelect.spec.tsx | 67 +++++++++++--- .../__specs__/MultiSelectPanel.spec.tsx | 29 +++++- 7 files changed, 154 insertions(+), 84 deletions(-) diff --git a/pro/src/ui-kit/MultiSelect/MultiSelect.module.scss b/pro/src/ui-kit/MultiSelect/MultiSelect.module.scss index 3c1f4eb66a4..4d1e7866930 100644 --- a/pro/src/ui-kit/MultiSelect/MultiSelect.module.scss +++ b/pro/src/ui-kit/MultiSelect/MultiSelect.module.scss @@ -62,13 +62,13 @@ align-items: center; justify-content: center; gap: rem.torem(8px); - max-width: 500px; } .trigger-label { overflow: hidden; text-overflow: ellipsis; white-space: nowrap; + max-width: 300px; } .badge { diff --git a/pro/src/ui-kit/MultiSelect/MultiSelect.tsx b/pro/src/ui-kit/MultiSelect/MultiSelect.tsx index 79e26ff3d47..712e72ba456 100644 --- a/pro/src/ui-kit/MultiSelect/MultiSelect.tsx +++ b/pro/src/ui-kit/MultiSelect/MultiSelect.tsx @@ -16,16 +16,24 @@ type MultiSelectProps = { defaultOptions?: Option[] label: string legend: string - hasSearch?: boolean - searchExample?: string - searchLabel?: string hasSelectAllOptions?: boolean disabled?: boolean -} +} & ( + | { + hasSearch: false | never + searchExample?: undefined + searchLabel?: undefined + } + | { + hasSearch: true + searchExample: string + searchLabel: string + } +) export const MultiSelect = ({ options, - defaultOptions, + defaultOptions = [], hasSearch, searchExample, searchLabel, @@ -35,78 +43,69 @@ export const MultiSelect = ({ disabled, }: MultiSelectProps): JSX.Element => { const [isOpen, setIsOpen] = useState(false) - const containerRef = useRef(null) - const [selectedItems, setSelectedItems] = useState( - defaultOptions ?? [] - ) + const [selectedItems, setSelectedItems] = useState(defaultOptions) const [isSelectAllChecked, setIsSelectAllChecked] = useState(false) - const handleSelectOrRemoveItem = (item: Option) => { - const itemsToBeSelected = selectedItems.some( - (prevItem) => prevItem.id === item.id - ) - ? selectedItems.filter((prevItem) => prevItem.id !== item.id) + const containerRef = useRef(null) + + const toggleDropdown = () => setIsOpen((prev) => !prev) + + const handleSelectItem = (item: Option) => { + const updatedSelectedItems = selectedItems.some((i) => i.id === item.id) + ? selectedItems.filter((i) => i.id !== item.id) : [...selectedItems, item] - setIsSelectAllChecked(itemsToBeSelected.length === options.length) - setSelectedItems(itemsToBeSelected) + setSelectedItems(updatedSelectedItems) + setIsSelectAllChecked(updatedSelectedItems.length === options.length) } const handleSelectAll = () => { + const updatedItems = isSelectAllChecked ? [] : options + setSelectedItems(updatedItems) setIsSelectAllChecked(!isSelectAllChecked) - if (isSelectAllChecked) { - setSelectedItems([]) - } else { - setSelectedItems(options) - } } const handleRemoveTag = (itemId: string) => { - setSelectedItems((prev) => prev.filter((item) => item.id !== itemId)) + const updatedItems = selectedItems.filter((item) => item.id !== itemId) + setSelectedItems(updatedItems) + setIsSelectAllChecked(updatedItems.length === options.length) } - const toggleDropdown = () => setIsOpen(!isOpen) - - const handleKeyDown = (event: React.KeyboardEvent) => { - event.preventDefault() - if (event.key === 'Enter' || event.key === ' ') { - toggleDropdown() + const handleClickOutside = (event: MouseEvent) => { + if ( + containerRef.current && + !containerRef.current.contains(event.target as Node) + ) { + setIsOpen(false) } } - useEffect(() => { - const handleWindowClick = (event: MouseEvent) => { - if ( - containerRef.current && - !containerRef.current.contains(event.target as Node) - ) { - setIsOpen(false) - } + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Escape') { + setIsOpen(false) } - - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape') { - setIsOpen(false) - } + if (event.key === 'Enter' || event.key === ' ') { + setIsOpen(true) } + } - window.addEventListener('click', handleWindowClick) + useEffect(() => { + window.addEventListener('click', handleClickOutside) window.addEventListener('keydown', handleKeyDown) return () => { - window.removeEventListener('click', handleWindowClick) + window.removeEventListener('click', handleClickOutside) window.removeEventListener('keydown', handleKeyDown) } }, []) return ( -
+
@@ -118,7 +117,7 @@ export const MultiSelect = ({ ...option, checked: selectedItems.some((item) => item.id === option.id), }))} - onOptionSelect={handleSelectOrRemoveItem} + onOptionSelect={handleSelectItem} onSelectAll={handleSelectAll} isAllChecked={isSelectAllChecked} hasSearch={hasSearch} diff --git a/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx b/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx index 7598f189c02..f3360798fab 100644 --- a/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx +++ b/pro/src/ui-kit/MultiSelect/MultiSelectPanel.tsx @@ -11,14 +11,22 @@ type MultiSelectPanelProps = { className?: string label: string options: (Option & { checked: boolean })[] - onOptionSelect: (option: Option) => void - onSelectAll: () => void - hasSearch?: boolean - searchExample?: string - searchLabel?: string hasSelectAllOptions?: boolean isAllChecked: boolean -} + onOptionSelect: (option: Option) => void + onSelectAll: () => void +} & ( + | { + hasSearch: false | never + searchExample?: undefined + searchLabel?: undefined + } + | { + hasSearch: true + searchExample: string + searchLabel: string + } +) export const MultiSelectPanel = ({ options, @@ -31,7 +39,8 @@ export const MultiSelectPanel = ({ isAllChecked, }: MultiSelectPanelProps): JSX.Element => { const [searchValue, setSearchValue] = useState('') - const searchedValues = useMemo( + + const filteredOptions = useMemo( () => options.filter((option) => option.label.toLowerCase().includes(searchValue.toLowerCase()) @@ -39,12 +48,10 @@ export const MultiSelectPanel = ({ [options, searchValue] ) - const onToggleAllOption = () => { - onSelectAll() - } - - const onToggleOption = (option: Option) => { - onOptionSelect(option) + const handleKeyDown = (e: React.KeyboardEvent, option: Option) => { + if (e.key === 'Enter' || e.key === ' ') { + onOptionSelect(option) + } } return ( @@ -65,7 +72,7 @@ export const MultiSelectPanel = ({ )} - {searchedValues.length > 0 ? ( + {filteredOptions.length > 0 ? (
    {hasSelectAllOptions && (
  • @@ -74,19 +81,22 @@ export const MultiSelectPanel = ({ checked={isAllChecked} labelClassName={styles['label']} inputClassName={styles['checkbox']} - onChange={() => onToggleAllOption()} + onChange={onSelectAll} + tabIndex={0} />
    • )} - {searchedValues.map((option) => ( + {filteredOptions.map((option) => (
  • onToggleOption(option)} + onChange={() => onOptionSelect(option)} + onKeyDown={(e) => handleKeyDown(e, option)} + tabIndex={0} />
    • ))} diff --git a/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx b/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx index 2b17e7275b8..2f080e84c07 100644 --- a/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx +++ b/pro/src/ui-kit/MultiSelect/MultiSelectTrigger.tsx @@ -11,7 +11,6 @@ type MultiSelectTriggerProps = { isOpen: boolean selectedCount: number toggleDropdown: () => void - handleKeyDown: (event: React.KeyboardEvent) => void legend: string label: string disabled?: boolean @@ -21,7 +20,6 @@ export const MultiSelectTrigger = ({ isOpen, selectedCount, toggleDropdown, - handleKeyDown, legend, label, disabled, @@ -38,7 +36,6 @@ export const MultiSelectTrigger = ({ [styles['trigger-selected']]: selectedCount > 0, })} onClick={toggleDropdown} - onKeyDown={handleKeyDown} aria-haspopup="listbox" aria-expanded={isOpen} aria-labelledby={legendId} diff --git a/pro/src/ui-kit/MultiSelect/TODO.md b/pro/src/ui-kit/MultiSelect/TODO.md index b8ef1519650..03a83fe749f 100644 --- a/pro/src/ui-kit/MultiSelect/TODO.md +++ b/pro/src/ui-kit/MultiSelect/TODO.md @@ -1,6 +1,6 @@ Fonctionnel -- [] gérer la navigation clavier +- [x] gérer la navigation clavier - [x] Corriger le all selected - [] jsdoc - [x] ne pas afficher plus de 3 lignes de tags (refacto des tags dans un autre ticket PC-33762) diff --git a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx index ad8159e816a..0735f82e702 100644 --- a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx +++ b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelect.spec.tsx @@ -20,6 +20,7 @@ describe('', () => { label="Select Options" legend="Legend" defaultOptions={defaultOptions} + hasSearch={false} /> ) @@ -33,6 +34,7 @@ describe('', () => { label="Select Options" legend="Legend" defaultOptions={defaultOptions} + hasSearch={false} /> ) @@ -46,9 +48,10 @@ describe('', () => { label="Select Options" legend="Legend" defaultOptions={defaultOptions} + hasSearch={false} /> ) - // Check that the initial selected options are rendered in the SelectedValuesTags + expect(screen.getByText('Option 1')).toBeInTheDocument() }) @@ -59,13 +62,14 @@ describe('', () => { label="Select Options" legend="Legend" hasSelectAllOptions={true} + hasSearch={false} /> ) const toggleButton = screen.getByText('Select Options') - await userEvent.click(toggleButton) // Open the dropdown + await userEvent.click(toggleButton) expect(screen.getByText(/Tout sélectionner/i)).toBeInTheDocument() - await userEvent.click(toggleButton) // Close the dropdown + await userEvent.click(toggleButton) expect(screen.queryByText(/Tout sélectionner/i)).not.toBeInTheDocument() }) @@ -76,16 +80,16 @@ describe('', () => { label="Select Options" legend="Legend" hasSelectAllOptions={true} + hasSearch={false} /> ) const toggleButton = screen.getByText('Select Options') - await userEvent.click(toggleButton) // Open the dropdown + await userEvent.click(toggleButton) const selectAllCheckbox = screen.getByLabelText(/Tout sélectionner/i) - fireEvent.click(selectAllCheckbox) // Select all options + fireEvent.click(selectAllCheckbox) - // Verify that all options are selected options.forEach((option) => { expect(screen.getByLabelText(option.label)).toBeChecked() }) @@ -98,35 +102,74 @@ describe('', () => { label="Select Options" legend="Legend" defaultOptions={defaultOptions} + hasSearch={false} /> ) // Initially, Option 1 is selected const selectedTag = screen.getByText('Option 1') - await userEvent.click(selectedTag) // Remove Option 1 + await userEvent.click(selectedTag) expect(screen.queryByText('Option 1')).not.toBeInTheDocument() }) it('closes the dropdown when clicked outside or when Escape key is pressed', async () => { render( - + ) const toggleButton = screen.getByText('Select Options') - fireEvent.click(toggleButton) // Open the dropdown + fireEvent.click(toggleButton) expect(screen.queryByText('Option 1')).toBeInTheDocument() - // Simulate a click outside the dropdown fireEvent.click(document.body) await waitFor(() => expect(screen.queryByText('Option 1')).not.toBeInTheDocument() ) - fireEvent.click(toggleButton) // Open the dropdown again - fireEvent.keyDown(toggleButton, { key: 'Escape' }) // Close with Escape key + fireEvent.click(toggleButton) + fireEvent.keyDown(toggleButton, { key: 'Escape' }) await waitFor(() => expect(screen.queryByText('Option 1')).not.toBeInTheDocument() ) }) + + it('should toggle dropdown with keyboard accessibility', async () => { + render( + + ) + + const toggleButton = screen.getByText('Select Options') + + fireEvent.keyDown(toggleButton, { key: 'Enter' }) + await waitFor(() => + expect(screen.getByText(/Tout sélectionner/i)).toBeInTheDocument() + ) + + fireEvent.keyDown(toggleButton, { key: 'Escape' }) + await waitFor(() => + expect(screen.queryByText(/Tout sélectionner/i)).not.toBeInTheDocument() + ) + + fireEvent.keyDown(toggleButton, { key: ' ' }) + await waitFor(() => + expect(screen.getByText(/Tout sélectionner/i)).toBeInTheDocument() + ) + + fireEvent.keyDown(toggleButton, { key: 'Escape' }) + await waitFor(() => + expect(screen.queryByText(/Tout sélectionner/i)).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 4af3901ae6c..7be2d49905a 100644 --- a/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectPanel.spec.tsx +++ b/pro/src/ui-kit/MultiSelect/__specs__/MultiSelectPanel.spec.tsx @@ -19,9 +19,14 @@ describe('', () => { ) @@ -40,6 +45,11 @@ describe('', () => { }} hasSearch={true} searchExample="Exemple: Nantes" + searchLabel="Search label" + onSelectAll={function (): void { + throw new Error('Function not implemented.') + }} + isAllChecked={false} /> ) @@ -55,7 +65,10 @@ describe('', () => { throw new Error('Function not implemented.') }} hasSearch={false} - searchExample="Exemple: Nantes" + onSelectAll={function (): void { + throw new Error('Function not implemented.') + }} + isAllChecked={false} /> ) @@ -71,6 +84,10 @@ describe('', () => { throw new Error('Function not implemented.') }} hasSearch={false} + onSelectAll={function (): void { + throw new Error('Function not implemented.') + }} + isAllChecked={false} /> ) @@ -82,16 +99,20 @@ describe('', () => { ) - // Initially, only Option 1 is selected const option2Checkbox = screen.getByLabelText(/Option 2/i) - await userEvent.click(option2Checkbox) // Select Option 2 + await userEvent.click(option2Checkbox) expect(onOptionSelect).toHaveBeenCalledWith(options[1]) - await userEvent.click(option2Checkbox) // Deselect Option 2 + await userEvent.click(option2Checkbox) expect(onOptionSelect).toHaveBeenCalledWith(options[1]) }) })