-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: 无障碍问题修复 #6918
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
base: master
Are you sure you want to change the base?
fix: 无障碍问题修复 #6918
Conversation
📝 WalkthroughWalkthrough本次变更主要涉及 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VirtualInput
participant NumberKeyboard
User->>VirtualInput: mouseDown/mouseUp(输入框聚焦/点击字符)
VirtualInput->>VirtualInput: 更新光标位置
User->>NumberKeyboard: mouseDown/mouseUp/click(按键操作)
NumberKeyboard->>VirtualInput: 调用输入/删除回调
VirtualInput->>VirtualInput: 更新输入内容与光标
Estimated code review effort2 (~15分钟) Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
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: 3
🧹 Nitpick comments (1)
src/components/virtual-input/tests/virtual-input.test.tsx (1)
437-447
: 存在注释掉的测试代码这段测试代码被注释掉了,看起来是测试在位置 0 输入小数点的场景。是否需要实现这个测试用例?
需要我帮助完成这个测试用例吗?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (3)
src/components/virtual-input/demos/demo1.tsx
(2 hunks)src/components/virtual-input/tests/virtual-input.test.tsx
(7 hunks)src/components/virtual-input/virtual-input.tsx
(7 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 (15)
src/components/virtual-input/demos/demo1.tsx (5)
3-5
: 导入和正则表达式定义合理React hooks 的导入和货币输入的正则表达式定义都很合适。
8-19
: 第一个演示正确展示了可移动光标功能添加了状态管理和 ref,正确设置了
cursor='movable'
属性。
30-36
: 新增的静态光标演示有助于对比这个演示很好地展示了默认的静态光标行为,与可移动光标形成对比。
61-81
: 货币输入演示实现完善实现了完整的货币输入验证逻辑:
- 正确处理以小数点开头的输入
- 移除前导零
- 使用正则表达式验证格式
- 大字体样式适合金额展示
82-82
: 用于测试的垂直空间这个大的垂直空间可能是为了测试滚动场景下的触摸交互。
src/components/virtual-input/tests/virtual-input.test.tsx (5)
7-106
: 测试基础设施实现完善新增的辅助函数和设置为光标交互测试提供了良好的基础:
clickSiblingElements
能精确模拟点击位置makeTouchEvent
正确模拟触摸事件- 模拟固定字符宽度确保测试一致性
218-285
: 受控组件光标位置测试全面测试覆盖了受控组件的光标交互:
- 键盘输入后的光标位置
- 点击重新定位光标
- 在特定位置删除字符
- 所有断言都正确
501-613
: 触摸拖动光标测试实现优秀测试全面覆盖了触摸拖动功能:
- 模拟真实的触摸拖动场景
- 验证拖动时的视觉反馈
- 测试边界条件和阈值
- 测试超时行为
615-681
: 静态光标模式测试完善正确验证了在默认静态模式下:
- 触摸移动无法改变光标位置
- 点击无法改变光标位置
324-328
: 测试注释与实际行为不一致注释说"点击 '1' 左侧,光标位置应该在值的末尾",但断言期望光标仍在位置 3。这可能是注释错误或测试逻辑问题。
建议修正注释或测试逻辑:
- // click '1' left side in inputbox, caret position should be value end + // click '1' left side in inputbox, caret position should remain at end (3)Likely an incorrect or invalid review comment.
src/components/virtual-input/virtual-input.tsx (5)
102-114
: 光标位置更新逻辑实现正确正确处理了受控组件的光标位置:
- 当值符合预期时保持相对位置
- 当父组件修改值时重置到末尾
147-166
: 键盘输入处理逻辑完善正确实现了光标位置的输入和删除:
- 在光标位置插入字符
- 删除光标前的字符
- 光标在起始位置时阻止删除
194-199
: 点击第一个字符的特殊处理逻辑当
index === 0
时,光标被设置到末尾而不是开头。这个行为是否符合预期?通常点击第一个字符应该将光标放在开头或第一个字符后。请确认这个特殊处理的设计意图。
212-262
: 触摸拖动实现完善触摸事件处理逻辑合理:
- 20px 的触摸阈值适合移动端
- 基于字符宽度的移动计算准确
- 边界检查防止无效位置
- 超时机制处理未完成的手势
314-333
: 清除按钮实现改进正确处理了禁用状态和焦点管理:
- 禁用时隐藏清除按钮
- 防止点击时失去焦点
- 保持无障碍属性
useEffect(() => { | ||
// 记录单个字符的宽度,用于光标移动时的计算 | ||
if (charRef.current) { | ||
charWidthRef.current = charRef.current.getBoundingClientRect().width | ||
} | ||
}, [value]) | ||
|
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.
🛠️ Refactor suggestion
字符宽度测量需要空值检查
当值为空时,charRef.current
可能为 null。
添加空值检查:
useEffect(() => {
// 记录单个字符的宽度,用于光标移动时的计算
- if (charRef.current) {
+ if (charRef.current && value.length > 0) {
charWidthRef.current = charRef.current.getBoundingClientRect().width
}
}, [value])
📝 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.
useEffect(() => { | |
// 记录单个字符的宽度,用于光标移动时的计算 | |
if (charRef.current) { | |
charWidthRef.current = charRef.current.getBoundingClientRect().width | |
} | |
}, [value]) | |
useEffect(() => { | |
// 记录单个字符的宽度,用于光标移动时的计算 | |
- if (charRef.current) { | |
+ if (charRef.current && value.length > 0) { | |
charWidthRef.current = charRef.current.getBoundingClientRect().width | |
} | |
}, [value]) |
🤖 Prompt for AI Agents
In src/components/virtual-input/virtual-input.tsx around lines 95 to 101, the
useEffect hook measures character width but does not safely handle the case when
charRef.current is null. Add a null check before accessing
charRef.current.getBoundingClientRect() to ensure the code only runs when
charRef.current is not null, preventing potential runtime errors.
ref={contentRef} | ||
role='textbox' | ||
tabIndex={mergedProps.disabled ? undefined : 0} | ||
aria-disabled={mergedProps.disabled} |
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.
存在重复的无障碍属性
role="textbox"
和 tabIndex
同时出现在根元素和内容元素上,这可能导致屏幕阅读器的混淆。
建议只在根元素上保留这些属性:
<div
className={`${classPrefix}-content`}
ref={contentRef}
- role='textbox'
- tabIndex={mergedProps.disabled ? undefined : 0}
aria-disabled={mergedProps.disabled}
aria-label={mergedProps.placeholder}
📝 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.
ref={contentRef} | |
role='textbox' | |
tabIndex={mergedProps.disabled ? undefined : 0} | |
aria-disabled={mergedProps.disabled} | |
<div | |
className={`${classPrefix}-content`} | |
ref={contentRef} | |
aria-disabled={mergedProps.disabled} | |
aria-label={mergedProps.placeholder} |
🤖 Prompt for AI Agents
In src/components/virtual-input/virtual-input.tsx around lines 280 to 283, the
accessibility attributes role="textbox" and tabIndex are duplicated on both the
root and content elements, which can confuse screen readers. Remove these
attributes from the content element and keep them only on the root element to
ensure proper accessibility semantics.
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6918 +/- ##
=======================================
Coverage 92.82% 92.82%
=======================================
Files 336 336
Lines 7271 7275 +4
Branches 1832 1833 +1
=======================================
+ Hits 6749 6753 +4
Misses 486 486
Partials 36 36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
新功能
role="textbox"
属性,提升了无障碍访问体验。修复
样式
测试
其他