feat: enhance allowClear prop to support disabled#160
feat: enhance allowClear prop to support disabled#160guoyunhe wants to merge 2 commits intoreact-component:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在增强输入组件的 allowClear 属性，使其能够通过一个内嵌的 disabled 属性来控制清除图标的禁用状态。这一改进简化了在某些场景下（例如在 ConfigProvider 中统一配置清除图标但又需要局部禁用清除功能时）的逻辑处理，提高了组件的灵活性和易用性。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
总体说明PR 修改了输入组件中清除按钮的行为：在 allowClear 配置中新增可选的 Walkthrough将 clear 按钮的可见性条件从仅检查 Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 分钟 Possibly related PRs
Suggested reviewers
诗歌
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/BaseInput.tsx`:
- Around line 77-81: The current needClear calculation uses a truthy check on
value which treats 0 and 0n as empty; change the condition in BaseInput (the
needClear boolean calculation that references disabled, readOnly, value, and
allowClear) to check presence explicitly (e.g., value != null or value !==
undefined && value !== null) so numeric zero values count as "has value", and
extract this presence test into a small helper (e.g., isValuePresent or
hasValue) and reuse that helper at the other usage around Line 118 so both the
clear-button rendering and the style/state logic use the identical presence
check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ec2e8e8-86e6-4d54-bc12-a3a7b598a8b0
📒 Files selected for processing (2)
src/BaseInput.tsxsrc/interface.ts
| !disabled && | ||
| !readOnly && | ||
| value && | ||
| !(typeof allowClear === 'object' && allowClear.disabled); |
There was a problem hiding this comment.
外层的 if (allowClear) 已经确保不为 null 了，因此这里就没有再判断了
| suffix?: CSSProperties; | ||
| }; | ||
| allowClear?: boolean | { clearIcon?: ReactNode }; | ||
| allowClear?: boolean | { disabled?: boolean; clearIcon?: ReactNode }; |
There was a problem hiding this comment.
我意思是 antd 里是否有 config 里带有 enabled or disabled 的属性可以参考一下。免得未来这个名字出现多种情况
There was a problem hiding this comment.
搜索了一下，大部分使用的是 disabled，比如 useClosable：
interface ClosableCollection {
closable?: ClosableType;
closeIcon?: ReactNode;
disabled?: boolean;
}比如 WaveConfig：
export interface WaveConfig {
disabled?: boolean;
showEffect?: ShowWaveEffect;
}而 enabled 只有一处使用：
export interface MaskConfig {
enabled?: boolean;
blur?: boolean;
closable?: boolean;
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #160 +/- ##
==========================================
+ Coverage 97.53% 97.57% +0.03%
==========================================
Files 4 4
Lines 203 206 +3
Branches 75 80 +5
==========================================
+ Hits 198 201 +3
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
在一些情况下，allowClear 和 clearIcon 分别由不同的变量控制。
以前的写法（略显杂乱）：
现在的写法（优雅一些）：
尤其是在 ConfigProvider 里，我们更常需要替换 clearIcon 而不改变 allowClear 的开关状态。因此增加一个单独的 disabled 属性，有利于我们更灵活处理多种情况。
Summary by CodeRabbit
新功能
变更