Skip to content

feat: add activeOptionFilter prop and update combobox examples #1156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ export default () => (
| direction | direction of dropdown | 'ltr' \| 'rtl' | 'ltr' |
| optionRender | Custom rendering options | (oriOption: FlattenOptionData\<BaseOptionType\> , info: { index: number }) => React.ReactNode | - |
| labelRender | Custom rendering label | (props: LabelInValueType) => React.ReactNode | - |
| activeOptionFilter | Custom filter function for active option when searching. | (searchValue: string, option: OptionType) => boolean | - |
| maxCount | The max number of items can be selected | number | - |

### Methods
Expand Down
34 changes: 34 additions & 0 deletions docs/examples/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,40 @@ class Combobox extends React.Component {
options={this.state.options}
onChange={this.onAsyncChange}
/>

<h3>Active Option Filter - Middle Match</h3>
<Select
style={{ width: 500 }}
showSearch
allowClear
mode="combobox"
placeholder="Search value can be matched anywhere in the option's value. Try input 'ran'"
activeOptionFilter={(searchValue, option) => {
return String(option.value).includes(searchValue);
}}
>
{['apple', 'banana', 'orange', 'grape'].map((i) => (
<Option value={i} key={i}>
{i}
</Option>
))}
</Select>

<h3>No Active Highlight</h3>
<Select
style={{ width: 500 }}
showSearch
allowClear
mode="combobox"
placeholder="No option will be actively highlighted."
activeOptionFilter={() => false}
>
{['apple', 'banana', 'orange', 'grape'].map((i) => (
<Option value={i} key={i}>
{i}
</Option>
))}
</Select>
</div>
</div>
);
Expand Down
10 changes: 7 additions & 3 deletions src/OptionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r
listHeight,
listItemHeight,
optionRender,
activeOptionFilter,
classNames: contextClassNames,
styles: contextStyles,
} = React.useContext(SelectContext);
Expand Down Expand Up @@ -159,9 +160,12 @@ const OptionList: React.ForwardRefRenderFunction<RefOptionListProps, {}> = (_, r
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,
);
const index = memoFlattenOptions.findIndex(({ data }) => {
if (activeOptionFilter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感觉有点怪异,如果是 value 被选中了,展开不应该是用当前这个逻辑。否则会导致选中的是一个,展开激活的却是另一个。它应该是 default active 的逻辑

return activeOptionFilter(searchValue, data);
}
return searchValue ? String(data.value).startsWith(searchValue) : data.value === value;
});

Comment on lines 160 to 169
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

主动过滤逻辑对分组项存在空指针风险

findIndex 的回调中直接使用 data.value,然而当当前项为分组节点时 data 可能为 undefined,会导致运行时抛出 TypeError: Cannot read properties of undefined (reading 'value')。同时把 activeOptionFilter 应用到分组节点也没有意义。

建议在回调里先排除分组节点或缺少 data 的情况:

-      const index = memoFlattenOptions.findIndex(({ data }) => {
-        if (activeOptionFilter) {
-          return activeOptionFilter(searchValue, data);
-        }
-        return searchValue ? String(data.value).startsWith(searchValue) : data.value === value;
-      });
+      const index = memoFlattenOptions.findIndex(({ group, data }) => {
+        // 忽略分组标题或异常节点
+        if (group || !data) {
+          return false;
+        }
+
+        if (activeOptionFilter) {
+          return activeOptionFilter(searchValue, data);
+        }
+        return searchValue
+          ? String(data.value).startsWith(searchValue)
+          : data.value === value;
+      });

这样可以避免空指针,同时保持逻辑的一致性。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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,
);
const index = memoFlattenOptions.findIndex(({ data }) => {
if (activeOptionFilter) {
return activeOptionFilter(searchValue, data);
}
return searchValue ? String(data.value).startsWith(searchValue) : data.value === value;
});
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 }) => {
- if (activeOptionFilter) {
- return activeOptionFilter(searchValue, data);
- }
- return searchValue ? String(data.value).startsWith(searchValue) : data.value === value;
- });
+ const index = memoFlattenOptions.findIndex(({ group, data }) => {
+ // 忽略分组标题或异常节点
+ if (group || !data) {
+ return false;
+ }
+
+ if (activeOptionFilter) {
+ return activeOptionFilter(searchValue, data);
+ }
+ return searchValue
+ ? String(data.value).startsWith(searchValue)
+ : data.value === value;
+ });
🤖 Prompt for AI Agents
In src/OptionList.tsx around lines 160 to 169, the findIndex callback accesses
data.value without checking if data exists, causing a TypeError when the item is
a group node with undefined data. To fix this, add a guard clause in the
callback to skip items where data is undefined or the item is a group node
before accessing data.value or applying activeOptionFilter, ensuring no null
pointer errors and preserving the filtering logic.

