From 1e5e468a4ab99e8a9247b2ef13897e381e0b329d Mon Sep 17 00:00:00 2001 From: Lu Khei Chong Date: Mon, 7 Oct 2024 17:04:47 +0800 Subject: [PATCH 1/3] fix(combobox): trigger state update of menulist when prop menuList changes #271 --- src/Combobox/Combobox.tsx | 25 +++- .../components/Combobox/Combobox.stories.mdx | 20 +++ tests/Combobox/Combobox.test.tsx | 131 +++++++++++++++--- 3 files changed, 149 insertions(+), 27 deletions(-) diff --git a/src/Combobox/Combobox.tsx b/src/Combobox/Combobox.tsx index 8b7dbdb8..66deea21 100644 --- a/src/Combobox/Combobox.tsx +++ b/src/Combobox/Combobox.tsx @@ -84,6 +84,19 @@ const filterIncludes = (inputValue: string, menuList: string[]) => { return filtered; }; +const filteredMenuList = (initialValue: string, filterMethod: 'startsWith' | 'includes' | CustomFilter, menuList: Array) => { + let filteredMenu = [] + if(filterMethod === "startsWith"){ + filteredMenu = filterStartsWith(initialValue, menuList) + } else if (filterMethod === "includes") { + filteredMenu = filterIncludes(initialValue, menuList) + } else { + filteredMenu = filterMethod(initialValue, menuList) + } + + return filteredMenu +} + export const Combobox: BsPrefixRefForwardingComponent<'input', ComboboxProps> = React.forwardRef( ( @@ -95,7 +108,7 @@ export const Combobox: BsPrefixRefForwardingComponent<'input', ComboboxProps> = label = '', icon, scrollable, - filterMethod, + filterMethod = "startsWith", ...props }, ref @@ -110,9 +123,7 @@ export const Combobox: BsPrefixRefForwardingComponent<'input', ComboboxProps> = value: initialValue, invalid: false, menuList: initialValue - ? menuList.filter((n) => - n.toLowerCase().startsWith(initialValue.toLowerCase()) - ) + ? filteredMenuList(initialValue, filterMethod, menuList) : menuList, }; const [state, setState] = useState(initialState); @@ -170,7 +181,11 @@ export const Combobox: BsPrefixRefForwardingComponent<'input', ComboboxProps> = React.useEffect(() => { setComboboxMenuId(generateId('combobox', 'ul')); }, []); - + React.useEffect(() => { + setState({...state, menuList: initialValue + ? filteredMenuList(initialValue, filterMethod, menuList) + : menuList}) + }, [menuList]); return ( <> {label && {label}} diff --git a/stories/components/Combobox/Combobox.stories.mdx b/stories/components/Combobox/Combobox.stories.mdx index c95ca6ea..2afeb1cb 100644 --- a/stories/components/Combobox/Combobox.stories.mdx +++ b/stories/components/Combobox/Combobox.stories.mdx @@ -255,6 +255,7 @@ export const ComboboxComponent = () => { }; ``` + ## onChangeInput `onChangeInput` is fired whenever the value of input changes whether due to typing directly in the input or from selection from menu. @@ -390,3 +391,22 @@ export const ComboboxCustomFilterExample = () => { ); }; ``` + +export const ComboboxFetchData = () => { + const [emails, setEmails] = useState([]); + useEffect(() => { + fetch(`https://jsonplaceholder.typicode.com/comments?postId=1`) + .then((response) => response.json()) + .then((json) => { + const emails = json.map(j => j.email) + setEmails(emails) + }); + }, []) + return ; +} + +## Demo with data fetching + + +{ComboboxFetchData.bind({})} + \ No newline at end of file diff --git a/tests/Combobox/Combobox.test.tsx b/tests/Combobox/Combobox.test.tsx index 4b4da790..cbd0ff94 100644 --- a/tests/Combobox/Combobox.test.tsx +++ b/tests/Combobox/Combobox.test.tsx @@ -226,8 +226,7 @@ describe(' a11y', () => { }); it('dropdown menu has role="listbox" on ul element', async () => { const { container } = render(); - fireEvent.click(container - .querySelector('input.form-control')!) + fireEvent.click(container.querySelector('input.form-control')!); expect(container.querySelector('ul.dropdown-menu')).toHaveAttribute( 'role', 'listbox' @@ -235,14 +234,13 @@ describe(' a11y', () => { }); it('form control input aria-controls id points to ul id', async () => { const { container } = render(); - fireEvent.click(container - .querySelector('input.form-control')!) + fireEvent.click(container.querySelector('input.form-control')!); const menuUlId = container.querySelector('ul')?.getAttribute('id'); const inputAriaControlValue = container .querySelector('input.form-control') ?.getAttribute('aria-controls'); - expect(inputAriaControlValue).toEqual(menuUlId); + expect(inputAriaControlValue).toEqual(menuUlId); }); }); describe('', () => { @@ -398,22 +396,100 @@ describe('', () => { await waitFor(() => { expect(container.querySelector('input')?.value).toEqual('Afghanistan'); }); - fireEvent.keyDown(container.querySelectorAll('li>button.dropdown-item')[0], { - key: 'ArrowDown', - code: 'ArrowDown', - }); + fireEvent.keyDown( + container.querySelectorAll('li>button.dropdown-item')[0], + { + key: 'ArrowDown', + code: 'ArrowDown', + } + ); await waitFor(() => { expect(container.querySelector('input')?.value).toEqual('Albania'); }); - fireEvent.keyDown(container.querySelectorAll('li>button.dropdown-item')[1], { - key: 'ArrowDown', - code: 'ArrowDown', - }); + fireEvent.keyDown( + container.querySelectorAll('li>button.dropdown-item')[1], + { + key: 'ArrowDown', + code: 'ArrowDown', + } + ); await waitFor(() => { expect(container.querySelector('input')?.value).toEqual('Algeria'); }); }); + it('menuList is filtered with "ang" initialValue and includes filter method gives 3 items', async () => { + const { container } = render( + + ); + fireEvent.click( + container.querySelector('input.form-control.dropdown-toggle')! + ); + + await waitFor(() => { + expect(container.querySelector('ul.dropdown-menu')).toBeInTheDocument(); + }); + expect(container.querySelectorAll('li>button.dropdown-item').length).toEqual(3); + }); + it('menuList is filtered with "ang" initialValue and includes filter method gives 2 items', async () => { + const { container } = render( + + ); + fireEvent.click( + container.querySelector('input.form-control.dropdown-toggle')! + ); + + await waitFor(() => { + expect(container.querySelector('ul.dropdown-menu')).toBeInTheDocument(); + }); + expect(container.querySelectorAll('li>button.dropdown-item').length).toEqual(2); + }); + it('menuList is filtered with "ang" initialValue and endsWith custom filter method gives 0 items', async () => { + const customFilter = (inputValue: string, menuItems: string[]) => { + const filtered = menuItems.filter((n) => { + const nLowerCase = n.toLowerCase(); + const valueLower = inputValue.toLowerCase(); + return nLowerCase.endsWith(valueLower); + }); + return filtered; + }; + const { container } = render( + + ); + fireEvent.click( + container.querySelector('input.form-control.dropdown-toggle')! + ); + + await waitFor(() => { + expect(container.querySelector('ul.dropdown-menu')).not.toBeInTheDocument(); + }); + expect(container.querySelectorAll('li>button.dropdown-item').length).toEqual(0); + }); + it('items in menu are updated when menuList prop changes', async () => { + const { container, rerender } = render(); + fireEvent.click( + container.querySelector('input.form-control.dropdown-toggle')! + ); + expect(container.querySelector('ul.dropdown-menu')).not.toBeInTheDocument(); + rerender(); + await waitFor(() => { + expect(container.querySelector('ul.dropdown-menu')).toBeInTheDocument(); + }); + const dropdownItems = container.querySelectorAll('li>button.dropdown-item'); + expect(dropdownItems.length).toEqual(menuList.length); + }); it('onChangeInput fires when input changes', async () => { const mockFn = jest.fn(); const { container } = render( @@ -458,7 +534,9 @@ describe('', () => { it('icon has a default value ', () => { const { container } = render(); expect( - container.querySelector('.dropdown.combobox> i.form-control-icon.bi-chevron-down') + container.querySelector( + '.dropdown.combobox> i.form-control-icon.bi-chevron-down' + ) ).toBeInTheDocument(); }); it('when icon defined, icon in container', () => { @@ -474,13 +552,15 @@ describe('', () => { }); it('when scrollable=true, adds scrollable class to combobox', () => { const { container } = render( - } /> + } + /> ); expect( - container.querySelector( - '.dropdown.combobox.scrollable' - ) + container.querySelector('.dropdown.combobox.scrollable') ).toBeInTheDocument(); }); it('For filterMethod=includes, when gh is typed matches one of menuList, country filtered menuList should show only strings including gh. Results in 2', async () => { @@ -499,14 +579,16 @@ describe('', () => { ); await waitFor(() => { expect(container.querySelector('ul.dropdown-menu')).toBeInTheDocument(); - const dropdownItem = container.querySelectorAll("li>button.dropdown-item") + const dropdownItem = container.querySelectorAll( + 'li>button.dropdown-item' + ); expect(dropdownItem.length).toEqual(2); expect(dropdownItem[0].textContent).toEqual('Afghanistan'); expect(dropdownItem[1].textContent).toEqual('Ghana'); }); }); it('For custom filterMethod, custom filter behaviour is applied instead', async () => { - const customFilter = (inputValue: string, menuItems: string[]) => { + const customFilter = (inputValue: string, menuItems: string[]) => { const filtered = menuItems.filter((n) => { const nLowerCase = n.toLowerCase(); const valueLower = inputValue.toLowerCase(); @@ -515,7 +597,10 @@ describe('', () => { return filtered; }; const { container } = render( - + ); fireEvent.change( @@ -525,7 +610,9 @@ describe('', () => { expect(container.querySelector('input')?.value).toEqual('e'); await waitFor(() => { expect(container.querySelector('ul.dropdown-menu')).toBeInTheDocument(); - const dropdownItem = container.querySelectorAll("li>button.dropdown-item") + const dropdownItem = container.querySelectorAll( + 'li>button.dropdown-item' + ); expect(dropdownItem.length).toEqual(2); expect(dropdownItem[0].textContent).toEqual('apple'); expect(dropdownItem[1].textContent).toEqual('orange'); From 612b81e19459ef19abea7b5f09844f17fd3c2581 Mon Sep 17 00:00:00 2001 From: Lu Khei Chong Date: Mon, 7 Oct 2024 17:37:49 +0800 Subject: [PATCH 2/3] fix(combobox): add type=button to dropdown items in combobox menu #269 --- src/Combobox/Combobox.tsx | 1 + tests/Combobox/Combobox.test.tsx | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Combobox/Combobox.tsx b/src/Combobox/Combobox.tsx index 66deea21..c35f8850 100644 --- a/src/Combobox/Combobox.tsx +++ b/src/Combobox/Combobox.tsx @@ -213,6 +213,7 @@ export const Combobox: BsPrefixRefForwardingComponent<'input', ComboboxProps> = {state.menuList.map((menuItem) => ( a11y', () => { it('dropdown menu has role="listbox" on ul element', async () => { const { container } = render(); fireEvent.click(container.querySelector('input.form-control')!); - expect(container.querySelector('ul.dropdown-menu')).toHaveAttribute( - 'role', - 'listbox' - ); + await waitFor(() => { + expect(container.querySelector('ul.dropdown-menu')).toHaveAttribute( + 'role', + 'listbox' + ); + }) }); it('form control input aria-controls id points to ul id', async () => { const { container } = render(); @@ -239,8 +242,7 @@ describe(' a11y', () => { const inputAriaControlValue = container .querySelector('input.form-control') ?.getAttribute('aria-controls'); - - expect(inputAriaControlValue).toEqual(menuUlId); + await waitFor(() => expect(inputAriaControlValue).toEqual(menuUlId)); }); }); describe('', () => { @@ -452,7 +454,7 @@ describe('', () => { expect(container.querySelectorAll('li>button.dropdown-item').length).toEqual(2); }); it('menuList is filtered with "ang" initialValue and endsWith custom filter method gives 0 items', async () => { - const customFilter = (inputValue: string, menuItems: string[]) => { + const customFilter: CustomFilter = (inputValue: string, menuItems: string[]) => { const filtered = menuItems.filter((n) => { const nLowerCase = n.toLowerCase(); const valueLower = inputValue.toLowerCase(); @@ -599,7 +601,7 @@ describe('', () => { const { container } = render( ); From 0b707167976c647acd92134cae32571b7574f9c3 Mon Sep 17 00:00:00 2001 From: Lu Khei Chong Date: Tue, 8 Oct 2024 09:19:35 +0800 Subject: [PATCH 3/3] refactor(combobox): filteredMenuList --- src/Combobox/Combobox.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Combobox/Combobox.tsx b/src/Combobox/Combobox.tsx index c35f8850..e1833afd 100644 --- a/src/Combobox/Combobox.tsx +++ b/src/Combobox/Combobox.tsx @@ -85,16 +85,13 @@ const filterIncludes = (inputValue: string, menuList: string[]) => { }; const filteredMenuList = (initialValue: string, filterMethod: 'startsWith' | 'includes' | CustomFilter, menuList: Array) => { - let filteredMenu = [] if(filterMethod === "startsWith"){ - filteredMenu = filterStartsWith(initialValue, menuList) - } else if (filterMethod === "includes") { - filteredMenu = filterIncludes(initialValue, menuList) - } else { - filteredMenu = filterMethod(initialValue, menuList) - } - - return filteredMenu + return filterStartsWith(initialValue, menuList); + } + if (filterMethod === "includes") { + return filterIncludes(initialValue, menuList); + } + return filterMethod(initialValue, menuList); } export const Combobox: BsPrefixRefForwardingComponent<'input', ComboboxProps> =