From cb30e6756107635f2288d1d2a8487017464954b7 Mon Sep 17 00:00:00 2001 From: Gururaj J <89023023+Gururajj77@users.noreply.github.com> Date: Tue, 11 Jun 2024 20:07:05 +0530 Subject: [PATCH] feat: Added floating-ui hook usage for `ComboBox` (#16585) * feat: changes * feat: added floating-ui for combobox * feat: added test helper function to pass tests * feat: test changes * Update packages/react/src/components/ComboBox/ComboBox.tsx update comments for interface Co-authored-by: Taylor Jones * Update packages/react/src/components/ComboBox/ComboBox.tsx update comment for prop-types Co-authored-by: Taylor Jones * Update packages/react/src/components/ComboBox/ComboBox.tsx removing `fallbackAxisSideDirection` and keeping it flip() Co-authored-by: Taylor Jones * Update packages/react/src/components/ComboBox/ComboBox.tsx adding placement as we already have direction prop coming in Co-authored-by: Taylor Jones * feat: yarn formatted --------- Co-authored-by: Taylor Jones Co-authored-by: Guilherme Datilio Ribeiro --- .../__snapshots__/PublicAPI-test.js.snap | 3 ++ .../src/components/ComboBox/ComboBox-test.js | 33 ++++++++------ .../components/ComboBox/ComboBox.stories.js | 15 +++++++ .../src/components/ComboBox/ComboBox.tsx | 43 ++++++++++++++++++- .../__tests__/FluidComboBox-test.js | 36 ++++++++++------ .../src/components/ListBox/test-helpers.js | 3 ++ 6 files changed, 106 insertions(+), 27 deletions(-) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 94fb3a79a572..60b019f0a0a8 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -1112,6 +1112,9 @@ Map { "type": "string", }, "ariaLabel": [Function], + "autoAlign": Object { + "type": "bool", + }, "className": Object { "type": "string", }, diff --git a/packages/react/src/components/ComboBox/ComboBox-test.js b/packages/react/src/components/ComboBox/ComboBox-test.js index 3a532ec3c865..69308e762dc8 100644 --- a/packages/react/src/components/ComboBox/ComboBox-test.js +++ b/packages/react/src/components/ComboBox/ComboBox-test.js @@ -14,6 +14,7 @@ import { assertMenuClosed, generateItems, generateGenericItem, + waitForPosition, } from '../ListBox/test-helpers'; import ComboBox from '../ComboBox'; import { act } from 'react'; @@ -144,23 +145,24 @@ describe('ComboBox', () => { expect(findInputNode()).toHaveDisplayValue('Apple'); }); - it('should respect slug prop', () => { + it('should respect slug prop', async () => { const { container } = render(} />); - + await waitForPosition(); expect(container.firstChild).toHaveClass( `${prefix}--list-box__wrapper--slug` ); }); describe('should display initially selected item found in `initialSelectedItem`', () => { - it('using an object type for the `initialSelectedItem` prop', () => { + it('using an object type for the `initialSelectedItem` prop', async () => { render( ); + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label); }); - it('using a string type for the `initialSelectedItem` prop', () => { + it('using a string type for the `initialSelectedItem` prop', async () => { // Replace the 'items' property in mockProps with a list of strings mockProps = { ...mockProps, @@ -170,19 +172,19 @@ describe('ComboBox', () => { render( ); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[1]); }); }); describe('should display selected item found in `selectedItem`', () => { - it('using an object type for the `selectedItem` prop', () => { + it('using an object type for the `selectedItem` prop', async () => { render(); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label); }); - it('using a string type for the `selectedItem` prop', () => { + it('using a string type for the `selectedItem` prop', async () => { // Replace the 'items' property in mockProps with a list of strings mockProps = { ...mockProps, @@ -190,7 +192,7 @@ describe('ComboBox', () => { }; render(); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[1]); }); }); @@ -198,7 +200,7 @@ describe('ComboBox', () => { describe('when disabled', () => { it('should not let the user edit the input node', async () => { render(); - + await waitForPosition(); expect(findInputNode()).toHaveAttribute('disabled'); expect(findInputNode()).toHaveDisplayValue(''); @@ -210,6 +212,7 @@ describe('ComboBox', () => { it('should not let the user expand the menu', async () => { render(); + await waitForPosition(); await openMenu(); expect(findListBoxNode()).not.toHaveClass( `${prefix}--list-box--expanded` @@ -220,7 +223,7 @@ describe('ComboBox', () => { describe('when readonly', () => { it('should not let the user edit the input node', async () => { render(); - + await waitForPosition(); expect(findInputNode()).toHaveAttribute('readonly'); expect(findInputNode()).toHaveDisplayValue(''); @@ -232,6 +235,7 @@ describe('ComboBox', () => { it('should not let the user expand the menu', async () => { render(); + await waitForPosition(); await openMenu(); expect(findListBoxNode()).not.toHaveClass( `${prefix}--list-box--expanded` @@ -240,9 +244,9 @@ describe('ComboBox', () => { }); describe('downshift quirks', () => { - it('should set `inputValue` to an empty string if a false-y value is given', () => { + it('should set `inputValue` to an empty string if a false-y value is given', async () => { render(); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(''); }); @@ -257,6 +261,7 @@ describe('ComboBox', () => { ); + await waitForPosition(); const firstCombobox = screen.getByTestId('combobox-1'); const secondCombobox = screen.getByTestId('combobox-2'); @@ -291,6 +296,7 @@ describe('ComboBox', () => { }); it('should open menu without moving focus on pressing Alt+ DownArrow', async () => { render(); + await waitForPosition(); act(() => { screen.getByRole('combobox').focus(); }); @@ -300,6 +306,7 @@ describe('ComboBox', () => { it('should close menu and return focus to combobox on pressing Alt+ UpArrow', async () => { render(); + await waitForPosition(); await openMenu(); await userEvent.keyboard('{Alt>}{ArrowUp}'); assertMenuClosed(mockProps); diff --git a/packages/react/src/components/ComboBox/ComboBox.stories.js b/packages/react/src/components/ComboBox/ComboBox.stories.js index dde72b5b4618..d43b231671a0 100644 --- a/packages/react/src/components/ComboBox/ComboBox.stories.js +++ b/packages/react/src/components/ComboBox/ComboBox.stories.js @@ -92,6 +92,21 @@ export const AllowCustomValue = () => { ); }; +export const ExperimentalAutoAlign = () => ( +
+
+ {}} + id="carbon-combobox" + items={items} + itemToString={(item) => (item ? item.text : '')} + titleText="ComboBox title" + helperText="Combobox helper text" + autoAlign={true} + /> +
+
+); export const _WithLayer = () => ( diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index a9b88d534495..3563cf7a207e 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -43,6 +43,7 @@ import mergeRefs from '../../tools/mergeRefs'; import deprecate from '../../prop-types/deprecate'; import { usePrefix } from '../../internal/usePrefix'; import { FormContext } from '../FluidForm'; +import { useFloating, flip, autoUpdate } from '@floating-ui/react'; const { InputBlur, @@ -150,6 +151,13 @@ export interface ComboBoxProps */ ariaLabel?: string; + /** + * **Experimental**: Will attempt to automatically align the floating + * element to avoid collisions with the viewport and being clipped by + * ancestor elements. + */ + autoAlign?: boolean; + /** * An optional className to add to the container node */ @@ -313,6 +321,7 @@ const ComboBox = forwardRef( const { ['aria-label']: ariaLabel = 'Choose an item', ariaLabel: deprecatedAriaLabel, + autoAlign = false, className: containerClassName, direction = 'bottom', disabled = false, @@ -342,6 +351,30 @@ const ComboBox = forwardRef( slug, ...rest } = props; + const { refs, floatingStyles } = useFloating( + autoAlign + ? { + placement: direction, + strategy: 'fixed', + middleware: [flip()], + whileElementsMounted: autoUpdate, + } + : {} + ); + const parentWidth = (refs?.reference?.current as HTMLElement)?.clientWidth; + + useEffect(() => { + if (autoAlign) { + Object.keys(floatingStyles).forEach((style) => { + if (refs.floating.current) { + refs.floating.current.style[style] = floatingStyles[style]; + } + }); + if (parentWidth && refs.floating.current) { + refs.floating.current.style.width = parentWidth + 'px'; + } + } + }, [autoAlign, floatingStyles, refs.floating, parentWidth]); const prefix = usePrefix(); const { isFluid } = useContext(FormContext); const textInput = useRef(null); @@ -630,6 +663,7 @@ const ComboBox = forwardRef( light={light} size={size} warn={warn} + ref={refs.setReference} warnText={warnText} warnTextId={warnTextId}>
@@ -739,7 +773,8 @@ const ComboBox = forwardRef( + })} + ref={mergeRefs(getMenuProps().ref, refs.setFloating)}> {isOpen ? filterItems(items, itemToString, inputValue).map( (item, index) => { @@ -821,6 +856,12 @@ ComboBox.propTypes = { PropTypes.string, 'This prop syntax has been deprecated. Please use the new `aria-label`.' ), + /** + * **Experimental**: Will attempt to automatically align the floating + * element to avoid collisions with the viewport and being clipped by + * ancestor elements. + */ + autoAlign: PropTypes.bool, /** * An optional className to add to the container node diff --git a/packages/react/src/components/FluidComboBox/__tests__/FluidComboBox-test.js b/packages/react/src/components/FluidComboBox/__tests__/FluidComboBox-test.js index 7d7647edcf65..94abc2dcfe68 100644 --- a/packages/react/src/components/FluidComboBox/__tests__/FluidComboBox-test.js +++ b/packages/react/src/components/FluidComboBox/__tests__/FluidComboBox-test.js @@ -14,6 +14,7 @@ import { assertMenuClosed, generateItems, generateGenericItem, + waitForPosition, } from '../../ListBox/test-helpers'; import FluidComboBox from '../FluidComboBox'; @@ -38,15 +39,17 @@ describe('FluidComboBox', () => { }; }); - it('should render with fluid classes', () => { + it('should render with fluid classes', async () => { const { container } = render(); + await waitForPosition(); expect(container.firstChild).toHaveClass( `${prefix}--list-box__wrapper--fluid` ); }); - it('should render with condensed styles if isCondensed is provided', () => { + it('should render with condensed styles if isCondensed is provided', async () => { const { container } = render(); + await waitForPosition(); expect(container.firstChild).toHaveClass( `${prefix}--list-box__wrapper--fluid--condensed` ); @@ -54,7 +57,7 @@ describe('FluidComboBox', () => { it('should display the menu of items when a user clicks on the input', async () => { render(); - + await waitForPosition(); await openMenu(); assertMenuOpen(mockProps); @@ -62,6 +65,7 @@ describe('FluidComboBox', () => { it('should call `onChange` each time an item is selected', async () => { render(); + await waitForPosition(); expect(mockProps.onChange).not.toHaveBeenCalled(); for (let i = 0; i < mockProps.items.length; i++) { @@ -79,7 +83,7 @@ describe('FluidComboBox', () => { it('capture filter text events', async () => { const onInputChange = jest.fn(); render(); - + await waitForPosition(); await userEvent.type(findInputNode(), 'something'); expect(onInputChange).toHaveBeenCalledWith('something'); @@ -90,6 +94,7 @@ describe('FluidComboBox', () => { return
{item.text}
; }); render(); + await waitForPosition(); await openMenu(); expect(itemToElement).toHaveBeenCalled(); @@ -97,6 +102,7 @@ describe('FluidComboBox', () => { it('should let the user select an option by clicking on the option node', async () => { render(); + await waitForPosition(); await openMenu(); await userEvent.click(screen.getAllByRole('option')[0]); @@ -119,17 +125,18 @@ describe('FluidComboBox', () => { }); describe('should display initially selected item found in `initialSelectedItem`', () => { - it('using an object type for the `initialSelectedItem` prop', () => { + it('using an object type for the `initialSelectedItem` prop', async () => { render( ); + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label); }); - it('using a string type for the `initialSelectedItem` prop', () => { + it('using a string type for the `initialSelectedItem` prop', async () => { // Replace the 'items' property in mockProps with a list of strings mockProps = { ...mockProps, @@ -142,21 +149,22 @@ describe('FluidComboBox', () => { initialSelectedItem={mockProps.items[1]} /> ); + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[1]); }); }); describe('should display selected item found in `selectedItem`', () => { - it('using an object type for the `selectedItem` prop', () => { + it('using an object type for the `selectedItem` prop', async () => { render( ); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label); }); - it('using a string type for the `selectedItem` prop', () => { + it('using a string type for the `selectedItem` prop', async () => { // Replace the 'items' property in mockProps with a list of strings mockProps = { ...mockProps, @@ -166,7 +174,7 @@ describe('FluidComboBox', () => { render( ); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[1]); }); }); @@ -174,7 +182,7 @@ describe('FluidComboBox', () => { describe('when disabled', () => { it('should not let the user edit the input node', async () => { render(); - + await waitForPosition(); expect(findInputNode()).toHaveAttribute('disabled'); expect(findInputNode()).toHaveDisplayValue(''); @@ -186,15 +194,16 @@ describe('FluidComboBox', () => { it('should not let the user expand the menu', async () => { render(); + await waitForPosition(); await openMenu(); expect(findListBoxNode()).not.toHaveClass('cds--list-box--expanded'); }); }); describe('downshift quirks', () => { - it('should set `inputValue` to an empty string if a false-y value is given', () => { + it('should set `inputValue` to an empty string if a false-y value is given', async () => { render(); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(''); }); @@ -209,6 +218,7 @@ describe('FluidComboBox', () => {
); + await waitForPosition(); const firstFluidComboBox = screen.getByTestId('fluidcombobox-1'); const secondFluidComboBox = screen.getByTestId('fluidcombobox-2'); diff --git a/packages/react/src/components/ListBox/test-helpers.js b/packages/react/src/components/ListBox/test-helpers.js index 5d2a72ef6b12..f9958e7f3e32 100644 --- a/packages/react/src/components/ListBox/test-helpers.js +++ b/packages/react/src/components/ListBox/test-helpers.js @@ -7,6 +7,7 @@ const prefix = 'cds'; import userEvent from '@testing-library/user-event'; +import { act } from '@testing-library/react'; // Finding nodes in a ListBox export const findListBoxNode = () => { @@ -105,3 +106,5 @@ export const generateItems = (amount, generator) => .map((_, i) => generator(i)); export const customItemToString = ({ field }) => field; + +export const waitForPosition = () => act(async () => {}); // Flush microtasks. Position state is ready by this line.