feat(Sender): implement maxLength and showCount support #1321
Conversation
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthrough向 Sender 组件添加 maxLength 与 showCount 属性：在组件实现中透传 maxLength 至 Input.TextArea、按需渲染字符计数；新增样式、演示和单元测试以覆盖截断与计数行为；同时微调部分导入与可选链调用；未更改除新增 props 之外的导出签名。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Sender as Sender 组件
participant Textarea as Input.TextArea
participant Counter as 计数显示
User->>Textarea: 输入字符
Textarea->>Sender: 触发 onChange
Sender->>Sender: 更新 innerValue（或使用受控 value）
alt showCount && typeof maxLength === "number"
Sender->>Counter: 计算并渲染 currentLength / maxLength
Counter-->>User: 显示计数
else
Sender-->>User: 不渲染计数
end
Note over Textarea: TextArea 同步遵循传入的 maxLength 做输入截断
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
dosubot
bot
added
enhancement
Summary of ChangesHello @pingtougea, 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 introduces significant enhancements to the Highlights
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully implements maxLength and showCount functionality for the Sender component. The implementation is clean and leverages Ant Design's existing capabilities. The new features are accompanied by a new demo and a comprehensive set of tests. My review focuses on improving the newly added tests to ensure they are accurate and robust. I've identified a few issues, including an unused import, incorrect assertions in the character count tests, and a flawed test for the controlled component's behavior. I've also provided a suggestion to make the maxLength test more reliable by using user-event to better simulate user interaction. Overall, this is a great contribution, and with these test fixes, it will be even better.
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('5 / 20'); | ||
|
|
||
| fireEvent.change(textarea, { target: { value: 'hello world' } }); | ||
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('11 / 20'); |
There was a problem hiding this comment.
The test expectations for the character count's text content are incorrect. The implementation renders the count as {length}/{maxLength} without spaces, but the assertions expect spaces around the slash (e.g., '5 / 20'). This will cause the test to fail. The expected strings should be updated to match the implementation.
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('5 / 20'); | |
| fireEvent.change(textarea, { target: { value: 'hello world' } }); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('11 / 20'); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('5/20'); | |
| fireEvent.change(textarea, { target: { value: 'hello world' } }); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('11/20'); |
| it('should work with controlled component', () => { | ||
| const onChange = jest.fn(); | ||
| const { container, rerender } = render( | ||
| <Sender maxLength={10} showCount value="initial" onChange={onChange} />, | ||
| ); | ||
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('7 / 10'); | ||
|
|
||
| rerender(<Sender maxLength={10} showCount value="update" onChange={onChange} />); | ||
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('7 / 10'); | ||
| }); |
There was a problem hiding this comment.
This test for the controlled component has a couple of issues:
- Similar to the other test, it expects spaces in the count string (
'7 / 10'), which is inconsistent with the implementation. - More importantly, after re-rendering with
value="update"(length 6), the test still asserts the count is'7 / 10', which is based on the old value'initial'. For a controlled component, the count should update to reflect the new prop value, so it should be'6/10'.
This test is logically incorrect and needs to be fixed.
| it('should work with controlled component', () => { | |
| const onChange = jest.fn(); | |
| const { container, rerender } = render( | |
| <Sender maxLength={10} showCount value="initial" onChange={onChange} />, | |
| ); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('7 / 10'); | |
| rerender(<Sender maxLength={10} showCount value="update" onChange={onChange} />); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('7 / 10'); | |
| }); | |
| it('should work with controlled component', () => { | |
| const onChange = jest.fn(); | |
| const { container, rerender } = render( | |
| <Sender maxLength={10} showCount value="initial" onChange={onChange} />, | |
| ); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('7/10'); | |
| rerender(<Sender maxLength={10} showCount value="update" onChange={onChange} />); | |
| expect(container.querySelector('.ant-sender-count')?.textContent).toBe('6/10'); | |
| }); |
| import mountTest from '../../../tests/shared/mountTest'; | ||
| import rtlTest from '../../../tests/shared/rtlTest'; | ||
| import Sender from '../index'; | ||
| import { values } from 'lodash'; |
There was a problem hiding this comment.
The values function is imported from lodash but it doesn't seem to be used in this file. It's good practice to remove unused imports to keep the code clean.
| it('should limit input length when maxLength is set', () => { | ||
| const { container } = render(<Sender maxLength={5} />); | ||
| const textarea = container.querySelector('textarea')!; | ||
| fireEvent.change(textarea, { target: { value: 'abcdefgh' } }); | ||
| expect(textarea).toHaveValue('abcde'); | ||
| }); |
There was a problem hiding this comment.
This test uses fireEvent.change to verify the maxLength behavior. This can be unreliable because jsdom (the environment for React Testing Library) doesn't fully simulate browser behavior for the maxLength attribute on a <textarea>. Programmatically setting the value with fireEvent.change bypasses this constraint, which may cause this test to fail or behave inconsistently.
To create a more robust and realistic test, I recommend using @testing-library/user-event's type method. It simulates actual user typing and correctly respects the maxLength attribute.
Example:
import userEvent from '@testing-library/user-event';
it('should limit input length when maxLength is set', async () => {
const { container } = render(<Sender maxLength={5} />);
const textarea = container.querySelector('textarea')!;
await userEvent.type(textarea, 'abcdefgh');
expect(textarea).toHaveValue('abcde');
});This will make your test more reliable.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/sender/__tests__/index.test.tsx(2 hunks)components/sender/demo/basic.tsx(1 hunks)components/sender/index.tsx(2 hunks)components/sender/style/index.ts(1 hunks)package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
components/sender/style/index.ts (1)
90-101: 样式实现正确且符合设计规范计数器元素的样式定义合理，正确使用了设计 token，布局属性配置得当。
package.json (1)
274-276: 确认 volta 配置的必要性添加 volta Node 版本固定配置似乎与 maxLength/showCount 功能本身无关。虽然 PR 描述中提到了 Windows 环境的 jsdom 兼容性问题，但请确认：
- 此配置是否确实解决了测试问题
- 是否应该在单独的 PR 中处理环境配置变更
components/sender/index.tsx (1)
51-52: 新增属性定义正确
maxLength和showCount属性的类型定义准确，符合组件需求。components/sender/demo/basic.tsx (1)
45-45: 示例演示准确且清晰新增的 Sender 示例正确展示了
maxLength和showCount功能，占位符文本也恰当地说明了限制。components/sender/__tests__/index.test.tsx (1)
265-303: 测试覆盖全面且准确新增的测试套件很好地覆盖了
maxLength和showCount功能的各种场景：
- 输入长度限制
- 计数器显示逻辑
- 实时更新
- 受控组件行为
测试用例设计合理，能够有效验证功能正确性。
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
- 移除可选链后的非空断言，符合lint规则 - 统一属性访问方式与计数器格式 - 移除未使用导入和冗余volta字段
|
机器人的回复需要解决下 |
2 participants
Related Issue
close #1151
Changes
maxLengthinput limitation using Ant Design's native TextArea capabilityshowCountcharacter counter with design token stylingEnvironment Note
Local tests failed on Windows due to
jsdomcompatibility issue (nodejs/node#49173).✅ Functionality verified manually
✅ CI will validate tests (Linux/Node 18)
Screenshots
Summary by CodeRabbit
发布说明
新功能
示例
测试
样式
您好，各位维护者～
我在提交这个 PR 时遇到了一些问题，尝试了一些方法但未能解决，想请大家帮忙指导一下：
测试失败问题：
所有测试用例都报
TypeError: Cannot read properties of undefined (reading 'get')，错误堆栈指向jsdom及其依赖（whatwg-url、webidl-conversions）。我已尝试过：
切换 Node.js 版本到 18.x（之前可能使用了其他版本），并重新安装了依赖，但
jsdom相关的TypeError: Cannot read properties of undefined (reading 'get')错误仍然存在。 查了一些资料，这个错误可能和jsdom与 Node.js 18+ 的内置URL模块冲突有关，或者是项目中某些依赖（如whatwg-url）的版本与jsdom不兼容，但我不太确定具体如何调整。