fix: should not create tag when input matches disabled option in tags mode#1227
fix: should not create tag when input matches disabled option in tags mode#1227EmilyyyLiu wants to merge 5 commits into
Conversation
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough在 ChangesTags 模式禁用值过滤
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request prevents the creation of tags for disabled options in tags mode and adds a corresponding test case. The review feedback points out a potential issue where option values that are numbers will not be correctly identified as disabled because the search input is processed as a string. To resolve this, it is recommended to check both the string and numeric representations of the input in the options lookup and to expand the test suite to cover numeric values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1227 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 31 31
Lines 1265 1272 +7
Branches 441 444 +3
=======================================
+ Hits 1258 1265 +7
Misses 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
这个方向可以解决 blur 触发的问题,不过我觉得可以再收敛一下实现边界:这里本质上是 tags 模式在“创建新 tag”时需要避免复用已有 disabled option 的 value,而不只是 submit 分支需要拦截。 现在只在 建议抽一个小判断,例如: const isDisabledOptionValue = React.useCallback(
(val: RawValueType) => !!valueOptions.get(val)?.disabled,
[valueOptions],
);然后同时用于两个地方:
这样规则会更靠近“tag 创建边界”,也能避免同一个 disabled value 通过不同入口被创建出来。测试上除了现在的 blur case,最好再补一个 |
9cd57c4 to
1ea5512
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Select.tsx (1)
468-481: 🎯 Functional Correctness | 🟡 Minor补充
filledSearchOptions的依赖项
filledSearchOptions里调用了isDisabledOptionValue(mergedSearchValue),但依赖数组没包含它;valueOptions变化时,这里可能继续用旧的禁用判断,导致临时 tag 处理结果过期。把它加进去。建议修改
}, [ createTagOption, normalizedOptionFilterProp, mode, filteredOptions, mergedSearchValue, mergedFieldNames, + isDisabledOptionValue, ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Select.tsx` around lines 468 - 481, filledSearchOptions uses isDisabledOptionValue(mergedSearchValue) but the dependency array is missing that callback, so it can keep stale disabled-state logic when valueOptions change. Update the useMemo dependencies around filledSearchOptions in Select.tsx to include isDisabledOptionValue, alongside the existing dependencies, so the temporary tag option is recomputed with the latest disabled check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/Select.tsx`:
- Around line 468-481: filledSearchOptions uses
isDisabledOptionValue(mergedSearchValue) but the dependency array is missing
that callback, so it can keep stale disabled-state logic when valueOptions
change. Update the useMemo dependencies around filledSearchOptions in Select.tsx
to include isDisabledOptionValue, alongside the existing dependencies, so the
temporary tag option is recomputed with the latest disabled check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ccfccbc-6176-4c99-8e4a-27691286105c
📒 Files selected for processing (2)
src/Select.tsxtests/Tags.test.tsx
|
又看了一下更新后的版本,整体方向已经比最初更完整了: 还有两个小点建议再收一下:
valueOptions.get(String(val))?.disabled || valueOptions.get(Number(val))?.disabled这个会有点宽。比如同时存在 更稳一点可以先尊重 exact raw value,再只在没有 exact match 时做字符串化匹配,例如: const isDisabledOptionValue = React.useCallback(
(val: RawValueType) => {
const exactOption = valueOptions.get(val);
if (exactOption) {
return !!exactOption.disabled;
}
return Array.from(valueOptions).some(
([optionValue, option]) => String(optionValue) === String(val) && option.disabled,
);
},
[valueOptions],
);这样可以覆盖输入 |
|
我再想了一下,这里可以更简单一点,不一定要为 numeric value 做额外兼容。tags 模式里用户输入本身是字符串,如果要把 建议先只修 exact value 命中的 disabled option: // filledSearchOptions
if (valueOptions.get(mergedSearchValue)?.disabled) {
return filteredOptions;
}// submit
if (valueOptions.get(formatted)?.disabled) {
setSearchValue("");
return;
}如果保留 tokenSeparators 的处理,也可以同样用 exact match: patchValues = patchValues.filter((val) => !valueOptions.get(val)?.disabled);这样可以去掉 |
关联 Issue:
类型:
背景和解决方案:
在
Select mode="tags"中,当输入一个 disabled option 的值然后失焦(或按回车)时,会自动创建一个 disabled tag,该 tag 无法删除。在 filterOption = {()=>{ false } 的时候创建临时的disabled tag
在 tokenSeparators 配置中会忽略 disabled 选项
解决方案
使用统一的 isDisabledOptionValue 函数在以下三个入口进行检查:
修改内容
Change Log:
Summary by CodeRabbit
Summary by CodeRabbit
onChange不触发且已选区域为空。