diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c5be89b9..5acd21c400 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +### [144.9.6](https://github.com/Sage/carbon/compare/v144.9.5...v144.9.6) (2024-11-28) + + +### Bug Fixes + +* **select-list:** selected option not scrolled into view ([d842293](https://github.com/Sage/carbon/commit/d8422931196b07d5550899421a2e33842378f340)) + +### [144.9.5](https://github.com/Sage/carbon/compare/v144.9.4...v144.9.5) (2024-11-27) + + +### Bug Fixes + +* **multi-select:** ensure that when the arrow icon is clicked with openOnFocus select list renders ([cf78fed](https://github.com/Sage/carbon/commit/cf78fed27658b6b9eca97c874d2737b702041c82)), closes [#7065](https://github.com/Sage/carbon/issues/7065) + ### [144.9.4](https://github.com/Sage/carbon/compare/v144.9.3...v144.9.4) (2024-11-26) diff --git a/package-lock.json b/package-lock.json index b275957836..beea5a2861 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "carbon-react", - "version": "144.9.4", + "version": "144.9.6", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "carbon-react", - "version": "144.9.4", + "version": "144.9.6", "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { diff --git a/package.json b/package.json index 809539244b..9a1c72350d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "carbon-react", - "version": "144.9.4", + "version": "144.9.6", "description": "A library of reusable React components for easily building user interfaces.", "files": [ "lib", diff --git a/src/components/select/__internal__/select-list/select-list.component.tsx b/src/components/select/__internal__/select-list/select-list.component.tsx index 5f1dea0c45..6ca2aed6bd 100644 --- a/src/components/select/__internal__/select-list/select-list.component.tsx +++ b/src/components/select/__internal__/select-list/select-list.component.tsx @@ -189,6 +189,12 @@ const SelectList = React.forwardRef( rangeExtractor, }); + useEffect(() => { + if (isOpen && currentOptionsListIndex > -1) { + virtualizer.scrollToIndex(currentOptionsListIndex, SCROLL_OPTIONS); + } + }, [currentOptionsListIndex, isOpen, virtualizer]); + const items = virtualizer.getVirtualItems(); const childrenList = useMemo( diff --git a/src/components/select/multi-select/multi-select.component.tsx b/src/components/select/multi-select/multi-select.component.tsx index 584d1a6090..8b2309f6f5 100644 --- a/src/components/select/multi-select/multi-select.component.tsx +++ b/src/components/select/multi-select/multi-select.component.tsx @@ -149,24 +149,27 @@ export const MultiSelect = React.forwardRef( const accessibilityLabelId = useRef(guid()); const containerRef = useRef(null); const listboxRef = useRef(null); + const isInputFocused = useRef(false); const isClickTriggeredBySelect = useRef(false); const isMouseDownReported = useRef(false); const isMouseDownOnInput = useRef(false); - const [isOpenedByFocus, setIsOpenedByFocus] = useState(false); + const isOpenedByFocus = useRef(false); const isControlled = useRef(value !== undefined); const [textboxRef, setTextboxRef] = useState(); - const [open, setOpen] = useState(false); + const [isOpen, setOpenState] = useState(false); const [textValue, setTextValue] = useState(""); const [selectedValue, setSelectedValue] = useState( value || defaultValue || [], ); const [highlightedValue, setHighlightedValue] = useState(""); + const [filterText, setFilterText] = useState(""); const [placeholderOverride, setPlaceholderOverride] = useState(); const inputId = useRef(id || guid()); const { labelId } = useInputAccessibility({ id: inputId.current, label, }); + const focusTimer = useRef>(null); const actualValue = isControlled.current ? value : selectedValue; @@ -180,6 +183,16 @@ export const MultiSelect = React.forwardRef( ); } + const setOpen = useCallback(() => { + setOpenState((isAlreadyOpen) => { + if (!isAlreadyOpen && onOpen) { + onOpen(); + } + + return true; + }); + }, [onOpen]); + const createCustomEvent = useCallback( (newValue, selectionConfirmed) => { const customEvent = { @@ -246,15 +259,11 @@ export const MultiSelect = React.forwardRef( setHighlightedValue(match.props.value); } + setFilterText(newValue); setTextValue(newValue); - - if (!open) { - onOpen?.(); - } - - setOpen(true); + setOpen(); }, - [children, open, onOpen], + [children, setOpen], ); const removeSelectedValue = useCallback( @@ -279,7 +288,9 @@ export const MultiSelect = React.forwardRef( const { key } = event; const isDeleteKey = key === "Backspace" || key === "Delete"; - onKeyDown?.(event); + if (onKeyDown) { + onKeyDown(event); + } if (readOnly) { return; @@ -287,19 +298,22 @@ export const MultiSelect = React.forwardRef( if (!event.defaultPrevented && isNavigationKey(key)) { event.preventDefault(); - - if (!open) { - onOpen?.(); - } - - setOpen(true); + setOpen(); } - if (isDeleteKey && textValue === "") { + if (isDeleteKey && (filterText === "" || textValue === "")) { removeSelectedValue(-1); } + // eslint-disable-next-line react-hooks/exhaustive-deps }, - [onKeyDown, readOnly, textValue, open, onOpen, removeSelectedValue], + [ + onKeyDown, + readOnly, + filterText, + textValue, + setOpen, + removeSelectedValue, + ], ); const accessibilityLabel = useMemo(() => { @@ -321,7 +335,7 @@ export const MultiSelect = React.forwardRef( (event) => { isMouseDownReported.current = false; - if (!open) { + if (!isOpen) { return; } @@ -333,13 +347,14 @@ export const MultiSelect = React.forwardRef( if (notInContainer && notInList && !isClickTriggeredBySelect.current) { setTextValue(""); + setFilterText(""); setHighlightedValue(""); - setOpen(false); + setOpenState(false); } isClickTriggeredBySelect.current = false; }, - [open], + [isOpen], ); const mapValuesToPills = useMemo(() => { @@ -439,14 +454,12 @@ export const MultiSelect = React.forwardRef( const onFilterChange = useStableCallback( onFilterChangeProp as (filterTextArg: unknown) => void, ); - const isFirstRender = useRef(true); - useEffect(() => { - if (!isFirstRender.current) { - onFilterChange?.(textValue); + if (onFilterChange && !isFirstRender.current) { + onFilterChange(filterText); } - }, [onFilterChange, textValue]); + }, [onFilterChange, filterText]); useEffect(() => { isFirstRender.current = false; @@ -455,22 +468,51 @@ export const MultiSelect = React.forwardRef( function handleTextboxClick(event: React.MouseEvent) { isMouseDownReported.current = false; - onClick?.(event); - - if (openOnFocus && isOpenedByFocus) { - setIsOpenedByFocus(false); - return; + if (onClick) { + onClick(event); } - if (open) { - setTextValue(""); - setOpen(false); - } else { + /* istanbul ignore else */ + if (!openOnFocus || (openOnFocus && !isOpenedByFocus.current)) { + if (isOpen) { + setFilterText(""); + setOpenState(false); + return; + } + onOpen?.(); - setOpen(true); + + setOpenState(true); + } else { + // This is tested in Playwright. + // This line of code is triggered when a user initially clicks on the input when `openOnFocus` is true. + isOpenedByFocus.current = false; } } + function handleDropdownIconClick( + event: React.MouseEvent | React.KeyboardEvent, + ) { + isMouseDownReported.current = false; + + if (onClick) { + onClick(event as React.MouseEvent); + } + + setOpenState((isAlreadyOpen) => { + if (isAlreadyOpen) { + setFilterText(""); + return false; + } + + if (onOpen) { + onOpen(); + } + + return true; + }); + } + function handleTextboxBlur(event: React.FocusEvent) { isMouseDownOnInput.current = false; @@ -478,15 +520,19 @@ export const MultiSelect = React.forwardRef( return; } - onBlur?.(event); + isInputFocused.current = false; - setTextValue(""); - setOpen(false); + if (onBlur) { + onBlur(event); + } } - function handleTextboxMouseDown() { + function handleTextboxMouseDown(event: React.MouseEvent) { isMouseDownReported.current = true; - isMouseDownOnInput.current = true; + + if ((event.target as HTMLInputElement).dataset.element === "input") { + isMouseDownOnInput.current = true; + } } function handleListMouseDown() { @@ -494,17 +540,43 @@ export const MultiSelect = React.forwardRef( } function handleTextboxFocus(event: React.FocusEvent) { - onFocus?.(event); + const triggerFocus = () => onFocus?.(event); if (openOnFocus) { - if (open) return; - - onOpen?.(); - - if (isMouseDownOnInput.current) { - setIsOpenedByFocus(true); + if (focusTimer.current) { + clearTimeout(focusTimer.current); } - setOpen(true); + + // we need to use a timeout here as there is a race condition when rendered in a modal + // whereby the select list isn't visible when the select is auto focused straight away + focusTimer.current = setTimeout(() => { + setOpenState((isAlreadyOpen) => { + if (isAlreadyOpen) { + return true; + } + + if (onOpen) { + onOpen(); + } + if (onFocus && !isInputFocused.current) { + triggerFocus(); + isInputFocused.current = true; + } + + if (isMouseDownReported.current && !isMouseDownOnInput.current) { + isOpenedByFocus.current = false; + return false; + } + + if (isMouseDownOnInput.current) { + isOpenedByFocus.current = true; + } + return true; + }); + }); + } else if (onFocus && !isInputFocused.current) { + triggerFocus(); + isInputFocused.current = true; } } @@ -548,8 +620,8 @@ export const MultiSelect = React.forwardRef( ); const onSelectListClose = useCallback(() => { - setOpen(false); - setTextValue(""); + setOpenState(false); + setFilterText(""); }, []); const assignInput = useCallback>( @@ -583,6 +655,8 @@ export const MultiSelect = React.forwardRef( onMouseDown: handleTextboxMouseDown, onFocus: handleTextboxFocus, onBlur: handleTextboxBlur, + iconOnClick: handleDropdownIconClick, + iconOnMouseDown: handleTextboxMouseDown, onKeyDown: handleTextboxKeydown, onChange: handleTextboxChange, tooltipPosition, @@ -615,7 +689,7 @@ export const MultiSelect = React.forwardRef( onSelect={onSelectOption} onSelectListClose={onSelectListClose} onMouseDown={handleListMouseDown} - filterText={textValue.trim()} + filterText={filterText.trim()} highlightedValue={highlightedValue} noResultsMessage={noResultsMessage} isLoading={isLoading} @@ -625,7 +699,7 @@ export const MultiSelect = React.forwardRef( listMaxHeight={listMaxHeight} flipEnabled={flipEnabled} multiselectValues={actualValue} - isOpen={open} + isOpen={isOpen} enableVirtualScroll={enableVirtualScroll} virtualScrollOverscan={virtualScrollOverscan} listWidth={listWidth} @@ -645,7 +719,7 @@ export const MultiSelect = React.forwardRef( data-component={dataComponent} data-role={dataRole} data-element={dataElement} - isOpen={open} + isOpen={isOpen} {...marginProps} >
@@ -664,7 +738,7 @@ export const MultiSelect = React.forwardRef( ariaLabel={ariaLabel} ariaLabelledby={ariaLabelledby} hasTextCursor - isOpen={open} + isOpen={isOpen} labelId={labelId} {...getTextboxProps()} /> diff --git a/src/components/select/multi-select/multi-select.pw.tsx b/src/components/select/multi-select/multi-select.pw.tsx index 63027f1cb1..ecceca4e57 100644 --- a/src/components/select/multi-select/multi-select.pw.tsx +++ b/src/components/select/multi-select/multi-select.pw.tsx @@ -351,7 +351,7 @@ test.describe("MultiSelect component", () => { ["left", "start"], ] as [MultiSelectProps["labelAlign"], string][] ).forEach(([alignment, cssProp]) => { - test(`should use ${alignment} as labelAligment and render with flex-${cssProp} as css properties`, async ({ + test(`should use ${alignment} as labelAlignment and render with flex-${cssProp} as css properties`, async ({ mount, page, }) => { diff --git a/src/components/select/multi-select/multi-select.test.tsx b/src/components/select/multi-select/multi-select.test.tsx index 6262873f3e..f3a9618178 100644 --- a/src/components/select/multi-select/multi-select.test.tsx +++ b/src/components/select/multi-select/multi-select.test.tsx @@ -703,7 +703,7 @@ test.each(["Backspace", "Delete"])( }, ); -test("a selected option, that renders dimissible pill, can be removed by clicking the dismiss button", async () => { +test("a selected option, that renders dismissible pill, can be removed by clicking the dismiss button", async () => { const user = userEvent.setup(); const ControlledMultiSelect = () => { const [value, setValue] = React.useState(["amber"]); @@ -725,8 +725,8 @@ test("a selected option, that renders dimissible pill, can be removed by clickin await user.click(screen.getByRole("combobox")); const amberPill = await screen.findByTitle("amber"); - const dimissButton = within(amberPill).getByRole("button"); - await user.click(dimissButton); + const dismissButton = within(amberPill).getByRole("button"); + await user.click(dismissButton); expect(amberPill).not.toBeInTheDocument(); }); @@ -1168,6 +1168,150 @@ test("dropdown list is open on initial render, when autoFocus and openOnFocus pr jest.useRealTimers(); }); +test("should not display the list when `openOnFocus` is set and mousedown is detected", () => { + jest.useFakeTimers(); + render( + {}}> + , + ); + + const icon = within(screen.getByRole("presentation")).getByTestId("icon"); + fireEvent.mouseDown(icon); + + act(() => jest.runOnlyPendingTimers()); + + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + jest.useRealTimers(); +}); + +test("should display the list when `openOnFocus` is not set", async () => { + const user = userEvent.setup(); + render( + {}}> + , + ); + const icon = within(screen.getByRole("presentation")).getByTestId("icon"); + await user.click(icon); + + expect(await screen.findByRole("listbox")).toBeVisible(); +}); + +test("should call `onOpen` callback if prop is passed", async () => { + const onOpenFn = jest.fn(); + const user = userEvent.setup(); + render( + {}}> + , + ); + await user.click(screen.getByRole("combobox")); + + expect(onOpenFn).toHaveBeenCalled(); +}); + +test("should call `onOpen` callback if prop is passed and navigation key opens select list", async () => { + const onOpenFn = jest.fn(); + const user = userEvent.setup(); + render( + {}}> + , + ); + const input = screen.getByRole("combobox"); + input.focus(); + + await user.keyboard("{ArrowDown}"); + + expect(onOpenFn).toHaveBeenCalled(); +}); + +test("should call `onFocus` callback when input is focused and `openOnFocus` is set", () => { + jest.useFakeTimers(); + const onFocusFn = jest.fn(); + render( + {}} + onFocus={onFocusFn} + > + , + ); + screen.getByRole("combobox").focus(); + act(() => jest.runOnlyPendingTimers()); + + expect(onFocusFn).toHaveBeenCalled(); + jest.useRealTimers(); +}); + +test("should close the list when the user clicks on the input icon and the list is already open", async () => { + const user = userEvent.setup(); + const onClickFn = jest.fn(); + render( + {}} onClick={onClickFn}> + , + ); + const icon = within(screen.getByRole("presentation")).getByTestId("icon"); + await user.click(icon); + await user.click(icon); + + expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + expect(onClickFn).toHaveBeenCalledTimes(2); +}); + +// coverage +test("should update the input value when the user presses 'Delete' and the text is highlighted", async () => { + const user = userEvent.setup(); + render( + {}}> + , + ); + const input = screen.getByRole("combobox"); + await user.click(input); + await user.type(input, " "); + (input as HTMLInputElement).setSelectionRange(0, 0); + await user.keyboard("{Delete}"); + + expect(input).toHaveValue(""); +}); + +// coverage +test("should focus the input when mousedown event is fired on input", async () => { + jest.useFakeTimers(); + render( + {}} openOnFocus> + , + ); + const input = screen.getByRole("combobox"); + fireEvent.mouseDown(input); + act(() => jest.runOnlyPendingTimers()); + + expect(input).toHaveFocus(); + jest.useRealTimers(); +}); + +// coverage +test("the SelectList should stay visible if the input has received a mousedown event, then a click event and `openOnFocus` is true", () => { + render( + {}} openOnFocus> + , + ); + + const input = screen.getByRole("combobox"); + fireEvent.mouseDown(input); + fireEvent.focus(input); + fireEvent.click(input); + + expect(screen.getByRole("listbox")).toBeVisible(); +}); + testStyledSystemMargin( (props) => ( { await expect(lastOption).toBeAttached(); }); + test("when reopening the select after selecting an option, the selected option is visible", async ({ + mount, + page, + }) => { + const maxHeight = 200; + await mount(); + + await selectText(page).click(); + await selectListScrollableWrapper(page).evaluate((wrapper) => + wrapper.scrollBy(0, wrapper.scrollHeight), + ); + + await expect(selectOptionByText(page, "Yellow")).toBeInViewport(); + await selectOptionByText(page, "Yellow").click(); + + await expect(selectOptionByText(page, "Yellow")).not.toBeVisible(); + + await selectText(page).click(); + await expect(selectOptionByText(page, "Yellow")).toBeInViewport(); + + await page.locator("body").click(); + + await selectText(page).click(); + await expect(selectOptionByText(page, "Yellow")).toBeInViewport(); + }); + test("a selected option stays rendered even when out of view", async ({ mount, page,