From 5a12f281dc4ffcf93ae06db537c3a0a71e61ee27 Mon Sep 17 00:00:00 2001 From: German Saracca Date: Fri, 3 May 2024 10:21:36 -0300 Subject: [PATCH] fix: avoid selecting all options instead of filteres options --- .../select-multiple/SelectMultiple.tsx | 26 ++++-- .../select-multiple/SelectMultipleMenu.tsx | 45 ++++++---- .../select-multiple/selectMultipleReducer.ts | 40 ++++++--- .../select-multiple/SelectMultiple.spec.tsx | 87 ++++++++++++++++++- .../selectMultipleReducer.spec.tsx | 63 ++++++++++++-- 5 files changed, 214 insertions(+), 47 deletions(-) diff --git a/packages/design-system/src/lib/components/select-multiple/SelectMultiple.tsx b/packages/design-system/src/lib/components/select-multiple/SelectMultiple.tsx index 9e7f010ea..c40ba10ee 100644 --- a/packages/design-system/src/lib/components/select-multiple/SelectMultiple.tsx +++ b/packages/design-system/src/lib/components/select-multiple/SelectMultiple.tsx @@ -5,7 +5,8 @@ import { selectMultipleReducer, selectOption, removeOption, - toggleAllOptions, + selectAllOptions, + deselectAllOptions, searchOptions } from './selectMultipleReducer' import { SelectMultipleToggle } from './SelectMultipleToggle' @@ -38,12 +39,14 @@ export const SelectMultiple = forwardRef( }: SelectMultipleProps, ref: ForwardedRef ) => { - const [{ selectedOptions, filteredOptions }, dispatch] = useReducer(selectMultipleReducer, { - ...selectMultipleInitialState, - options: options, - filteredOptions: options, - selectedOptions: defaultValue || [] - }) + const [{ selectedOptions, filteredOptions, searchValue }, dispatch] = useReducer( + selectMultipleReducer, + { + ...selectMultipleInitialState, + options: options, + selectedOptions: defaultValue || [] + } + ) const isFirstRender = useIsFirstRender() const menuId = useId() @@ -70,7 +73,13 @@ export const SelectMultiple = forwardRef( const handleRemoveSelectedOption = (option: string): void => dispatch(removeOption(option)) - const handleToggleAllOptions = (): void => dispatch(toggleAllOptions()) + const handleToggleAllOptions = (e: React.ChangeEvent): void => { + if (e.target.checked) { + dispatch(selectAllOptions()) + } else { + dispatch(deselectAllOptions()) + } + } return ( @@ -87,6 +96,7 @@ export const SelectMultiple = forwardRef( options={options} selectedOptions={selectedOptions} filteredOptions={filteredOptions} + searchValue={searchValue} handleToggleAllOptions={handleToggleAllOptions} handleSearch={handleSearch} handleCheck={handleCheck} diff --git a/packages/design-system/src/lib/components/select-multiple/SelectMultipleMenu.tsx b/packages/design-system/src/lib/components/select-multiple/SelectMultipleMenu.tsx index 00c167a91..30912e7bd 100644 --- a/packages/design-system/src/lib/components/select-multiple/SelectMultipleMenu.tsx +++ b/packages/design-system/src/lib/components/select-multiple/SelectMultipleMenu.tsx @@ -6,7 +6,8 @@ interface SelectMultipleMenuProps { options: string[] selectedOptions: string[] filteredOptions: string[] - handleToggleAllOptions: () => void + searchValue: string + handleToggleAllOptions: (e: React.ChangeEvent) => void handleSearch: (e: React.ChangeEvent) => void handleCheck: (e: React.ChangeEvent) => void isSearchable: boolean @@ -17,6 +18,7 @@ export const SelectMultipleMenu = ({ options, selectedOptions, filteredOptions, + searchValue, handleToggleAllOptions, handleSearch, handleCheck, @@ -26,8 +28,15 @@ export const SelectMultipleMenu = ({ const searchInputControlID = useId() const toggleAllControlID = useId() - const noOptionsFound = filteredOptions.length === 0 - const allOptionsSelected = selectedOptions.length === options.length + const menuOptions = filteredOptions.length > 0 ? filteredOptions : options + + const noOptionsFound = searchValue !== '' && filteredOptions.length === 0 + + const allOptionsShownAreSelected = !noOptionsFound + ? filteredOptions.length > 0 + ? filteredOptions.every((option) => selectedOptions.includes(option)) + : options.every((option) => selectedOptions.includes(option)) + : false return ( {isSearchable ? ( - {filteredOptions.map((option) => ( - - - - ))} + {!noOptionsFound && + menuOptions.map((option) => ( + + + + ))} {noOptionsFound && ( diff --git a/packages/design-system/src/lib/components/select-multiple/selectMultipleReducer.ts b/packages/design-system/src/lib/components/select-multiple/selectMultipleReducer.ts index d5c468e10..107e4c6fb 100644 --- a/packages/design-system/src/lib/components/select-multiple/selectMultipleReducer.ts +++ b/packages/design-system/src/lib/components/select-multiple/selectMultipleReducer.ts @@ -22,7 +22,10 @@ type SelectMultipleActions = payload: string } | { - type: 'TOGGLE_ALL_OPTIONS' + type: 'SELECT_ALL_OPTIONS' + } + | { + type: 'DESELECT_ALL_OPTIONS' } | { type: 'SEARCH' @@ -44,19 +47,32 @@ export const selectMultipleReducer = ( ...state, selectedOptions: state.selectedOptions.filter((option) => option !== action.payload) } - case 'TOGGLE_ALL_OPTIONS': + case 'SELECT_ALL_OPTIONS': + return { + ...state, + selectedOptions: + state.filteredOptions.length > 0 + ? Array.from(new Set([...state.selectedOptions, ...state.filteredOptions])) + : state.options + } + case 'DESELECT_ALL_OPTIONS': return { ...state, - selectedOptions: state.selectedOptions.length === state.options.length ? [] : state.options + selectedOptions: + state.filteredOptions.length > 0 + ? state.selectedOptions.filter((option) => !state.filteredOptions.includes(option)) + : [] } + case 'SEARCH': return { ...state, - filteredOptions: action.payload - ? state.options.filter((option) => - option.toLowerCase().includes(action.payload.toLowerCase()) - ) - : state.options, + filteredOptions: + action.payload !== '' + ? state.options.filter((option) => + option.toLowerCase().includes(action.payload.toLowerCase()) + ) + : [], searchValue: action.payload } default: @@ -74,8 +90,12 @@ export const removeOption = /* istanbul ignore next */ (option: string): SelectM payload: option }) -export const toggleAllOptions = /* istanbul ignore next */ (): SelectMultipleActions => ({ - type: 'TOGGLE_ALL_OPTIONS' +export const selectAllOptions = /* istanbul ignore next */ (): SelectMultipleActions => ({ + type: 'SELECT_ALL_OPTIONS' +}) + +export const deselectAllOptions = /* istanbul ignore next */ (): SelectMultipleActions => ({ + type: 'DESELECT_ALL_OPTIONS' }) export const searchOptions = /* istanbul ignore next */ (value: string): SelectMultipleActions => ({ diff --git a/packages/design-system/tests/component/select-multiple/SelectMultiple.spec.tsx b/packages/design-system/tests/component/select-multiple/SelectMultiple.spec.tsx index 1df2c9f3f..e8363f8c3 100644 --- a/packages/design-system/tests/component/select-multiple/SelectMultiple.spec.tsx +++ b/packages/design-system/tests/component/select-multiple/SelectMultiple.spec.tsx @@ -90,7 +90,7 @@ describe('SelectMultiple', () => { cy.get('@onChange').should('not.have.been.called') }) - it('selects an option is shown as selected both in the menu as well as in the selected options', () => { + it('should select an option and be shown as selected both in the menu as well as in the selected options', () => { cy.mount( { }) }) - it('selects all options when the toggle all checkbox is checked while been unchecked before', () => { + it('selects all options', () => { cy.mount( { cy.findByText('Select...').should('not.exist') }) - it('deselects all options when the toggle all checkbox is unchecked while been checked before', () => { + it('deselects all options', () => { cy.mount( { cy.findByText('Select...').should('exist') }) + it('should select all filtered options', () => { + cy.mount( + + ) + cy.clock() + + cy.findByLabelText('Toggle options menu').click() + cy.findByPlaceholderText('Search...').type('Read') + + cy.tick(SELECT_MENU_SEARCH_DEBOUNCE_TIME) + + cy.findByLabelText('Reading').should('exist') + cy.findByLabelText('Swimming').should('not.exist') + cy.findByLabelText('Running').should('not.exist') + cy.findByLabelText('Cycling').should('not.exist') + cy.findByLabelText('Cooking').should('not.exist') + cy.findByLabelText('Gardening').should('not.exist') + + cy.findByLabelText('Toggle all options').click() + + cy.findByLabelText('List of selected options') + .should('exist') + .within(() => { + cy.findByText('Reading').should('exist') + cy.findByText('Swimming').should('not.exist') + cy.findByText('Running').should('not.exist') + cy.findByText('Cycling').should('not.exist') + cy.findByText('Cooking').should('not.exist') + cy.findByText('Gardening').should('not.exist') + }) + cy.findByText('Select...').should('not.exist') + }) + + it('should unselect only filtered options', () => { + cy.mount( + + ) + cy.clock() + + cy.findByLabelText('Toggle options menu').click() + cy.findByLabelText('Toggle all options').click() + + cy.findByLabelText('Reading').should('be.checked') + cy.findByLabelText('Swimming').should('be.checked') + cy.findByLabelText('Running').should('be.checked') + cy.findByLabelText('Cycling').should('be.checked') + cy.findByLabelText('Cooking').should('be.checked') + cy.findByLabelText('Gardening').should('be.checked') + + cy.findByPlaceholderText('Search...').type('Read') + + cy.tick(SELECT_MENU_SEARCH_DEBOUNCE_TIME) + + cy.findByLabelText('Reading').should('exist') + cy.findByLabelText('Swimming').should('not.exist') + cy.findByLabelText('Running').should('not.exist') + cy.findByLabelText('Cycling').should('not.exist') + cy.findByLabelText('Cooking').should('not.exist') + cy.findByLabelText('Gardening').should('not.exist') + + cy.findByLabelText('Toggle all options').click() + + cy.findByLabelText('List of selected options') + .should('exist') + .within(() => { + cy.findByText('Reading').should('not.exist') + cy.findByText('Swimming').should('exist') + cy.findByText('Running').should('exist') + cy.findByText('Cycling').should('exist') + cy.findByText('Cooking').should('exist') + cy.findByText('Gardening').should('exist') + }) + }) + it('should show correct filtered options when searching for a value', () => { cy.mount( { cy.findByText('2 selected').should('exist') }) - it('should show No Options Found when search does not match any option', () => { + it('should show No Options Found and toggle all chebox be disabled when search does not match any option', () => { cy.mount( { cy.findByPlaceholderText('Search...').type('Yoga') cy.findByText('No options found').should('exist') + cy.findByLabelText('Toggle all options').should('be.disabled') }) it('should be disabled when isDisabled is true', () => { diff --git a/packages/design-system/tests/component/select-multiple/selectMultipleReducer.spec.tsx b/packages/design-system/tests/component/select-multiple/selectMultipleReducer.spec.tsx index a70cf75e5..b8a93852d 100644 --- a/packages/design-system/tests/component/select-multiple/selectMultipleReducer.spec.tsx +++ b/packages/design-system/tests/component/select-multiple/selectMultipleReducer.spec.tsx @@ -13,7 +13,7 @@ describe('selectMultipleReducer', () => { expect(state).deep.equal(selectMultipleInitialState) }) - it('it should select an option', () => { + it('should select an option', () => { const state = selectMultipleReducer(selectMultipleInitialState, { type: 'SELECT_OPTION', payload: 'Reading' @@ -22,7 +22,7 @@ describe('selectMultipleReducer', () => { expect(state.selectedOptions).to.include('Reading') }) - it('it should remove an option', () => { + it('should remove an option', () => { const state = selectMultipleReducer( { ...selectMultipleInitialState, selectedOptions: ['Reading'] }, { @@ -34,18 +34,18 @@ describe('selectMultipleReducer', () => { expect(state.selectedOptions).to.not.include('Reading') }) - it('it should toggle all options', () => { + it('should select all available options when there are no current filtered options', () => { const state = selectMultipleReducer( { ...selectMultipleInitialState, options: ['Reading', 'Swimming'] }, { - type: 'TOGGLE_ALL_OPTIONS' + type: 'SELECT_ALL_OPTIONS' } ) - expect(state.selectedOptions).to.include('Reading', 'Swimming') + expect(state.selectedOptions).to.deep.equal(['Reading', 'Swimming']) }) - it('it should unselect all options', () => { + it('should deselect all available options when there are no current filtered options', () => { const state = selectMultipleReducer( { ...selectMultipleInitialState, @@ -53,14 +53,61 @@ describe('selectMultipleReducer', () => { selectedOptions: ['Reading', 'Swimming'] }, { - type: 'TOGGLE_ALL_OPTIONS' + type: 'DESELECT_ALL_OPTIONS' } ) expect(state.selectedOptions).to.be.empty }) - it('it should filter options', () => { + it('should select all filtered options', () => { + const state = selectMultipleReducer( + { + ...selectMultipleInitialState, + options: ['Reading', 'Swimming', 'Running'], + filteredOptions: ['Reading', 'Swimming'] + }, + { + type: 'SELECT_ALL_OPTIONS' + } + ) + + expect(state.selectedOptions).to.deep.equal(['Reading', 'Swimming']) + }) + + it('should deselect all filtered options', () => { + const state = selectMultipleReducer( + { + ...selectMultipleInitialState, + options: ['Reading', 'Swimming', 'Running'], + selectedOptions: ['Reading', 'Swimming'], + filteredOptions: ['Reading', 'Swimming'] + }, + { + type: 'DESELECT_ALL_OPTIONS' + } + ) + + expect(state.selectedOptions).to.be.empty + }) + + it('should add filtered options to selected options when selecting all if filtered options are present', () => { + const state = selectMultipleReducer( + { + ...selectMultipleInitialState, + options: ['Reading', 'Swimming', 'Running'], + selectedOptions: ['Reading', 'Swimming'], + filteredOptions: ['Running'] + }, + { + type: 'SELECT_ALL_OPTIONS' + } + ) + + expect(state.selectedOptions).to.deep.equal(['Reading', 'Swimming', 'Running']) + }) + + it('should filter options', () => { const state = selectMultipleReducer( { ...selectMultipleInitialState, options: ['Reading', 'Swimming', 'Running'] }, {