if (index !== -1) {
setActive(index);
Expand Down
4 changes: 4 additions & 0 deletions src/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ export interface SelectProps<ValueType = any, OptionType extends BaseOptionType
listHeight?: number;
listItemHeight?: number;
labelRender?: (props: LabelInValueType) => React.ReactNode;
activeOptionFilter?: (searchValue: string, option: OptionType) => boolean;

// >>> Icon
menuItemSelectedIcon?: RenderNode;
Expand Down Expand Up @@ -213,6 +214,7 @@ const Select = React.forwardRef<BaseSelectRef, SelectProps<any, DefaultOptionTyp
maxCount,
classNames,
styles,
activeOptionFilter,
...restProps
} = props;

Expand Down Expand Up @@ -647,6 +649,7 @@ const Select = React.forwardRef<BaseSelectRef, SelectProps<any, DefaultOptionTyp
optionRender,
classNames,
styles,
activeOptionFilter,
};
}, [
maxCount,
Expand All @@ -667,6 +670,7 @@ const Select = React.forwardRef<BaseSelectRef, SelectProps<any, DefaultOptionTyp
optionRender,
classNames,
styles,
activeOptionFilter,
]);

// ========================== Warning ===========================
Expand Down
1 change: 1 addition & 0 deletions src/SelectContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface SelectContextProps {
listItemHeight?: number;
childrenAsData?: boolean;
maxCount?: number;
activeOptionFilter?: SelectProps['activeOptionFilter'];
}

const SelectContext = React.createContext<SelectContextProps>(null);
Expand Down
47 changes: 47 additions & 0 deletions tests/Combobox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,53 @@ describe('Select.Combobox', () => {
expect(container.querySelector('input').value).toEqual('1');
});

it('activeOptionFilter should work', async () => {
const { container } = render(
<Select
mode="combobox"
showSearch
activeOptionFilter={(searchValue, option) => {
return String(option.value).includes(searchValue);
}}
>
<Option value="apple">apple</Option>
<Option value="banana">banana</Option>
<Option value="orange">orange</Option>
</Select>,
);

Comment on lines +120 to +134
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

建议使用 waitForfindBy... 替换自定义 delay 以消除非必要的 setTimeout

delay() 只是为了等下一个事件循环,但这会引入真实 setTimeout,在 CI 上容易出现偶发现象。
采用 Testing-Library 自带的 waitFor(或直接使用 findByRole / findByText)能在内部自动处理异步刷新,更符合该库的最佳实践,也无需显式 await delay()

- fireEvent.change(input, { target: { value: 'an' } });
- await delay();
+ fireEvent.change(input, { target: { value: 'an' } });
+ await waitFor(() =>
+   expect(
+     container.querySelector('.rc-select-item-option-active .rc-select-item-option-content'),
+   ).toHaveTextContent('banana'),
+ );

同理可删除自定义 delay 函数。
这样做能减少对实现细节的耦合,并提升测试稳定性。

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('activeOptionFilter should work', async () => {
const { container } = render(
<Select
mode="combobox"
showSearch
activeOptionFilter={(searchValue, option) => {
return String(option.value).includes(searchValue);
}}
>
<Option value="apple">apple</Option>
<Option value="banana">banana</Option>
<Option value="orange">orange</Option>
</Select>,
);
it('activeOptionFilter should work', async () => {
const { container } = render(
<Select
mode="combobox"
showSearch
activeOptionFilter={(searchValue, option) => {
return String(option.value).includes(searchValue);
}}
>
<Option value="apple">apple</Option>
<Option value="banana">banana</Option>
<Option value="orange">orange</Option>
</Select>,
);
const input = container.querySelector('input')!;
fireEvent.change(input, { target: { value: 'an' } });
await waitFor(() =>
expect(
container.querySelector('.rc-select-item-option-active .rc-select-item-option-content'),
).toHaveTextContent('banana'),
);
});
🤖 Prompt for AI Agents
In tests/Combobox.test.tsx around lines 120 to 134, replace any custom delay or
setTimeout calls used to wait for asynchronous updates with Testing Library's
built-in waitFor or findBy... queries. Remove the custom delay function
entirely. This change will make the tests more stable and aligned with best
practices by relying on Testing Library's internal async handling instead of
manual timing.

const input = container.querySelector('input')!;
fireEvent.change(input, { target: { value: 'an' } });
await delay();

expect(
container.querySelector('.rc-select-item-option-active .rc-select-item-option-content'),
).toHaveTextContent('banana');

fireEvent.change(input, { target: { value: 'ora' } });
await delay();

expect(
container.querySelector('.rc-select-item-option-active .rc-select-item-option-content'),
).toHaveTextContent('orange');
});

it('activeOptionFilter with empty function should not activate any option', async () => {
const { container } = render(
<Select mode="combobox" showSearch activeOptionFilter={() => false}>
<Option value="apple">apple</Option>
<Option value="banana">banana</Option>
<Option value="orange">orange</Option>
</Select>,
);

const input = container.querySelector('input')!;
fireEvent.change(input, { target: { value: 'an' } });
await delay();

expect(container.querySelector('.rc-select-item-option-active')).toBeNull();
});

describe('input value', () => {
const createSelect = (props?: Partial<SelectProps>) => (
<Select mode="combobox" {...props}>
Expand Down
Loading