Conversation
Ensures the -content-has-value className is only added when displayValue has a truthy label property, preventing the class from being added when value is empty or null. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. 概述修复了 SingleContent.tsx 中的条件类名逻辑,将 "-content-has-value" 修饰符的应用条件从 变更
预估代码审查工作量🎯 2 (Simple) | ⏱️ ~12 分钟 可能相关的 PR
建议审查人
诗
✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1207 +/- ##
=======================================
Coverage 99.43% 99.43%
=======================================
Files 31 31
Lines 1230 1230
Branches 419 441 +22
=======================================
Hits 1223 1223
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello @zombieJ, 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! This pull request resolves a styling bug within the 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
|
There was a problem hiding this comment.
Code Review
This PR aims to fix the issue where the Select component's style incorrectly displays as having a value when its value is an empty string. While the changes in SingleContent.tsx address this, they introduce a new bug where the numeric value 0 is also incorrectly treated as empty. I've provided a suggestion for this. Please also add a test case for value={0} to prevent similar regressions.
| className={clsx( | ||
| `${prefixCls}-content`, | ||
| displayValue && `${prefixCls}-content-has-value`, | ||
| displayValue && displayValue.label && `${prefixCls}-content-has-value`, |
There was a problem hiding this comment.
目前的条件 displayValue && displayValue.label 会错误地将 0 视为假值,这意味着当 Select 的值为 0 时,.rc-select-content-has-value 这个 class 不会被应用,可能导致样式不正确。
一个更完善的条件应该能确保 0 被视为有效标签,同时正确处理空字符串的情况。
| displayValue && displayValue.label && `${prefixCls}-content-has-value`, | |
| displayValue && (displayValue.label || displayValue.label === 0) && `${prefixCls}-content-has-value`, |
| it('should not add -content-has-value className when value is empty string', () => { | ||
| const { container } = render( | ||
| <Select value=""> | ||
| <Option value="1">One</Option> | ||
| </Select>, | ||
| ); | ||
| expect(container.querySelector('.rc-select-content-has-value')).toBeFalsy(); | ||
| }); |
There was a problem hiding this comment.
针对空字符串值的测试用例很好。为了防止回归并确保数值 0 被正确处理,建议也为 value={0} 添加一个测试用例。SingleContent.tsx 中当前的修复存在一个 bug,即 0 会被视为一个空值。在修复该问题后,添加如下测试用例会很有价值:
it('should add -content-has-value className when value is 0', () => {
const { container } = render(
<Select value={0}>
<Option value={1}>One</Option>
</Select>,
);
expect(container.querySelector('.rc-select-content-has-value')).toBeTruthy();
});
fix ant-design/ant-design#56973
Summary by CodeRabbit
Bug Fixes