From b8dcd0b17e95326f6ed788c24d7ed7f761ce2d5d Mon Sep 17 00:00:00 2001 From: Gururajj77 Date: Tue, 28 May 2024 15:27:14 +0530 Subject: [PATCH 1/9] feat: changes --- .../components/ComboBox/ComboBox.stories.js | 15 +++++++++++ .../src/components/ComboBox/ComboBox.tsx | 26 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) 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 a6c85a6323c9..2fc9cab12010 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, @@ -342,6 +343,27 @@ const ComboBox = forwardRef( slug, ...rest } = props; + const { refs, floatingStyles } = useFloating({ + strategy: 'fixed', + middleware: [ + flip({ + fallbackAxisSideDirection: 'none', + }), + ], + whileElementsMounted: autoUpdate, + }); + const parentWidth = (refs?.reference?.current as HTMLElement)?.clientWidth; + + useEffect(() => { + 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'; + } + }, [floatingStyles, refs.floating, parentWidth]); const prefix = usePrefix(); const { isFluid } = useContext(FormContext); const textInput = useRef(null); @@ -630,6 +652,7 @@ const ComboBox = forwardRef( light={light} size={size} warn={warn} + ref={refs.setReference} warnText={warnText} warnTextId={warnTextId}>
@@ -739,7 +762,8 @@ const ComboBox = forwardRef( + })} + ref={mergeRefs(getMenuProps().ref, refs.setFloating)}> {isOpen ? filterItems(items, itemToString, inputValue).map( (item, index) => { From 1d0200b3651c5cca604d6ebde3b9eee6cf27dd33 Mon Sep 17 00:00:00 2001 From: Gururajj77 Date: Tue, 28 May 2024 15:36:22 +0530 Subject: [PATCH 2/9] feat: added floating-ui for combobox --- .../src/components/ComboBox/ComboBox.tsx | 48 ++++++++++++------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index 2fc9cab12010..fa4b319a90b4 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -151,6 +151,11 @@ export interface ComboBoxProps */ ariaLabel?: string; + /** + * Will auto-align the dropdown on first render if it is not visible. This prop is currently experimental and is subject to future changes. + */ + autoAlign?: boolean; + /** * An optional className to add to the container node */ @@ -314,6 +319,7 @@ const ComboBox = forwardRef( const { ['aria-label']: ariaLabel = 'Choose an item', ariaLabel: deprecatedAriaLabel, + autoAlign = false, className: containerClassName, direction = 'bottom', disabled = false, @@ -343,27 +349,33 @@ const ComboBox = forwardRef( slug, ...rest } = props; - const { refs, floatingStyles } = useFloating({ - strategy: 'fixed', - middleware: [ - flip({ - fallbackAxisSideDirection: 'none', - }), - ], - whileElementsMounted: autoUpdate, - }); + const { refs, floatingStyles } = useFloating( + autoAlign + ? { + strategy: 'fixed', + middleware: [ + flip({ + fallbackAxisSideDirection: 'none', + }), + ], + whileElementsMounted: autoUpdate, + } + : {} + ); const parentWidth = (refs?.reference?.current as HTMLElement)?.clientWidth; useEffect(() => { - Object.keys(floatingStyles).forEach((style) => { - if (refs.floating.current) { - refs.floating.current.style[style] = floatingStyles[style]; + 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'; } - }); - if (parentWidth && refs.floating.current) { - refs.floating.current.style.width = parentWidth + 'px'; } - }, [floatingStyles, refs.floating, parentWidth]); + }, [autoAlign, floatingStyles, refs.floating, parentWidth]); const prefix = usePrefix(); const { isFluid } = useContext(FormContext); const textInput = useRef(null); @@ -845,6 +857,10 @@ ComboBox.propTypes = { PropTypes.string, 'This prop syntax has been deprecated. Please use the new `aria-label`.' ), + /** + * Will auto-align the dropdown on first render if it is not visible. This prop is currently experimental and is subject to future changes. + */ + autoAlign: PropTypes.bool, /** * An optional className to add to the container node From 17846894d644c2be5c85be3d5cfd9f1bd93c1e35 Mon Sep 17 00:00:00 2001 From: Gururajj77 Date: Wed, 5 Jun 2024 15:33:47 +0530 Subject: [PATCH 3/9] feat: added test helper function to pass tests --- .../src/components/ComboBox/ComboBox-test.js | 33 ++++++++++------- .../__tests__/FluidComboBox-test.js | 36 ++++++++++++------- .../src/components/ListBox/test-helpers.js | 3 ++ 3 files changed, 46 insertions(+), 26 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox-test.js b/packages/react/src/components/ComboBox/ComboBox-test.js index e98ff05d2c09..4cf6f3a5d689 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-dom/test-utils'; @@ -143,23 +144,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, @@ -169,19 +171,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, @@ -189,7 +191,7 @@ describe('ComboBox', () => { }; render(); - + await waitForPosition(); expect(findInputNode()).toHaveDisplayValue(mockProps.items[1]); }); }); @@ -197,7 +199,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(''); @@ -209,6 +211,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` @@ -219,7 +222,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(''); @@ -231,6 +234,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` @@ -239,9 +243,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(''); }); @@ -256,6 +260,7 @@ describe('ComboBox', () => {
); + await waitForPosition(); const firstCombobox = screen.getByTestId('combobox-1'); const secondCombobox = screen.getByTestId('combobox-2'); @@ -290,6 +295,7 @@ describe('ComboBox', () => { }); it('should open menu without moving focus on pressing Alt+ DownArrow', async () => { render(); + await waitForPosition(); act(() => { screen.getByRole('combobox').focus(); }); @@ -299,6 +305,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/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. From bc2e9a85b043936eb887d0e314df7f643362b82d Mon Sep 17 00:00:00 2001 From: Gururajj77 Date: Wed, 5 Jun 2024 15:59:47 +0530 Subject: [PATCH 4/9] feat: test changes --- packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap index 3fb61718ecb1..d4b6ea7483c6 100644 --- a/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap +++ b/packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap @@ -1103,6 +1103,9 @@ Map { "type": "string", }, "ariaLabel": [Function], + "autoAlign": Object { + "type": "bool", + }, "className": Object { "type": "string", }, From 94ae3ddffb12879c084fa48d59a9bb0919e3300f Mon Sep 17 00:00:00 2001 From: Gururaj J <89023023+Gururajj77@users.noreply.github.com> Date: Thu, 6 Jun 2024 19:13:19 +0530 Subject: [PATCH 5/9] Update packages/react/src/components/ComboBox/ComboBox.tsx update comments for interface Co-authored-by: Taylor Jones --- packages/react/src/components/ComboBox/ComboBox.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index fa4b319a90b4..896e65ec982a 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -152,7 +152,9 @@ export interface ComboBoxProps ariaLabel?: string; /** - * Will auto-align the dropdown on first render if it is not visible. This prop is currently experimental and is subject to future changes. + * **Experimental**: Will attempt to automatically align the floating + * element to avoid collisions with the viewport and being clipped by + * ancestor elements. */ autoAlign?: boolean; From 60bd094063d1f30ea523cbfd3b8a5040b48800ce Mon Sep 17 00:00:00 2001 From: Gururaj J <89023023+Gururajj77@users.noreply.github.com> Date: Thu, 6 Jun 2024 19:13:33 +0530 Subject: [PATCH 6/9] Update packages/react/src/components/ComboBox/ComboBox.tsx update comment for prop-types Co-authored-by: Taylor Jones --- packages/react/src/components/ComboBox/ComboBox.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index 896e65ec982a..1ee3583fa4c3 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -860,7 +860,9 @@ ComboBox.propTypes = { 'This prop syntax has been deprecated. Please use the new `aria-label`.' ), /** - * Will auto-align the dropdown on first render if it is not visible. This prop is currently experimental and is subject to future changes. + * **Experimental**: Will attempt to automatically align the floating + * element to avoid collisions with the viewport and being clipped by + * ancestor elements. */ autoAlign: PropTypes.bool, From f395bbc492db08d63218923fa0bc4686545bab56 Mon Sep 17 00:00:00 2001 From: Gururaj J <89023023+Gururajj77@users.noreply.github.com> Date: Thu, 6 Jun 2024 19:14:07 +0530 Subject: [PATCH 7/9] Update packages/react/src/components/ComboBox/ComboBox.tsx removing `fallbackAxisSideDirection` and keeping it flip() Co-authored-by: Taylor Jones --- packages/react/src/components/ComboBox/ComboBox.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index 1ee3583fa4c3..efee5cdceb81 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -356,9 +356,7 @@ const ComboBox = forwardRef( ? { strategy: 'fixed', middleware: [ - flip({ - fallbackAxisSideDirection: 'none', - }), + flip(), ], whileElementsMounted: autoUpdate, } From c0ffda418d6e4db804c1841d8773e926330eabac Mon Sep 17 00:00:00 2001 From: Gururaj J <89023023+Gururajj77@users.noreply.github.com> Date: Thu, 6 Jun 2024 19:15:39 +0530 Subject: [PATCH 8/9] Update packages/react/src/components/ComboBox/ComboBox.tsx adding placement as we already have direction prop coming in Co-authored-by: Taylor Jones --- packages/react/src/components/ComboBox/ComboBox.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index efee5cdceb81..a09fee5b4f24 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -354,6 +354,7 @@ const ComboBox = forwardRef( const { refs, floatingStyles } = useFloating( autoAlign ? { + placement: direction, strategy: 'fixed', middleware: [ flip(), From 6c8ba5cce30b8df2d28b3153b4165d5a7c1a97cd Mon Sep 17 00:00:00 2001 From: Gururajj77 Date: Thu, 6 Jun 2024 19:46:56 +0530 Subject: [PATCH 9/9] feat: yarn formatted --- packages/react/src/components/ComboBox/ComboBox.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx index a09fee5b4f24..05fa7061bc22 100644 --- a/packages/react/src/components/ComboBox/ComboBox.tsx +++ b/packages/react/src/components/ComboBox/ComboBox.tsx @@ -356,9 +356,7 @@ const ComboBox = forwardRef( ? { placement: direction, strategy: 'fixed', - middleware: [ - flip(), - ], + middleware: [flip()], whileElementsMounted: autoUpdate, } : {}