From a3880bbe598673538618d27a7192416e84981fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9D=91=BE=F0=9D=92=96=F0=9D=92=99=F0=9D=92=89?= Date: Mon, 26 May 2025 13:32:02 +0800 Subject: [PATCH 1/3] fix: import RawValueType in Select component for type consistency --- src/Select.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Select.tsx b/src/Select.tsx index 9ead1bf40..5b9c7e756 100644 --- a/src/Select.tsx +++ b/src/Select.tsx @@ -51,7 +51,7 @@ import useFilterOptions from './hooks/useFilterOptions'; import useId from './hooks/useId'; import useOptions from './hooks/useOptions'; import useRefFunc from './hooks/useRefFunc'; -import type { FlattenOptionData } from './interface'; +import type { FlattenOptionData, RawValueType } from './interface'; import { hasValue, isComboNoValue, toArray } from './utils/commonUtil'; import { fillFieldNames, flattenOptions, injectPropsWithOption } from './utils/valueUtil'; import warningProps, { warningNullOptions } from './utils/warningPropsUtil'; @@ -66,7 +66,6 @@ export type OnActiveValue = ( export type OnInternalSelect = (value: RawValueType, info: { selected: boolean }) => void; -export type RawValueType = string | number; export interface LabelInValueType { label: React.ReactNode; value: RawValueType; From f3987c2b8606c5777630d8f44dd76ceba9d2c7d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9D=91=BE=F0=9D=92=96=F0=9D=92=99=F0=9D=92=89?= Date: Mon, 26 May 2025 13:41:38 +0800 Subject: [PATCH 2/3] fix: add onActiveValue prop and improve accessibility handling in Select component --- src/Select.tsx | 36 +++++++++++++++++++----------------- tests/Select.test.tsx | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/Select.tsx b/src/Select.tsx index 5b9c7e756..81c344120 100644 --- a/src/Select.tsx +++ b/src/Select.tsx @@ -55,6 +55,7 @@ import type { FlattenOptionData, RawValueType } from './interface'; import { hasValue, isComboNoValue, toArray } from './utils/commonUtil'; import { fillFieldNames, flattenOptions, injectPropsWithOption } from './utils/valueUtil'; import warningProps, { warningNullOptions } from './utils/warningPropsUtil'; +import { useEvent } from 'rc-util'; const OMIT_DOM_PROPS = ['inputValue']; @@ -129,6 +130,9 @@ export interface SelectProps, OptionType>; onDeselect?: SelectHandler, OptionType>; + // >>> active + onActive?: OnActiveValue; + // >>> Options /** * In Select, `false` means do nothing. @@ -209,6 +213,7 @@ const Select = React.forwardRef(null); + const [innerActiveValue, setInnerActiveValue] = React.useState(null); const [accessibilityIndex, setAccessibilityIndex] = React.useState(0); const mergedDefaultActiveFirstOption = defaultActiveFirstOption !== undefined ? defaultActiveFirstOption : mode !== 'combobox'; - const onActiveValue: OnActiveValue = React.useCallback( - (active, index, { source = 'keyboard' } = {}) => { - setAccessibilityIndex(index); - - if (backfill && mode === 'combobox' && active !== null && source === 'keyboard') { - setActiveValue(String(active)); - } - }, - [backfill, mode], - ); + const handleActiveValueChange = useEvent((active, index, info = {}) => { + setAccessibilityIndex(index); + onActive?.(active, index, info); + if (backfill && mode === 'combobox' && active !== null && info.source === 'keyboard') { + setInnerActiveValue(String(active)); + } + }); // ========================= OptionList ========================= const triggerSelect = (val: RawValueType, selected: boolean, type?: DisplayInfoType) => { @@ -547,10 +549,10 @@ const Select = React.forwardRef { setSearchValue(searchText); - setActiveValue(null); + setInnerActiveValue(null); // [Submit] Tag mode should flush input if (info.source === 'submit') { @@ -620,7 +622,7 @@ const Select = React.forwardRef>> Accessibility - activeValue={activeValue} + activeValue={innerActiveValue} activeDescendantId={`${mergedId}_list_${accessibilityIndex}`} /> diff --git a/tests/Select.test.tsx b/tests/Select.test.tsx index f73c5975b..a27c5c479 100644 --- a/tests/Select.test.tsx +++ b/tests/Select.test.tsx @@ -1648,6 +1648,41 @@ describe('Select.Basic', () => { expect(onKeyUp).toHaveBeenCalled(); }); + it('onActive', () => { + const onActive = jest.fn(); + const { getByRole, container } = render( + , + ); + + const input = getByRole('combobox'); + + keyDown(input, KeyCode.DOWN); + keyDown(input, KeyCode.DOWN); + expect(onActive).toHaveBeenCalledTimes(3); + expect(onActive).toHaveBeenLastCalledWith( + 3, + 2, + expect.objectContaining({ + source: 'keyboard', + }), + ); + + fireEvent.mouseMove(container.querySelector('.rc-select-item-option')); + expect(onActive).toHaveBeenCalledTimes(4); + + expect(onActive).toHaveBeenLastCalledWith( + '1', + 0, + expect.objectContaining({ + source: 'mouse', + }), + ); + }); + describe('warning if label not same as option', () => { it('should work', () => { resetWarned(); From e3876eeacc672187ddf4053bc9f5fdda7b125c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9D=91=BE=F0=9D=92=96=F0=9D=92=99=F0=9D=92=89?= Date: Mon, 26 May 2025 17:48:00 +0800 Subject: [PATCH 3/3] chore: improve --- src/OptionList.tsx | 58 ++++++++++++++++++--------------------- tests/OptionList.test.tsx | 1 + 2 files changed, 28 insertions(+), 31 deletions(-) diff --git a/src/OptionList.tsx b/src/OptionList.tsx index a4859b248..ed5960220 100644 --- a/src/OptionList.tsx +++ b/src/OptionList.tsx @@ -15,6 +15,7 @@ import useBaseProps from './hooks/useBaseProps'; import type { FlattenOptionData } from './interface'; import { isPlatformMac } from './utils/platformUtil'; import { isValidCount } from './utils/valueUtil'; +import { useEvent } from 'rc-util'; // export interface OptionListProps { export type OptionListProps = Record; @@ -128,11 +129,6 @@ const OptionList: React.ForwardRefRenderFunction = (_, r onActiveValue(flattenItem.value, index, info); }; - // Auto active first item when list length or searchValue changed - useEffect(() => { - setActive(defaultActiveFirstOption !== false ? getEnabledActiveIndex(0) : -1); - }, [memoFlattenOptions.length, searchValue]); - // https://github.com/ant-design/ant-design/issues/48036 const isAriaSelected = React.useCallback( (value: RawValueType) => { @@ -144,35 +140,35 @@ const OptionList: React.ForwardRefRenderFunction = (_, r [mode, searchValue, [...rawValues].toString(), rawValues.size], ); - // Auto scroll to item position in single mode - useEffect(() => { - /** - * React will skip `onChange` when component update. - * `setActive` function will call root accessibility state update which makes re-render. - * So we need to delay to let Input component trigger onChange first. - */ - const timeoutId = setTimeout(() => { - if (!multiple && open && rawValues.size === 1) { - const value: RawValueType = Array.from(rawValues)[0]; - // Scroll to the option closest to the searchValue if searching. - const index = memoFlattenOptions.findIndex(({ data }) => - searchValue ? String(data.value).startsWith(searchValue) : data.value === value, - ); - - if (index !== -1) { - setActive(index); - scrollIntoView(index); - } - } - }); + const handleActive = useEvent(() => { + if (!open) return; + + let index = -1; + + // Auto scroll to item position in single mode + if (!multiple && rawValues.size === 1) { + const value: RawValueType = Array.from(rawValues)[0]; + // Scroll to the option closest to the searchValue if searching. + index = memoFlattenOptions.findIndex(({ data }) => + searchValue ? String(data.value).startsWith(searchValue) : data.value === value, + ); + } - // Force trigger scrollbar visible when open - if (open) { - listRef.current?.scrollTo(undefined); + if (index === -1 && defaultActiveFirstOption !== false) { + // If no value found, scroll to the first enabled item + index = getEnabledActiveIndex(0); } - return () => clearTimeout(timeoutId); - }, [open, searchValue]); + setActive(index); + + if (index !== -1) { + scrollIntoView(index); + } + + listRef.current?.scrollTo(undefined); + }); + + useEffect(handleActive, [open, searchValue, memoFlattenOptions.length]); // ========================== Values ========================== const onSelectValue = (value: RawValueType) => { diff --git a/tests/OptionList.test.tsx b/tests/OptionList.test.tsx index ba7186a08..3a4abd95a 100644 --- a/tests/OptionList.test.tsx +++ b/tests/OptionList.test.tsx @@ -111,6 +111,7 @@ describe('OptionList', () => { render( generateList({ + open: true, options: [{ value: '1' }, { value: '2' }], values: new Set('1'), onActiveValue,