-
-
Notifications
You must be signed in to change notification settings - Fork 466
feat:Using the showScrollBar property of the List component #1122
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改在 BaseSelect 和 OptionList 组件中增加了新的可选属性 showScrollBar,该属性支持 boolean 和字符串 "optional" 两种类型,用于控制组件内部滚动条的显示。该属性通过 BaseSelectContext 传递给子组件,并在 OptionList 中传递给 List 组件。同时,OptionList 的属性定义由接口调整为 Record<string, never>,并新增了相关测试用例,验证不同情况下滚动条的显示逻辑。 Changes
Sequence Diagram(s)sequenceDiagram
participant 用户 as User
participant BaseSelect
participant Context as BaseSelectContext
participant OptionList
participant List
用户->>BaseSelect: 传入 showScrollBar 属性
BaseSelect->>Context: 在上下文中注入 showScrollBar
Context-->>OptionList: 传递 showScrollBar 属性
OptionList->>List: 将 showScrollBar 属性传递给 List
List-->>用户: 渲染组件(带或不带滚动条)
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
tests/OptionList.test.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1122 +/- ##
=======================================
Coverage 98.26% 98.26%
=======================================
Files 39 39
Lines 1498 1499 +1
Branches 451 452 +1
=======================================
+ Hits 1472 1473 +1
Misses 26 26 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/ListScrollBar.test.tsx (2)
8-25
: 虚拟列表的模拟实现可以更完善。当前的虚拟列表模拟主要关注
scrollTo
方法,建议增加对其他重要属性和方法的模拟。建议添加以下增强:
return OriReact.forwardRef((props, ref) => { const oriRef = OriReact.useRef(); + // 添加更多虚拟列表的核心功能模拟 + const mockListRef = { + scrollHeight: 1000, + clientHeight: 200, + scrollTop: 0, + ...oriRef.current, + }; OriReact.useImperativeHandle(ref, () => ({ - ...oriRef.current, + ...mockListRef, scrollTo: (arg) => { global.scrollToArgs = arg; - oriRef.current.scrollTo(arg); + mockListRef.scrollTo(arg); }, }));
69-81
: 测试用例需要覆盖更多场景。当前测试用例仅验证了基本的显示和隐藏场景,建议增加更多边界情况的测试。
建议添加以下测试场景:
- 测试
showScrollBar='optional'
的情况- 测试列表项数量少于可视区域时的滚动条行为
- 测试动态切换
showScrollBar
属性的情况- 测试不同设备(移动端/桌面端)的滚动条表现
Also applies to: 82-94
src/BaseSelect/index.tsx (1)
139-139
: 属性定义需要添加文档注释。
showScrollBar
属性缺少 JSDoc 注释来说明其用途和可选值的含义。建议添加如下文档注释:
+/** + * 控制下拉列表滚动条的显示方式 + * @default 'optional' + */ showScrollBar?: boolean | 'optional';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/BaseSelect/index.tsx
(3 hunks)src/OptionList.tsx
(2 hunks)tests/ListScrollBar.test.tsx
(1 hunks)tests/utils/domHook.ts
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
tests/utils/domHook.ts
[warning] 23-23: tests/utils/domHook.ts#L23
Added line #L23 was not covered by tests
[warning] 25-25: tests/utils/domHook.ts#L25
Added line #L25 was not covered by tests
[warning] 27-27: tests/utils/domHook.ts#L27
Added line #L27 was not covered by tests
[warning] 33-33: tests/utils/domHook.ts#L33
Added line #L33 was not covered by tests
[warning] 47-47: tests/utils/domHook.ts#L47
Added line #L47 was not covered by tests
[warning] 56-57: tests/utils/domHook.ts#L56-L57
Added lines #L56 - L57 were not covered by tests
🔇 Additional comments (6)
tests/utils/domHook.ts (2)
4-54
: 代码实现完善,但需要增加测试覆盖率。
spyElementPrototypes
函数的实现逻辑清晰,但有几个关键路径缺少测试覆盖:
- 属性的 set 方法调用
- 原始属性的 get 方法调用
- 函数类型属性的恢复
建议添加以下场景的测试用例:
- 测试属性的 set/get 方法
- 测试函数类型属性的 spy 和恢复
- 测试
NO_EXIST
情况的处理🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 23-23: tests/utils/domHook.ts#L23
Added line #L23 was not covered by tests
[warning] 25-25: tests/utils/domHook.ts#L25
Added line #L25 was not covered by tests
[warning] 27-27: tests/utils/domHook.ts#L27
Added line #L27 was not covered by tests
[warning] 33-33: tests/utils/domHook.ts#L33
Added line #L33 was not covered by tests
[warning] 47-47: tests/utils/domHook.ts#L47
Added line #L47 was not covered by tests
56-60
: 实用的便捷函数封装。
spyElementPrototype
函数提供了一个简洁的接口来监视单个属性,这是一个很好的抽象。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 56-57: tests/utils/domHook.ts#L56-L57
Added lines #L56 - L57 were not covered by testssrc/OptionList.tsx (2)
19-20
: 类型定义的改变可能影响向后兼容性。将
OptionListProps
从泛型接口改为Record<string, never>
可能会影响现有代码。请确认这个改动不会影响现有的使用场景。如果有使用这个接口的代码,需要相应更新。
47-47
: 正确传递了 showScrollBar 属性。从
useBaseProps
中解构并传递showScrollBar
属性到List
组件的实现是正确的。Also applies to: 329-329
src/BaseSelect/index.tsx (2)
226-226
: 默认值设置合理。将
showScrollBar
的默认值设置为'optional'
是一个合理的选择,这样可以在需要时显示滚动条。
697-709
: 上下文依赖项更新完整。正确地将
showScrollBar
添加到了上下文值和依赖数组中。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/OptionList.test.tsx (3)
9-11
: 导入语句的顺序需要调整建议将相关的导入语句进行分组和排序:
- 外部依赖
- 内部组件
- 工具函数
-import { createEvent, fireEvent, render, waitFor } from '@testing-library/react'; -import Select from '../src'; -import { spyElementPrototypes } from 'rc-util/lib/test/domHook'; +import { createEvent, fireEvent, render, waitFor } from '@testing-library/react'; +import { spyElementPrototypes } from 'rc-util/lib/test/domHook'; +import Select from '../src';
507-519
: 建议增强滚动条显示测试用例当前测试用例可以进一步完善:
- 测试不同的滚动条状态切换
- 验证滚动条样式属性
it('should show scrollbar when showScrollBar is true', async () => { const options = Array.from({ length: 10 }, (_, index) => ({ label: `${index + 1}`, value: `${index + 1}`, })); - const { container } = render(<Select open showScrollBar options={options} />); + const { container, rerender } = render(<Select open showScrollBar options={options} />); await waitFor(() => { const scrollbarElement = container.querySelector('.rc-virtual-list-scrollbar-visible'); expect(scrollbarElement).not.toBeNull(); + expect(scrollbarElement).toHaveStyle({ display: 'block' }); }); + + // 测试状态切换 + rerender(<Select open showScrollBar={false} options={options} />); + await waitFor(() => { + const scrollbarElement = container.querySelector('.rc-virtual-list-scrollbar-visible'); + expect(scrollbarElement).toBeNull(); + }); });
520-532
: 建议补充边界情况测试当前测试用例可以添加以下场景:
- 选项数量少于显示区域时的滚动条行为
- 测试 showScrollBar="optional" 的情况
it('not have scrollbar when showScrollBar is false', async () => { const options = Array.from({ length: 10 }, (_, index) => ({ label: `${index + 1}`, value: `${index + 1}`, })); const { container } = render(<Select open showScrollBar={false} options={options} />); await waitFor(() => { const scrollbarElement = container.querySelector('.rc-virtual-list-scrollbar-visible'); expect(scrollbarElement).toBeNull(); }); }); + +it('should handle optional scrollbar correctly', async () => { + // 测试选项数量少时不显示滚动条 + const fewOptions = Array.from({ length: 3 }, (_, index) => ({ + label: `${index + 1}`, + value: `${index + 1}`, + })); + + const { container, rerender } = render( + <Select open showScrollBar="optional" options={fewOptions} /> + ); + + await waitFor(() => { + const scrollbarElement = container.querySelector('.rc-virtual-list-scrollbar-visible'); + expect(scrollbarElement).toBeNull(); + }); + + // 测试选项数量多时显示滚动条 + const manyOptions = Array.from({ length: 20 }, (_, index) => ({ + label: `${index + 1}`, + value: `${index + 1}`, + })); + + rerender(<Select open showScrollBar="optional" options={manyOptions} />); + + await waitFor(() => { + const scrollbarElement = container.querySelector('.rc-virtual-list-scrollbar-visible'); + expect(scrollbarElement).not.toBeNull(); + }); +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/OptionList.test.tsx
(2 hunks)
🔇 Additional comments (1)
tests/OptionList.test.tsx (1)
465-506
: 测试套件的设置逻辑清晰且完整测试套件中的 mock 设置和清理逻辑完整,包括:
- 模拟 HTMLElement 的必要属性
- 正确的生命周期钩子使用
- 合理的默认值设置
相关链接:ant-design/ant-design#33915
Summary by CodeRabbit