feat: focus item when focusing menu #464
Conversation
Codecov Report
@@ Coverage Diff @@
## master #464 +/- ##
=======================================
Coverage 99.85% 99.85%
=======================================
Files 25 25
Lines 675 677 +2
Branches 175 178 +3
=======================================
+ Hits 674 676 +2
Misses 1 1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
src/Menu.tsx
Outdated
| if (shouldFocusIndex > 0) { | ||
| containerRef.current | ||
| ?.querySelectorAll('li') | ||
| [shouldFocusIndex]?.focus(options); |
There was a problem hiding this comment.
There was a problem hiding this comment.
通过控制 activeKey 实现和 dom 的 focus 感觉上有些差异，是否提供另一个方法比如 activate 来自动设置 activeKey 好些？focus 可以保留 dom 的 focus 行为
There was a problem hiding this comment.
Menu 和 Select 还是有差别，需要 focus 才能用键盘交互，Select 就没有 focus。
There was a problem hiding this comment.
主要现在的实现是始终 focus 第一个，是不是有用户回车选择后需要保持 active 项的需求（而不是每次打开重置回第一个），所以我感觉内部需要维护一个 activeItemIndex 的 state 状态。
There was a problem hiding this comment.
目前 active 有 activeKey 控制，可以加入判断。
|
和 @zombieJ 讨论了下，Dropdown 下面的 menu 和正常的导航 Menu 对 accessibility 的处理应当是不一样的，前者和 Select 类似，后者应该是 focus 实现的。目前看没什么太好的办法让这两种实现融合，最佳方案还是能把两种 Menu 分开比较好，但是对用户习惯改动会比较大。当前情况下的最优解还是用 focus 实现。 |
|
desc 里加点描述说明一下，方便以后好查看前因后果。 |
src/Menu.tsx
Outdated
| } | ||
| if (shouldFocusIndex > 0) { | ||
| containerRef.current | ||
| ?.querySelectorAll('li') |
There was a problem hiding this comment.
感觉这么查有点问题，如果我就是 MenuItem 里再加 li，你这个 Selector 的顺序和 findIndex 是不匹配的
之前通过 ref focus 只能聚焦到 ul 元素上，从用户角度看根本不知道有聚焦过这回事。这里修改成：
ref: ant-design/ant-design#35391 (comment)