refactor: enhance tab navigation and accessibility features #763
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
src/TabNavList/index.tsx (3)
303-312: 建议优化键盘事件监听的实现当前在
useEffect中对window添加了键盘事件监听器，这可能导致组件卸载后仍然存在事件监听。虽然您的清理函数已正确处理，但为了更好的封装，建议将键盘事件监听器添加到组件的根节点上，而不是全局window，以避免全局事件可能带来的意外影响。
332-397: 建议使用e.key替代已废弃的e.which在处理键盘事件时，
e.which属性已被废弃，建议使用e.key或e.keyCode，以提高兼容性和未来的可维护性。这有助于确保代码在所有浏览器中都能正常工作。您可以将代码修改为：
- const { which } = e; + const key = e.key; switch (key) { case 'ArrowLeft': // ... case 'ArrowRight': // ... case 'ArrowUp': // ... case 'ArrowDown': // ... case 'Home': // ... case 'End': // ... case 'Enter': case ' ': // ... }
374-385: 优化重复计算的enabledTabs列表在多处使用了
enabledTabs = tabs.filter(tab => !tab.disabled).map(tab => tab.key);，建议将其提取为一个缓存或使用useMemo，以避免重复计算，提高性能。您可以修改为：
const enabledTabs = React.useMemo( () => tabs.filter(tab => !tab.disabled).map(tab => tab.key), [tabs], );assets/index.less (1)
129-131: 检查焦点样式的颜色对比度在
&-focus类中新增了outline: 1px auto #1677ff;，请确保该颜色与背景之间有足够的对比度，以满足无障碍标准，提升用户体验。tests/index.test.tsx (1)
Line range hint
1-606: 缺少键盘导航相关的测试用例根据 PR 的主要目标是增强键盘导航和可访问性功能，但在测试文件中没有看到相关的测试用例。建议添加以下测试场景：
- 水平标签页的左右键导航
- 垂直标签页的上下键导航
- Home/End 键快速导航
- Enter/Space 键激活标签页
- RTL 布局下的键盘导航
- 焦点状态的视觉指示器
建议添加如下测试用例：
+describe('Keyboard Navigation', () => { + it('should support horizontal navigation with arrow keys', () => { + const { container } = render(getTabs()); + const tabs = container.querySelectorAll('.rc-tabs-tab'); + + // 模拟键盘事件 + fireEvent.keyDown(tabs[0], { key: 'ArrowRight' }); + expect(document.activeElement).toBe(tabs[1]); + + fireEvent.keyDown(tabs[1], { key: 'ArrowLeft' }); + expect(document.activeElement).toBe(tabs[0]); + }); + + it('should support vertical navigation with arrow keys', () => { + const { container } = render(getTabs({ tabPosition: 'left' })); + const tabs = container.querySelectorAll('.rc-tabs-tab'); + + fireEvent.keyDown(tabs[0], { key: 'ArrowDown' }); + expect(document.activeElement).toBe(tabs[1]); + + fireEvent.keyDown(tabs[1], { key: 'ArrowUp' }); + expect(document.activeElement).toBe(tabs[0]); + }); + + it('should support Home/End navigation', () => { + const { container } = render(getTabs()); + const tabs = container.querySelectorAll('.rc-tabs-tab'); + + fireEvent.keyDown(tabs[0], { key: 'End' }); + expect(document.activeElement).toBe(tabs[tabs.length - 1]); + + fireEvent.keyDown(tabs[tabs.length - 1], { key: 'Home' }); + expect(document.activeElement).toBe(tabs[0]); + }); + + it('should activate tab with Enter/Space', () => { + const onChange = jest.fn(); + const { container } = render(getTabs({ onChange })); + const tabs = container.querySelectorAll('.rc-tabs-tab'); + + fireEvent.keyDown(tabs[1], { key: 'Enter' }); + expect(onChange).toHaveBeenCalledWith('bamboo'); + + fireEvent.keyDown(tabs[2], { key: ' ' }); + expect(onChange).toHaveBeenCalledWith('cute'); + }); +});
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
tests/__snapshots__/index.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
assets/index.less(5 hunks)docs/examples/basic.tsx(2 hunks)src/TabNavList/TabNode.tsx(4 hunks)src/TabNavList/index.tsx(14 hunks)tests/index.test.tsx(1 hunks)
🔇 Additional comments (3)
docs/examples/basic.tsx (1)
78-82: 验证方向切换功能
在切换 direction 状态时，请确保界面能够正确响应并更新布局方向，以支持 ltr 和 rtl 两种模式的正确显示和键盘导航。
src/TabNavList/TabNode.tsx (1)
87-95: 确保 tabIndex 的设置符合无障碍原则
当前非活动的标签被设置了 tabIndex={-1}，请验证这不会影响键盘导航的连贯性，并确保用户可以使用键盘焦点正确导航所有可用的标签。
tests/index.test.tsx (1)
27-27: 将 jest.useFakeTimers() 移至 beforeEach 是个好的改进
将计时器模拟的设置移到 beforeEach 中确保了所有测试用例都能在一致的环境下运行，这是一个很好的改进。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
tests/accessibility.test.tsx (2)
38-71: 键盘导航测试覆盖全面，建议增加边缘情况测试测试用例很好地覆盖了基本的键盘导航功能。建议添加以下测试场景：
- 快速按键的情况
- Tab 列表更新后焦点保持的情况
92-111: Enter/Space 激活测试可以更完善建议增加以下测试场景：
- 在禁用的选项卡上按 Enter/Space 键
- 验证焦点状态在激活后的行为
src/TabNavList/index.tsx (2)
331-395: 建议优化键盘事件处理的性能handleKeyDown 函数在每次渲染时都会重新创建。建议使用 useCallback 优化性能：
- const handleKeyDown = (e: React.KeyboardEvent) => { + const handleKeyDown = useCallback((e: React.KeyboardEvent) => { // ... existing code ... - }; + }, [isHorizontal, rtl, enabledTabs, onOffset, setFocusKey, onTabClick, focusKey]);
Line range hint
418-445: 建议增强 TabNode 的可访问性属性为了提高可访问性，建议在 TabNode 组件中添加更多的 ARIA 属性：
<TabNode id={id} prefixCls={prefixCls} key={key} tab={tab} style={i === 0 ? undefined : tabNodeStyle} closable={tab.closable} editable={editable} active={key === activeKey} focus={key === focusKey} + aria-selected={key === activeKey} + aria-disabled={tab.disabled} + aria-controls={`${id}-panel-${key}`} renderWrapper={children} removeAriaLabel={locale?.removeAriaLabel} // ... rest of the props />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
package.json(1 hunks)src/TabNavList/index.tsx(13 hunks)tests/accessibility.test.tsx(1 hunks)
🔇 Additional comments (2)
package.json (1)
53-53: 添加用户事件测试库的依赖项是合适的
添加 @testing-library/user-event 依赖对于实现键盘导航的自动化测试是必要的。版本 14.5.2 是稳定的发布版本。
tests/accessibility.test.tsx (1)
128-150: 键盘和鼠标导航区分测试设计合理
测试很好地验证了键盘和鼠标导航的不同行为，确保了正确的焦点样式应用。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/TabNavList/index.tsx (2)
297-314: 建议优化键盘事件监听器的实现当前实现可以通过使用 useCallback 来优化性能：
+const captureTabKey = useCallback((e: KeyboardEvent) => { + if (e.key === 'Tab') { + setIsKeyboard(true); + } +}, []); -useEffect(() => { - const captureTabKey = (e: KeyboardEvent) => { - if (e.key === 'Tab') { - setIsKeyboard(true); - } - }; - - window.addEventListener('keydown', captureTabKey); - return () => { - window.removeEventListener('keydown', captureTabKey); - }; -}, []); +useEffect(() => { + window.addEventListener('keydown', captureTabKey); + return () => { + window.removeEventListener('keydown', captureTabKey); + }; +}, [captureTabKey]);
Line range hint
418-445: 建议添加无障碍功能的文档说明为了帮助其他开发者理解这些无障碍功能的实现，建议在代码中添加以下注释：
+// 处理键盘焦点状态 focus={key === focusKey} onFocus={() => { + // 仅在键盘导航时更新焦点 if (isKeyboard) { setFocusKey(key); } scrollToTab(key); doLockAnimation(); if (!tabsWrapperRef.current) { return; } + // 重置滚动位置以确保正确的焦点显示 if (!rtl) { tabsWrapperRef.current.scrollLeft = 0; } tabsWrapperRef.current.scrollTop = 0; }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/TabNavList/index.tsx(13 hunks)tests/accessibility.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/accessibility.test.tsx
🔇 Additional comments (2)
src/TabNavList/index.tsx (2)
91-92: 参数命名优化得当！
将 tabPositionTopOrBottom 重命名为 isHorizontal 使得参数含义更加清晰直观。
331-395: 建议验证键盘导航的边界情况
键盘导航的实现非常全面，包括了水平、垂直方向的导航以及RTL布局支持。建议添加以下验证：
- 在没有可用标签时的处理
- 在所有标签都被禁用时的处理
✅ Verification successful
Let me analyze the test coverage for keyboard navigation and boundary cases before making a final recommendation.
键盘导航的边界情况已经得到妥善处理
经过代码验证，发现：
-
已有完整的键盘导航测试用例（tests/accessibility.test.tsx），包括：
- 水平和垂直方向的导航
- 禁用标签的跳过处理
- Home/End 键的边界处理
- RTL布局支持
-
代码中已经实现了边界情况处理：
enabledTabs过滤掉了所有禁用的标签- 在导航时会自动循环（当到达末尾时回到开始，反之亦然）
- 通过
firstEnabledTab和lastEnabledTab确保 Home/End 键只会定位到可用标签
原审查意见中提出的顾虑已经在实现中得到了充分考虑。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否存在边界情况的测试用例
rg -l "describe.*keyboard.*navigation" --type ts
Length of output: 50
Script:
#!/bin/bash
# Search for test files related to TabNavList
fd ".*test.*" --type f --extension ts --extension tsx | rg -i "tabnav"
# Search for keyboard event handling tests
rg "keydown|keypress|keyboard" --type ts --type tsx -C 2
Length of output: 157
Script:
#!/bin/bash
# Search for test files and keyboard event handling
rg "keydown|keypress|keyboard" -t ts -C 2
# Check implementation for boundary case handling
ast-grep --pattern 'const handleKeyDown = ($_) => {
$$$
if ($enabledTabs.length === 0) {
$$$
}
$$$
}'
# Look for disabled tabs handling
rg "enabledTabs" -t ts -C 2
Length of output: 8055
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/examples/basic.tsx (2)
40-42: 建议改进点击处理函数的实现当前的
onTabClick函数仅打印日志，建议添加更有意义的处理逻辑。const onTabClick = (key: string) => { - console.log('key', key); + // 处理标签页激活状态 + setActiveKey(key); + // 触发回调 + props.onChange?.(key); };
75-82: 建议增强方向切换按钮的可访问性为了提高可访问性，建议添加 ARIA 属性和更清晰的按钮文本。
<button type="button" + aria-label={`切换到${direction === 'ltr' ? '从右到左' : '从左到右'}模式`} onClick={() => { setDirection(direction === 'ltr' ? 'rtl' : 'ltr'); }} > - {direction === 'ltr' ? 'rtl' : 'ltr'} + {direction === 'ltr' ? '切换到从右到左' : '切换到从左到右'} </button>src/TabNavList/index.tsx (1)
334-398: 建议优化键盘导航处理函数的组织结构当前实现完整地支持了键盘导航，但可以通过提取常量和辅助函数来提高代码的可维护性。
+const NAVIGATION_KEYS = { + LEFT: 'ArrowLeft', + RIGHT: 'ArrowRight', + UP: 'ArrowUp', + DOWN: 'ArrowDown', + HOME: 'Home', + END: 'End', + ENTER: 'Enter', + SPACE: 'Space', +}; +const handleDirectionalNavigation = ( + code: string, + isRTL: boolean, + tabPositionTopOrBottom: boolean, + onOffset: (offset: number) => void, +) => { + switch (code) { + case NAVIGATION_KEYS.LEFT: + if (tabPositionTopOrBottom) { + onOffset(isRTL ? 1 : -1); + } + break; + case NAVIGATION_KEYS.RIGHT: + if (tabPositionTopOrBottom) { + onOffset(isRTL ? -1 : 1); + } + break; + // ... 其他方向键处理 + } +}; const handleKeyDown = (e: React.KeyboardEvent) => { const { code } = e; const isRTL = rtl && tabPositionTopOrBottom; - // ... 原有的 switch 语句 + handleDirectionalNavigation(code, isRTL, tabPositionTopOrBottom, onOffset); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
docs/examples/basic.tsx(2 hunks)src/TabNavList/index.tsx(4 hunks)
🔇 Additional comments (4)
docs/examples/basic.tsx (1)
27-32: 代码结构清晰且符合规范！
新增的标签页遵循了现有的数据结构模式。
src/TabNavList/index.tsx (3)
153-154: 优化滚动计算的精确度！
使用 Math.floor 处理计算结果，避免了浮点数精度问题。
319-332: 循环导航实现完善！
标签页偏移计算逻辑处理了边界情况，确保了循环导航的正确性。
300-317:
避免使用全局键盘事件监听器
根据之前的评审意见，全局事件监听可能会干扰嵌套标签页的正常工作。建议改用本地焦点管理。
-useEffect(() => {
- const captureTabKey = (e: KeyboardEvent) => {
- if (e.key === 'Tab') {
- setIsKeyboard(true);
- }
- };
-
- window.addEventListener('keydown', captureTabKey);
- return () => {
- window.removeEventListener('keydown', captureTabKey);
- };
-}, []);
+const handleFocus = (e: React.FocusEvent) => {
+ // 检查是否是键盘触发的焦点
+ if (e.target.matches(':focus-visible')) {
+ setIsKeyboard(true);
+ }
+};
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
src/TabNavList/index.tsx (5)
300-302: 状态管理优化建议建议使用
useCallback优化状态更新函数，并考虑将这些相关状态合并到一个对象中。-const [focusKey, setFocusKey] = useState<string>(); -const [isKeyboard, setIsKeyboard] = useState(false); +const [focusState, setFocusState] = useState<{ + focusKey?: string; + isKeyboard: boolean; +}>({ isKeyboard: false });
304-319: 建议优化 Tab 导航逻辑当前实现在循环导航时可能会造成性能问题。建议使用取模运算来优化循环逻辑。
const onOffset = (offset: number) => { const currentIndex = enabledTabs.indexOf(focusKey || activeKey); - let newIndex = currentIndex + offset; - if (newIndex < 0) { - newIndex = enabledTabs.length - 1; - } else if (newIndex >= enabledTabs.length) { - newIndex = 0; - } + const newIndex = (currentIndex + offset + enabledTabs.length) % enabledTabs.length; const newKey = enabledTabs[newIndex]; setFocusKey(newKey); };
321-385: 键盘事件处理优化建议建议将键盘事件处理逻辑拆分为更小的函数，提高代码可维护性。同时建议使用 Map 或对象来替代 switch 语句。
+const keyActionMap = { + ArrowLeft: (isRTL: boolean, tabPositionTopOrBottom: boolean) => + tabPositionTopOrBottom && onOffset(isRTL ? 1 : -1), + ArrowRight: (isRTL: boolean, tabPositionTopOrBottom: boolean) => + tabPositionTopOrBottom && onOffset(isRTL ? -1 : 1), + ArrowUp: (isRTL: boolean, tabPositionTopOrBottom: boolean) => + !tabPositionTopOrBottom && onOffset(-1), + ArrowDown: (isRTL: boolean, tabPositionTopOrBottom: boolean) => + !tabPositionTopOrBottom && onOffset(1), + Home: () => setFocusKey(firstEnabledTab), + End: () => setFocusKey(lastEnabledTab), + Enter: (e: React.KeyboardEvent) => onTabClick(focusKey, e), + Space: (e: React.KeyboardEvent) => onTabClick(focusKey, e), +}; const handleKeyDown = (e: React.KeyboardEvent) => { const { code } = e; const isRTL = rtl && tabPositionTopOrBottom; - switch(code) { - // ... existing switch cases - } + const action = keyActionMap[code]; + if (action) { + e.preventDefault(); + action(isRTL, tabPositionTopOrBottom); + } };
Line range hint
414-438: 建议优化焦点管理逻辑当前的焦点管理实现可能在某些边缘情况下出现问题。建议添加防抖处理，并优化事件处理函数。
+import { debounce } from 'lodash'; +const debouncedSetFocus = debounce((key: string) => { + setFocusKey(key); + scrollToTab(key); +}, 100); onFocus={() => { if (isKeyboard) { - setFocusKey(key); + debouncedSetFocus(key); } - scrollToTab(key); doLockAnimation(); // ... rest of the code }}
153-154: 建议优化滚动判断逻辑当前的滚动判断逻辑使用
Math.floor可能在某些浏览器中出现精度问题。建议使用更稳健的比较方式。-const needScroll = Math.floor(containerExcludeExtraSizeValue) < Math.floor(tabContentSizeValue + addSizeValue); +const EPSILON = 0.1; // 精度阈值 +const needScroll = (containerExcludeExtraSizeValue + EPSILON) < (tabContentSizeValue + addSizeValue);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/TabNavList/TabNode.tsx(4 hunks)src/TabNavList/index.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/TabNavList/TabNode.tsx
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/TabNavList/index.tsx (5)
300-302: 状态管理可以优化建议使用
useCallback来优化状态更新函数，并考虑将这些相关状态合并到一个对象中。-const [focusKey, setFocusKey] = useState<string>(); -const [isKeyboard, setIsKeyboard] = useState(true); +const [focusState, setFocusState] = useState<{ + focusKey?: string; + isKeyboard: boolean; +}>({ + focusKey: undefined, + isKeyboard: true, +}); + +const setFocusKey = useCallback((key?: string) => { + setFocusState(prev => ({ ...prev, focusKey: key })); +}, []);
304-304: 建议添加性能优化
enabledTabs在每次渲染时都会重新计算。建议使用useMemo优化性能。-const enabledTabs = tabs.filter(tab => !tab.disabled).map(tab => tab.key); +const enabledTabs = useMemo( + () => tabs.filter(tab => !tab.disabled).map(tab => tab.key), + [tabs] +);
306-319: 建议优化循环逻辑当前的循环逻辑可以简化，使用取模运算来处理边界情况。
const onOffset = (offset: number) => { const currentIndex = enabledTabs.indexOf(focusKey || activeKey); - let newIndex = currentIndex + offset; - if (newIndex < 0) { - newIndex = enabledTabs.length - 1; - } else if (newIndex >= enabledTabs.length) { - newIndex = 0; - } + const newIndex = (currentIndex + offset + enabledTabs.length) % enabledTabs.length; const newKey = enabledTabs[newIndex]; setFocusKey(newKey); };
433-438: 鼠标事件处理可以优化建议合并鼠标按下和抬起的处理逻辑，使用单个事件处理器。
-onMouseDown={() => { - setIsKeyboard(false); -}} -onMouseUp={() => { - setIsKeyboard(true); -}} +onMouseDown={() => { + setIsKeyboard(false); + const handleMouseUp = () => { + setIsKeyboard(true); + document.removeEventListener('mouseup', handleMouseUp); + }; + document.addEventListener('mouseup', handleMouseUp); +}}
153-154: 建议优化数学计算使用
Math.floor进行比较可能会导致精度问题，建议使用一个小的误差范围。-const needScroll = - Math.floor(containerExcludeExtraSizeValue) < Math.floor(tabContentSizeValue + addSizeValue); +const EPSILON = 0.1; +const needScroll = + containerExcludeExtraSizeValue < tabContentSizeValue + addSizeValue + EPSILON;
src/TabNavList/index.tsx
Outdated
| const handleKeyDown = (e: React.KeyboardEvent) => { | ||
| const { code } = e; | ||
|
|
||
| const isRTL = rtl && tabPositionTopOrBottom; | ||
| const firstEnabledTab = enabledTabs[0]; | ||
| const lastEnabledTab = enabledTabs[enabledTabs.length - 1]; | ||
|
|
||
| switch (code) { | ||
| // LEFT | ||
| case 'ArrowLeft': { | ||
| if (tabPositionTopOrBottom) { | ||
| onOffset(isRTL ? 1 : -1); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| // RIGHT | ||
| case 'ArrowRight': { | ||
| if (tabPositionTopOrBottom) { | ||
| onOffset(isRTL ? -1 : 1); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| // UP | ||
| case 'ArrowUp': { | ||
| e.preventDefault(); | ||
| if (!tabPositionTopOrBottom) { | ||
| onOffset(-1); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| // DOWN | ||
| case 'ArrowDown': { | ||
| e.preventDefault(); | ||
| if (!tabPositionTopOrBottom) { | ||
| onOffset(1); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| // HOME | ||
| case 'Home': { | ||
| e.preventDefault(); | ||
| setFocusKey(firstEnabledTab); | ||
| break; | ||
| } | ||
|
|
||
| // END | ||
| case 'End': { | ||
| e.preventDefault(); | ||
| setFocusKey(lastEnabledTab); | ||
| break; | ||
| } | ||
|
|
||
| // Enter & Space | ||
| case 'Enter': | ||
| case 'Space': { | ||
| e.preventDefault(); | ||
| onTabClick(focusKey, e); | ||
| break; | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
键盘事件处理需要优化
- 建议将键盘事件处理函数使用
useCallback包装以优化性能 - 建议添加
aria-keyshortcuts属性来提升可访问性
+const handleKeyDown = useCallback(
(e: React.KeyboardEvent) => {
const { code } = e;
const isRTL = rtl && tabPositionTopOrBottom;
// ... existing code ...
},
- []
+ [rtl, tabPositionTopOrBottom, focusKey, onTabClick]
+);
// 在 TabNode 组件上添加
aria-keyshortcuts="ArrowLeft ArrowRight ArrowUp ArrowDown Home End Enter Space"Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/TabNavList/TabNode.tsx (2)
91-107: 建议添加键盘快捷键的 ARIA 属性为了提升可访问性，建议添加
aria-keyshortcuts属性来说明支持的键盘操作。<div ref={btnRef} role="tab" aria-selected={active} id={id && `${id}-tab-${key}`} className={`${tabPrefix}-btn`} aria-controls={id && `${id}-panel-${key}`} aria-disabled={disabled} + aria-keyshortcuts="ArrowLeft ArrowRight ArrowUp ArrowDown Home End Enter Space" tabIndex={disabled ? null : active ? 0 : -1} onClick={e => { e.stopPropagation(); onInternalClick(e); }} onKeyDown={onKeyDown} onMouseDown={onMouseDown} onMouseUp={onMouseUp} onFocus={onFocus} onBlur={onBlur} >
117-119: 建议添加键盘导航相关的代码注释为了提高代码可维护性，建议添加注释说明删除按钮的键盘导航行为。
<button type="button" aria-label={removeAriaLabel || 'remove'} + // 仅在标签页激活时才能通过键盘导航到删除按钮 tabIndex={active ? 0 : -1} className={`${tabPrefix}-remove`}src/TabNavList/index.tsx (2)
314-401: 建议增加边缘情况的错误处理键盘事件处理逻辑完善，但建议添加以下安全检查：
- 在执行
onOffset前检查enabledTabs是否为空- 在删除标签页时确保
focusKey存在const onOffset = (offset: number) => { + if (enabledTabs.length === 0) return; const currentIndex = enabledTabs.indexOf(focusKey || activeKey); const len = enabledTabs.length; const nextIndex = (currentIndex + offset + len) % len; const newKey = enabledTabs[nextIndex]; setFocusKey(newKey); }; // 在 Backspace case 中 case 'Backspace': { + if (!focusKey) return; const removeIndex = enabledTabs.indexOf(focusKey);
Line range hint
430-454: 建议对焦点事件处理添加防抖为了优化性能，建议对
onFocus和onBlur事件处理添加防抖。+import { useDebounceFn } from 'rc-util/lib/hooks/useDebounceFn'; const TabNavList = React.forwardRef<HTMLDivElement, TabNavListProps>((props, ref) => { + const debouncedSetFocusKey = useDebounceFn( + (key?: string) => { + setFocusKey(key); + }, + { wait: 50 } + ); // 在 onFocus 中 onFocus={() => { if (isKeyboard) { - setFocusKey(key); + debouncedSetFocusKey.run(key); } // ... }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/TabNavList/TabNode.tsx(4 hunks)src/TabNavList/index.tsx(5 hunks)
🔇 Additional comments (5)
src/TabNavList/TabNode.tsx (2)
11-23: 接口更改符合可访问性要求！
新增的属性完善了键盘导航和焦点管理功能，类型定义准确。
68-74: 焦点管理实现得当！
使用 useRef 和 useEffect 的组合来管理焦点状态，实现简洁有效。
src/TabNavList/index.tsx (3)
300-302: 状态管理实现合理！
使用独立的状态来管理焦点键值和键盘交互模式，实现清晰。
306-312: 标签页偏移计算优化得当！
使用取模运算实现循环导航，代码简洁高效。
153-154: 滚动行为处理准确！
使用 Math.floor 处理尺寸比较，避免了小数误差问题。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/TabNavList/index.tsx (3)
306-312: 建议优化循环导航逻辑当前实现使用了简洁的取模运算来处理循环导航,但建议添加一些防御性检查:
const onOffset = (offset: number) => { + if (!enabledTabs.length) return; const currentIndex = enabledTabs.indexOf(focusKey || activeKey); const len = enabledTabs.length; const nextIndex = (currentIndex + offset + len) % len; const newKey = enabledTabs[nextIndex]; setFocusKey(newKey); };
314-402: 键盘事件处理实现完善键盘导航实现符合 WAI-ARIA 规范要求:
- 支持水平/垂直方向的箭头键导航
- Home/End 快速跳转
- Enter/Space 激活标签
- Delete/Backspace 删除标签
- 考虑了 RTL 布局
建议添加 ARIA 属性以提升可访问性:
<div ref={useComposeRef(ref, containerRef)} role="tablist" + aria-orientation={tabPositionTopOrBottom ? 'horizontal' : 'vertical'} className={classNames(`${prefixCls}-nav`, className)}
Line range hint
425-455: TabNode 焦点管理逻辑完善焦点状态管理的实现考虑周全:
- 通过
focus属性传递焦点状态- 正确处理键盘和鼠标交互的切换
- 实现了焦点状态的自动滚动
建议使用 useCallback 优化事件处理函数:
+const handleMouseDown = useCallback(() => { + setIsKeyboard(false); +}, []); +const handleMouseUp = useCallback(() => { + setIsKeyboard(true); +}, []); <TabNode ... - onMouseDown={() => { - setIsKeyboard(false); - }} - onMouseUp={() => { - setIsKeyboard(true); - }} + onMouseDown={handleMouseDown} + onMouseUp={handleMouseUp} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/TabNavList/index.tsx(5 hunks)
🔇 Additional comments (1)
src/TabNavList/index.tsx (1)
300-304: 焦点管理状态实现得当
焦点状态管理的实现采用了合理的方案:
- 使用
focusKey追踪当前焦点标签 - 使用
isKeyboard区分键盘和鼠标交互 - 通过
enabledTabs过滤出可用标签
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/TabNavList/TabNode.tsx (1)
11-23: 接口更新看起来不错，建议添加JSDoc注释新增的属性完全符合无障碍访问的需求，建议为新增的属性添加JSDoc注释以提高代码可维护性。
export interface TabNodeProps { + /** 标识当前标签是否获得焦点 */ focus: boolean; + /** 键盘事件处理函数 */ onKeyDown: React.KeyboardEventHandler; + /** 鼠标按下事件处理函数 */ onMouseDown: React.MouseEventHandler; + /** 鼠标释放事件处理函数 */ onMouseUp: React.MouseEventHandler; + /** 失去焦点事件处理函数 */ onBlur: React.FocusEventHandler;src/TabNavList/index.tsx (2)
314-402: 键盘事件处理逻辑建议重构虽然功能完整，包含了RTL支持和删除操作，但建议将代码重构为更小的函数以提高可维护性。
建议将代码重构如下：
+const handleArrowNavigation = (e: React.KeyboardEvent, code: string) => { + if (code === 'ArrowUp' || code === 'ArrowDown') { + e.preventDefault(); + } + + const isHorizontal = tabPositionTopOrBottom; + const isRTL = rtl && isHorizontal; + + switch (code) { + case 'ArrowLeft': + if (isHorizontal) onOffset(isRTL ? 1 : -1); + break; + case 'ArrowRight': + if (isHorizontal) onOffset(isRTL ? -1 : 1); + break; + case 'ArrowUp': + if (!isHorizontal) onOffset(-1); + break; + case 'ArrowDown': + if (!isHorizontal) onOffset(1); + break; + } +}; + +const handleHomeEnd = (e: React.KeyboardEvent, code: string) => { + e.preventDefault(); + setFocusKey(code === 'Home' ? firstEnabledTab : lastEnabledTab); +}; + +const handleRemoveTab = (e: React.KeyboardEvent) => { + const removeTab = tabs.find(tab => tab.key === focusKey); + const removable = getRemovable( + removeTab?.closable, + removeTab?.closeIcon, + editable, + removeTab?.disabled, + ); + + if (removable) { + e.preventDefault(); + e.stopPropagation(); + editable.onEdit('remove', { key: focusKey, event: e }); + + const removeIndex = enabledTabs.indexOf(focusKey); + onOffset(removeIndex === enabledTabs.length - 1 ? -1 : 1); + } +}; const handleKeyDown = (e: React.KeyboardEvent) => { const { code } = e; switch (code) { case 'ArrowLeft': case 'ArrowRight': case 'ArrowUp': case 'ArrowDown': - // 原有的方向键处理代码 + handleArrowNavigation(e, code); break; case 'Home': case 'End': - // 原有的Home/End处理代码 + handleHomeEnd(e, code); break; case 'Enter': case 'Space': e.preventDefault(); onTabClick(focusKey, e); break; case 'Backspace': case 'Delete': - // 原有的删除处理代码 + handleRemoveTab(e); break; } };
Line range hint
425-455: 建议优化焦点管理和滚动重置逻辑焦点管理的实现很全面，但滚动重置的逻辑可以更简洁。
建议重构如下：
-onFocus={() => { - if (isKeyboard) { - setFocusKey(key); - } - scrollToTab(key); - doLockAnimation(); - if (!tabsWrapperRef.current) { - return; - } - // Focus element will make scrollLeft change which we should reset back - if (!rtl) { - tabsWrapperRef.current.scrollLeft = 0; - } - tabsWrapperRef.current.scrollTop = 0; -}} +const resetScroll = () => { + const wrapper = tabsWrapperRef.current; + if (!wrapper) return; + + if (!rtl) wrapper.scrollLeft = 0; + wrapper.scrollTop = 0; +}; + +onFocus={() => { + if (isKeyboard) setFocusKey(key); + scrollToTab(key); + doLockAnimation(); + resetScroll(); +}}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/__snapshots__/index.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/TabNavList/TabNode.tsx(4 hunks)src/TabNavList/index.tsx(6 hunks)tests/accessibility.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/accessibility.test.tsx
🔇 Additional comments (6)
src/TabNavList/TabNode.tsx (3)
70-74: 焦点管理实现正确
useEffect的使用恰当，依赖项设置正确，能够有效管理标签页的焦点状态。
80-85: 无障碍属性设置完善
ARIA属性和tabIndex的设置符合WAI-ARIA规范，可以确保屏幕阅读器正确识别和朗读标签页的状态。
118-118: 删除按钮的焦点管理正确
tabIndex的设置确保只有当标签页处于激活状态时，删除按钮才能获得焦点，这是符合预期的行为。
src/TabNavList/index.tsx (3)
300-305: 键盘导航状态管理设计合理
focusKey和isKeyboard的状态管理清晰，enabledTabs的过滤逻辑正确处理了禁用标签的情况。
306-312: 标签页偏移计算优化得当
采用取模运算实现循环导航，代码简洁高效，完全符合之前的优化建议。
567-567: ARIA方向属性设置正确
aria-orientation属性根据标签页方向正确设置，有助于提升无障碍访问体验。
aojunhao123
force-pushed
the
a11y-keyboard
branch
from
a119a48 to
a9bde54
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/TabNavList/TabNode.tsx (1)
102-102: 建议优化屏幕阅读器反馈样式当前实现很好地支持了屏幕阅读器，但建议将 live region 的样式移至 CSS 文件：
+// 在 assets/index.less 中添加 +.rc-tabs-live-region { + position: absolute; + width: 1px; + height: 1px; + overflow: hidden; + padding: 0; + margin: -1px; + clip: rect(0, 0, 0, 0); +} -<div - aria-live="polite" - style={{ - position: 'absolute', - width: 1, - height: 1, - overflow: 'hidden', - padding: 0, - margin: -1, - clip: 'rect(0, 0, 0, 0)', - }} -> +<div aria-live="polite" className="rc-tabs-live-region">Also applies to: 113-128
src/TabNavList/index.tsx (1)
314-402: 建议添加键盘快捷键文档键盘事件处理实现完整，建议在组件文档中添加快捷键说明：
- 方向键：在标签间导航
- Home/End：跳转到第一个/最后一个标签
- Enter/Space：激活标签
- Backspace/Delete：删除标签（如果允许）
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/__snapshots__/index.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
src/TabNavList/TabNode.tsx(4 hunks)src/TabNavList/index.tsx(6 hunks)tests/accessibility.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/accessibility.test.tsx
🔇 Additional comments (6)
src/TabNavList/TabNode.tsx (3)
11-11: 接口更改增强了可访问性支持！
新增的属性很好地支持了键盘导航和焦点管理：
focus用于跟踪当前焦点状态tabCount和currentPosition用于屏幕阅读器反馈- 新增的事件处理器完整覆盖了键盘交互需求
Also applies to: 18-19, 21-25
72-78: 焦点管理实现符合最佳实践！
使用 useRef 和 useEffect 来管理焦点状态，实现了可控的焦点切换。
138-138: 删除按钮的焦点管理正确实现！
根据标签激活状态正确设置了删除按钮的 tabIndex。
src/TabNavList/index.tsx (3)
300-305: 键盘导航状态管理设计合理！
- 使用
focusKey追踪当前焦点标签 - 使用
isKeyboard区分键盘与鼠标交互 - 正确过滤出可用标签列表
306-312: 标签导航逻辑简洁有效！
使用取模运算实现了循环导航，代码简洁优雅。
569-569: 正确设置了 ARIA 属性！
根据标签位置正确设置了 aria-orientation 属性，增强了可访问性。
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/TabNavList/TabNode.tsx (1)
Line range hint
1-143: 建议添加键盘导航文档！组件的可访问性得到了显著提升，建议：
- 在组件文档中添加键盘导航说明
- 添加 ARIA 属性的使用说明
- 考虑添加自动化测试来验证键盘导航功能
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/TabNavList/TabNode.tsx(4 hunks)
🔇 Additional comments (4)
src/TabNavList/TabNode.tsx (4)
11-25: 接口更改增强了可访问性支持！
新增的属性很好地支持了键盘导航和焦点管理的需求：
focus用于控制焦点状态tabCount和currentPosition用于屏幕阅读器反馈- 事件处理器类型的完善提高了类型安全性
72-78: 焦点管理实现得当！
使用 useRef 和 useEffect 来管理焦点状态是一个很好的实践：
- 响应式地处理焦点变化
- 避免了不必要的重渲染
102-102:
需要重新考虑 tabIndex 的实现！
根据之前的评审意见，简单地设置 tabIndex={disabled ? null : active ? 0 : -1} 可能会导致 Tab 键导航的问题。当用户使用 Shift+Tab 返回时可能无法正确定位到当前的 TabNode。
建议修改为：
-tabIndex={disabled ? null : active ? 0 : -1}
+tabIndex={disabled ? null : 0}Likely invalid or redundant comment.
130-130:
移除按钮的 tabIndex 需要调整！
根据之前的评审意见，移除按钮的 tabIndex 设置需要考虑更复杂的场景：
- 当 Tab 处于激活状态时，用户应该能够通过第二次 Tab 键导航到移除按钮
- 这个行为不应该影响左右键导航
建议修改为：
-tabIndex={active ? 0 : -1}
+tabIndex={0}Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/TabNavList/index.tsx (4)
300-304: 建议使用 useCallback 优化状态管理建议将状态管理相关的函数使用 useCallback 包装以避免不必要的重渲染：
+const setFocusKeyCallback = useCallback((key: string) => { + setFocusKey(key); +}, []); +const setIsMouseCallback = useCallback((value: boolean) => { + setIsMouse(value); +}, []);
306-312: 建议添加键盘快捷键文档为了提高代码可维护性，建议在 onOffset 函数上方添加 JSDoc 注释，说明循环导航的实现逻辑。
+/** + * 处理标签页的循环导航 + * @param offset - 偏移量，正数向后移动，负数向前移动 + */ const onOffset = (offset: number) => {
Line range hint
433-457: 建议优化鼠标事件处理当前的鼠标事件处理可能在快速操作时触发不必要的状态更新。建议：
- 使用 debounce 优化鼠标事件处理
- 添加注释说明状态切换逻辑
+import debounce from 'lodash/debounce'; +// 使用 debounce 优化鼠标事件处理 +const debouncedSetIsMouse = debounce((value: boolean) => { + setIsMouse(value); +}, 100);
569-569: 建议增加更多无障碍属性为了进一步提高组件的可访问性，建议添加以下 ARIA 属性：
aria-label描述当前标签组aria-controls关联标签页与其内容<div ref={useComposeRef(ref, containerRef)} role="tablist" aria-orientation={tabPositionTopOrBottom ? 'horizontal' : 'vertical'} + aria-label={locale?.tabListLabel} + aria-controls={`${id}-panel`}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/TabNavList/index.tsx(6 hunks)
🔇 Additional comments (1)
src/TabNavList/index.tsx (1)
314-402: 🛠️ Refactor suggestion
建议增强键盘事件的错误处理
当前的键盘事件处理缺少对以下边界情况的处理：
- 没有可用标签页时的处理
- 删除最后一个标签页时的焦点处理
建议添加相应的错误处理：
const handleKeyDown = (e: React.KeyboardEvent) => {
+ if (enabledTabs.length === 0) {
+ return;
+ }Likely invalid or redundant comment.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #763 +/- ##
==========================================
+ Coverage 97.72% 98.96% +1.24%
==========================================
Files 18 18
Lines 702 774 +72
Branches 193 227 +34
==========================================
+ Hits 686 766 +80
+ Misses 16 8 -8 ☔ View full report in Codecov by Sentry. |
🚀 为 Tabs 组件添加焦点管理和键盘导航能力
📝 功能描述
为 Tabs 组件添加了焦点管理并增强了键盘导航能力，以提升组件的可访问性和用户体验。
主要特性
键盘导航说明
水平标签页:
垂直标签页:
录屏
Summary by CodeRabbit
新特性
样式
测试