From 16896616256a6a892f045c7460c8d92da4649316 Mon Sep 17 00:00:00 2001 From: Preeti Bansal Date: Thu, 18 Apr 2024 14:19:29 +0530 Subject: [PATCH 01/48] feat: Add selectAll option in Multiselect --- .../src/components/MultiSelect/MultiSelect.tsx | 17 ++++++++++++++--- .../src/components/MultiSelect/tools/sorting.js | 3 +++ packages/react/src/internal/Selection.js | 7 +++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 685b3e7226ac..523c2e8104c8 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -220,6 +220,8 @@ export interface MultiSelectProps */ items: ItemType[]; + itemsWithSelectAll: ItemType[]; + /** * Generic `label` that will be used as the textual representation of what * this field is for @@ -362,6 +364,14 @@ const MultiSelect = React.forwardRef( const [prevOpenProp, setPrevOpenProp] = useState(open); const [topItems, setTopItems] = useState([]); const [itemsCleared, setItemsCleared] = useState(false); + + const selectAllOption = { + id: 'select-all-option', + text: 'All' + }; + + const itemsWithSelectAll = [selectAllOption, ...items]; + const { selectedItems: controlledSelectedItems, onItemChange, @@ -371,6 +381,7 @@ const MultiSelect = React.forwardRef( initialSelectedItems, onChange, selectedItems: selected, + itemsWithSelectAll }); const selectProps: UseSelectProps = { @@ -389,7 +400,7 @@ const MultiSelect = React.forwardRef( ); }, selectedItem: controlledSelectedItems, - items, + items: itemsWithSelectAll, isItemDisabled(item, _index) { return (item as any).disabled; }, @@ -558,7 +569,7 @@ const MultiSelect = React.forwardRef( } else { return { ...changes, - highlightedIndex: props.items.indexOf(highlightedIndex), + highlightedIndex: itemsWithSelectAll.indexOf(highlightedIndex), }; } case ToggleButtonKeyDownArrowDown: @@ -701,7 +712,7 @@ const MultiSelect = React.forwardRef( {isOpen && // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - sortItems!(items, sortOptions as SortItemsOptions).map( + sortItems!(itemsWithSelectAll, sortOptions as SortItemsOptions).map( (item, index) => { const isChecked = selectedItems.filter((selected) => isEqual(selected, item)) diff --git a/packages/react/src/components/MultiSelect/tools/sorting.js b/packages/react/src/components/MultiSelect/tools/sorting.js index 570c10871aa7..cac5bb88bbc4 100644 --- a/packages/react/src/components/MultiSelect/tools/sorting.js +++ b/packages/react/src/components/MultiSelect/tools/sorting.js @@ -32,6 +32,9 @@ export const defaultSortItems = ( const hasItemB = selectedItems.includes(itemB); // Prefer whichever item is in the `selectedItems` array first + if(itemA.id === 'select-all-option' || itemB.id === 'select-all-option') { + return 1 + } if (hasItemA && !hasItemB) { return -1; } diff --git a/packages/react/src/internal/Selection.js b/packages/react/src/internal/Selection.js index 35bc753c8ddd..5671d6c39f20 100644 --- a/packages/react/src/internal/Selection.js +++ b/packages/react/src/internal/Selection.js @@ -30,6 +30,7 @@ export function useSelection({ onChange, initialSelectedItems = [], selectedItems: controlledItems, + itemsWithSelectAll }) { const isMounted = useRef(false); const savedOnChange = useRef(onChange); @@ -54,6 +55,12 @@ export function useSelection({ if (disabled) { return; } + //select all option + if(item && item.id === 'select-all-option') { + setSelectedItems(itemsWithSelectAll); + return; + } + let selectedIndex; selectedItems.forEach((selectedItem, index) => { if (isEqual(selectedItem, item)) { From 5d94668b44845959f28685b61f5d03fef1c6d02f Mon Sep 17 00:00:00 2001 From: Gururajj77 Date: Thu, 18 Apr 2024 18:02:07 +0530 Subject: [PATCH 02/48] disabled items and length corrected --- packages/react/src/components/MultiSelect/MultiSelect.tsx | 4 +++- packages/react/src/internal/Selection.js | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 523c2e8104c8..d3b86f7b766d 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -639,7 +639,9 @@ const MultiSelect = React.forwardRef( const itemsSelectedText = selectedItems.length > 0 && selectedItems.map((item) => (item as selectedItemType).text); + console.log(selectedItems) + const selectedItemsWithoutSelectAll = selectedItems.filter((item: any) => item.id !== "select-all-option"); return (
); diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 91a325f18d55..621f8d74b53c 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -331,27 +331,17 @@ const MultiSelect = React.forwardRef( }: MultiSelectProps, ref: ForwardedRef ) => { - const sortOptions = { - selectedItems: selected, - itemToString, - compareItems, - locale, - }; - const filteredItems = useMemo(() => { - return sortItems!( - items.filter((item) => { - if (typeof item === 'object' && item !== null) { - for (const key in item) { - if (Object.hasOwn(item, key) && item[key] === undefined) { - return false; // Return false if any property has an undefined value - } + return items.filter((item) => { + if (typeof item === 'object' && item !== null) { + for (const key in item) { + if (Object.hasOwn(item, key) && item[key] === undefined) { + return false; // Return false if any property has an undefined value } } - return true; // Return true if item is not an object with undefined values - }), - sortOptions as SortItemsOptions - ); + } + return true; // Return true if item is not an object with undefined values + }); }, [items]); let selectAll = filteredItems.some((item) => (item as any).isSelectAll); @@ -421,6 +411,13 @@ const MultiSelect = React.forwardRef( filteredItems, }); + const sortOptions = { + selectedItems: controlledSelectedItems, + itemToString, + compareItems, + locale, + }; + const selectProps: UseSelectProps = { stateReducer, isOpen, @@ -675,9 +672,9 @@ const MultiSelect = React.forwardRef( selectedItems.length > 0 && selectedItems.map((item) => (item as selectedItemType)?.text); - const selectedItemsWithoutSelectAll = selectedItems.filter( - (item: any) => !item.isSelectAll - ); + const selectedItemsWithoutSelectAll = selectAll + ? selectedItems.filter((item: any) => !item.isSelectAll) + : selectedItems; // Memoize the value of getMenuProps to avoid an infinite loop const menuProps = useMemo( @@ -761,7 +758,10 @@ const MultiSelect = React.forwardRef( {isOpen && - filteredItems.map((item, index) => { + sortItems!( + filteredItems, + sortOptions as SortItemsOptions + ).map((item, index) => { const isChecked = selectedItems.filter((selected) => isEqual(selected, item)) .length > 0; diff --git a/packages/react/src/components/MultiSelect/tools/sorting.js b/packages/react/src/components/MultiSelect/tools/sorting.js index fde26755b055..aadb00c0f5e1 100644 --- a/packages/react/src/components/MultiSelect/tools/sorting.js +++ b/packages/react/src/components/MultiSelect/tools/sorting.js @@ -27,30 +27,19 @@ export const defaultSortItems = ( items, { selectedItems = [], itemToString, compareItems, locale = 'en' } ) => { - // Extract the "select all" option - const selectAllOption = items.find((item) => item.isSelectAll); + return items.sort((itemA, itemB) => { + // Always place "select all" option at the beginning + if (itemA.isSelectAll) return -1; + if (itemB.isSelectAll) return 1; - // Filter out the "select all" option from the items array - const filteredItems = items.filter((item) => !item.isSelectAll); - - // Sort the filtered items - const sortedItems = filteredItems.sort((itemA, itemB) => { const hasItemA = selectedItems.includes(itemA); const hasItemB = selectedItems.includes(itemB); - if (hasItemA && !hasItemB) { - return -1; - } - - if (hasItemB && !hasItemA) { - return 1; - } + if (hasItemA && !hasItemB) return -1; + if (hasItemB && !hasItemA) return 1; return compareItems(itemToString(itemA), itemToString(itemB), { locale, }); }); - - // Add the "select all" option to the beginning of the sorted array, if it exists - return selectAllOption ? [selectAllOption, ...sortedItems] : sortedItems; }; From 5f082b69febb1a1cb3b1cc16f150a716836a524b Mon Sep 17 00:00:00 2001 From: Preeti Bansal Date: Fri, 23 Aug 2024 16:46:26 +0530 Subject: [PATCH 46/48] fix: test case --- packages/react/src/components/MultiSelect/MultiSelect.tsx | 8 ++++---- .../components/MultiSelect/__tests__/MultiSelect-test.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react/src/components/MultiSelect/MultiSelect.tsx b/packages/react/src/components/MultiSelect/MultiSelect.tsx index 621f8d74b53c..61271e3ce42b 100644 --- a/packages/react/src/components/MultiSelect/MultiSelect.tsx +++ b/packages/react/src/components/MultiSelect/MultiSelect.tsx @@ -672,9 +672,9 @@ const MultiSelect = React.forwardRef( selectedItems.length > 0 && selectedItems.map((item) => (item as selectedItemType)?.text); - const selectedItemsWithoutSelectAll = selectAll - ? selectedItems.filter((item: any) => !item.isSelectAll) - : selectedItems; + const selectedItemsLength = selectAll + ? selectedItems.filter((item: any) => !item.isSelectAll).length + : selectedItems.length; // Memoize the value of getMenuProps to avoid an infinite loop const menuProps = useMemo( @@ -727,7 +727,7 @@ const MultiSelect = React.forwardRef( clearSelection={ !disabled && !readOnly ? clearSelection : noopFn } - selectionCount={selectedItemsWithoutSelectAll.length} + selectionCount={selectedItemsLength} // eslint-disable-next-line @typescript-eslint/no-non-null-assertion translateWithId={translateWithId!} disabled={disabled} diff --git a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js index 65abe13d6d3c..f595211b9645 100644 --- a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js +++ b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js @@ -543,7 +543,7 @@ describe('MultiSelect', () => { ); // the first option in the list to the the former third option in the list - expect(optionsArray[2]).toHaveAttribute('aria-label', 'Item 2'); + expect(optionsArray[0]).toHaveAttribute('aria-label', 'Item 2'); }); it('should accept a `ref` for the underlying button element', async () => { From 03d645742a63b214ecb4991db68890d13bb83a73 Mon Sep 17 00:00:00 2001 From: Preeti Bansal Date: Tue, 27 Aug 2024 16:20:57 +0530 Subject: [PATCH 47/48] fix: alignment issue --- .../components/multiselect/_multiselect.scss | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/styles/scss/components/multiselect/_multiselect.scss b/packages/styles/scss/components/multiselect/_multiselect.scss index b586dbaa6dae..a9acb0c08573 100644 --- a/packages/styles/scss/components/multiselect/_multiselect.scss +++ b/packages/styles/scss/components/multiselect/_multiselect.scss @@ -82,6 +82,23 @@ white-space: nowrap; } + .#{$prefix}--multi-select + .#{$prefix}--list-box__menu-item__option + .#{$prefix}--checkbox-label::before { + margin-block: 0; + } + + .#{$prefix}--multi-select + .#{$prefix}--list-box__menu-item__option + .cds--checkbox:indeterminate + + .cds--checkbox-label::after { + inset-block-start: convert.to-rem(9px); + } + .#{$prefix}--multi-select + .#{$prefix}--list-box__menu-item__option + .cds--checkbox-label::after { + inset-block-start: convert.to-rem(5px); + } .#{$prefix}--multi-select .#{$prefix}--list-box__menu-item__option > .#{$prefix}--form-item { From 38e27980469efa83fd50bd1461c4b469d51ce0d2 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Fri, 30 Aug 2024 14:41:47 -0500 Subject: [PATCH 48/48] test(multiselect): refactor assertions off data attributes --- .../MultiSelect/__tests__/MultiSelect-test.js | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js index f595211b9645..fd977410fd70 100644 --- a/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js +++ b/packages/react/src/components/MultiSelect/__tests__/MultiSelect-test.js @@ -158,10 +158,16 @@ describe('MultiSelect', () => { // eslint-disable-next-line testing-library/prefer-screen-queries const itemNode = getByText(container, item.label); - expect( - // eslint-disable-next-line testing-library/no-node-access - document.querySelector('[aria-selected="true"][role="option"]') - ).toBeNull(); + const options = screen.getAllByRole('option'); + for (const option of options) { + expect(option).toHaveAttribute('aria-selected', 'false'); + } + + await userEvent.click(itemNode); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + + await userEvent.click(itemNode); + expect(options[0]).toHaveAttribute('aria-selected', 'false'); }); it('should close the menu when the user hits the Escape key', async () => { @@ -227,6 +233,23 @@ describe('MultiSelect', () => { await userEvent.tab(); await userEvent.keyboard('[Space]'); + + const [item] = items; + // eslint-disable-next-line testing-library/prefer-screen-queries + const itemNode = getByText(container, item.label); + + const options = screen.getAllByRole('option'); + for (const option of options) { + expect(option).toHaveAttribute('aria-selected', 'false'); + } + + expect(options[0]).toHaveAttribute('aria-selected', 'false'); + + await userEvent.keyboard('[Enter]'); + await userEvent.keyboard('[ArrowDown]'); + await userEvent.keyboard('[Enter]'); + + expect(options[0]).toHaveAttribute('aria-selected', 'true'); }); it('should clear selected items when the user clicks the clear selection button', async () => { @@ -321,6 +344,12 @@ describe('MultiSelect', () => { const labelNode = getByText(container, label); await userEvent.click(labelNode); + + const options = screen.getAllByRole('option'); + expect(options[0]).toHaveAttribute('aria-selected', 'true'); + expect(options[1]).toHaveAttribute('aria-selected', 'true'); + expect(options[2]).toHaveAttribute('aria-selected', 'false'); + expect(options[3]).toHaveAttribute('aria-selected', 'false'); }); it('should trigger onChange with selected items', async () => {