-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
fix: tab to select an active option #1078
fix: tab to select an active option #1078
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough在此拉取请求中, Changes
Assessment against linked issues
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 #1078 +/- ##
=======================================
Coverage 98.24% 98.24%
=======================================
Files 39 39
Lines 1482 1483 +1
Branches 418 447 +29
=======================================
+ Hits 1456 1457 +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
🧹 Outside diff range and nitpick comments (2)
tests/OptionList.test.tsx (1)
205-243
: 测试用例结构清晰,覆盖了关键功能点这个新增的测试用例很好地验证了Tab键选择活动选项的功能,符合PR的目标和#26876问题的要求。测试步骤逻辑清晰,包括了打开下拉菜单、更改活动选项、选择选项和关闭下拉菜单等关键操作。
为了进一步提高可读性,建议在每个主要步骤前添加简短的注释,例如:
// 打开下拉菜单 act(() => { toggleOpen(true); }); // 模拟按下向下箭头键 act(() => { listRef.current.onKeyDown({ which: KeyCode.DOWN } as any); }); // 验证活动选项已更改 expect(onActiveValue).toHaveBeenCalledWith( '2', expect.anything(), expect.objectContaining({ source: 'keyboard' }), ); // 模拟按下Tab键 act(() => { listRef.current.onKeyDown({ which: KeyCode.TAB } as any); }); // 验证选项被选中并且下拉菜单关闭 expect(onSelect).toHaveBeenCalledTimes(1); expect(onSelect).toHaveBeenCalledWith('2', expect.objectContaining({ selected: true })); expect(toggleOpen).toHaveBeenCalledWith(false);这样可以使测试用例的结构和目的更加清晰。
src/OptionList.tsx (1)
Line range hint
220-234
: Tab键处理实现正确,建议小幅改进Tab键的处理逻辑与Enter键一致,这是正确的实现。它检查了活动项是否存在、是否禁用,以及是否超过最大数量限制,这些都是必要的检查。
为了提高代码的可读性,我们可以考虑将Tab和Enter的处理逻辑提取到一个单独的函数中。例如:
+const handleSelection = () => { + const item = memoFlattenOptions[activeIndex]; + if (item && !item?.data?.disabled && !overMaxCount) { + onSelectValue(item.value); + } else { + onSelectValue(undefined); + } +}; case KeyCode.TAB: case KeyCode.ENTER: { - const item = memoFlattenOptions[activeIndex]; - if (item && !item?.data?.disabled && !overMaxCount) { - onSelectValue(item.value); - } else { - onSelectValue(undefined); - } + handleSelection(); if (open) { event.preventDefault(); } break; }这样可以减少重复代码,并使逻辑更加清晰。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/OptionList.tsx (1 hunks)
- tests/OptionList.test.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/OptionList.tsx (2)
Line range hint
1-624
: 总体评审:键盘导航功能得到改进此PR成功地实现了使用Tab键选择活动选项的功能,提高了组件的键盘可访问性。更改是针对性的,并且与现有代码库保持一致。主要修改点如下:
- 在
onKeyDown
处理程序中添加了对Tab键的支持。- Tab键的处理逻辑与Enter键保持一致,确保了行为的一致性。
这些更改很好地解决了issue #26876中提出的问题,同时没有引入明显的回归或副作用。
建议在合并此PR之前,确保进行充分的测试,特别是针对键盘导航和选择行为的测试。
218-219
: 键盘导航功能增强这个更改增加了对Tab键的支持,使其能够选择当前活动的选项。这符合PR的目标,提高了键盘导航的可访问性。
为了验证这个更改的影响,我们可以运行以下脚本来检查是否有其他地方使用了类似的键盘事件处理逻辑:
✅ Verification successful
为了修正之前脚本中的错误,请运行以下修正后的脚本以重新验证:
键盘导航功能增强已验证
通过运行搜索脚本,我们确认除了
src/OptionList.tsx
外,没有其他文件使用类似的键盘事件处理逻辑。该更改专注于增强选项列表的键盘导航功能,因此本次审查通过。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:搜索其他可能需要类似Tab键处理的地方 # 测试:搜索其他包含键盘事件处理的文件 rg --type typescript "case KeyCode\.(ENTER|ESC|UP|DOWN)"Length of output: 93
Script:
#!/bin/bash # 描述:搜索其他可能需要类似Tab键处理的地方 # 测试:搜索其他包含键盘事件处理的文件 rg "case KeyCode\.(ENTER|ESC|UP|DOWN)" --glob "*.ts" --glob "*.tsx"Length of output: 257
@afc163 麻烦有空可以看看这个小改动, 支持tab to select 感谢! |
+1 |
This is a
keyboard accessibility
Background
When the dropdown is open and an option is active, pressing Tab should select the currently active option.
Fix ant-design/ant-design#26876
Solution
Summary by CodeRabbit
OptionList
组件的键盘导航功能,支持使用Tab和Enter键选择选项。