feat: add aria required prop to slider #1051
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此拉取请求引入了一个新的属性 Changes
Possibly related PRs
Poem
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 #1051 +/- ##
=======================================
Coverage 99.39% 99.39%
=======================================
Files 14 14
Lines 661 661
Branches 191 199 +8
=======================================
Hits 657 657
Misses 4 4 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Co-authored-by: afc163 <[email protected]>
![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
🧹 Outside diff range and nitpick comments (3)
src/context.ts (1)
18-18: 代码变更正确，建议添加属性文档注释新增的
ariaRequired属性类型定义正确，且与其他 ARIA 相关属性放置在一起，符合代码组织逻辑。建议添加 JSDoc 注释说明该属性的用途和使用场景。建议添加如下注释：
+ /** 指定滑块是否为必填项，用于提升表单可访问性 */ ariaRequired?: "true" | "false";README.md (1)
135-135: 建议改进属性描述以保持一致性为了与文档中其他 ARIA 相关属性保持一致，并提供更清晰的用途说明，建议修改描述。
-| ariaRequired | string | - | Set the `aria-required` attribute on the slider handle. | +| ariaRequired | string | - | 设置滑块手柄的 `aria-required` 属性，用于指示此滑块是否为必填项。当设置为 "true" 时，辅助技术会通知用户该字段为必填项。 |tests/Slider.test.js (1)
444-450: 测试实现正确但覆盖率可以提高测试用例遵循了文件中其他 aria 属性测试的模式，实现正确。建议添加以下测试场景以提高覆盖率：
it('sets aria-required on the handle', () => { const { container } = render(<Slider ariaRequired='true' />); expect(container.getElementsByClassName('rc-slider-handle')[0]).toHaveAttribute( 'aria-required', 'true', ); }); + it('does not set aria-required when prop is not provided', () => { + const { container } = render(<Slider />); + expect(container.getElementsByClassName('rc-slider-handle')[0]).not.toHaveAttribute('aria-required'); + }); + + it('sets aria-required to false when explicitly set', () => { + const { container } = render(<Slider ariaRequired='false' />); + expect(container.getElementsByClassName('rc-slider-handle')[0]).toHaveAttribute( + 'aria-required', + 'false' + ); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
README.md(1 hunks)src/Handles/Handle.tsx(2 hunks)src/Slider.tsx(4 hunks)src/context.ts(1 hunks)tests/Slider.test.js(1 hunks)
🔇 Additional comments (5)
src/context.ts (1)
18-18: 验证相关组件中的实现
需要确保此属性在 Handle 组件中正确应用到 DOM 元素上。
运行以下脚本以验证实现：
✅ Verification successful
ariaRequired 属性已正确实现
通过代码分析可以确认：
- Handle 组件（src/Handles/Handle.tsx）正确地将 ariaRequired 属性映射到了
aria-requiredDOM 属性 - Slider 组件正确传递该属性到 Handle 组件
- 已包含相关测试用例验证此功能
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查 Handle 组件是否正确使用了 ariaRequired 属性
# 查找 Handle 组件中的 aria-required 属性使用
ast-grep --pattern 'aria-required={$_}'
# 查找相关测试用例
rg -l "aria-required.*test"
Length of output: 71
Script:
#!/bin/bash
# 搜索包含 Handle 组件定义的文件
fd -e ts -e tsx Handle
# 搜索 Handle 组件的实现，查看属性的使用
ast-grep --pattern 'function Handle($$$) {
$$$
}'
# 搜索 Handle 组件的属性定义
ast-grep --pattern 'interface HandleProps {
$$$
}'
# 搜索任何包含 ariaRequired 的代码
rg "ariaRequired"
Length of output: 685
src/Handles/Handle.tsx (2)
64-64: 代码变更符合预期！
ariaRequired 属性的添加位置正确，与其他 ARIA 相关属性保持一致的代码风格。
172-172: 实现正确，建议验证文档完整性！
aria-required 属性的实现采用了 getIndex 辅助函数，与其他 ARIA 属性保持一致的处理方式。
建议验证以下几点：
- 确保新属性在组件文档中有清晰的使用说明
- 验证 TypeScript 类型定义的完整性
运行以下脚本来验证文档和类型定义：
✅ Verification successful
文档和类型定义已完整实现，无需额外修改
通过验证发现：
ariaRequired属性已在 README.md 中有清晰的说明文档- TypeScript 类型定义在
context.ts和Slider.tsx中已正确声明为"true" | "false" - 属性在组件层级间的传递链路完整：
Slider->Handle-> DOM 属性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查文档和类型定义的完整性
# 查找文档中是否包含 ariaRequired 的说明
rg -i "ariaRequired" "README.md" "docs/"
# 检查相关的类型定义文件
fd -e ts -e tsx | xargs rg "ariaRequired"
Length of output: 502
src/Slider.tsx (2)
184-184: 属性解构中正确添加了 ariaRequired
在属性解构中新增了 ariaRequired，确保了该属性能够在组件中正确使用。
547-547: 在上下文中传递 ariaRequired 属性
已将 ariaRequired 属性添加到 SliderContext 中，确保其能够在子组件中被访问和使用，实现了属性的有效传递。
Also applies to: 566-566
| @@ -112,6 +112,7 @@ export interface SliderProps<ValueType = number | number[]> { | |||
| tabIndex?: number | number[]; | |||
| ariaLabelForHandle?: string | string[]; | |||
| ariaLabelledByForHandle?: string | string[]; | |||
| ariaRequired?: "true" | "false"; | |||
There was a problem hiding this comment.
添加的 ariaRequired 属性类型定义有误
ariaRequired?: "true" | "false"; 中的类型限定为字符串 "true" 或 "false"，但根据 WAI-ARIA 规范，aria-required 属性值应为布尔类型。建议将类型修改为 boolean，以避免类型不匹配的问题。
建议修改代码如下：
- ariaRequired?: "true" | "false";
+ ariaRequired?: boolean;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ariaRequired?: "true" | "false"; | |
| ariaRequired?: boolean; |
What the PR does
Add the aria-required prop to slider
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-required
Summary by CodeRabbit
Slider组件中添加了ariaRequired属性，增强了可访问性。README.md，包含ariaRequired属性的详细信息。aria-required属性在句柄上的存在。