feat(swipe-action): 优化滑动操作组件#7051
Conversation
- 提取打开侧边栏逻辑到独立函数 - 增加焦点事件处理外部点击关闭 - 添加焦点时自动展开侧边栏
dosubot
Bot
added
size:M
📝 WalkthroughWalkthrough为 SwipeAction 组件新增内部 open(side) 统一展开逻辑，重构外部关闭监听以同时处理 touchstart 与 focusout，并在左右 actions 上添加 onFocus 以支持键盘/无障碍触发展开；补充对应测试。 变更SwipeAction 无障碍和交互改进
估计审查工作量🎯 3 (Moderate) | ⏱️ ~20 分钟 建议标签
建议审查者
诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the SwipeAction component by introducing a centralized open function and enhancing focus management. Key changes include adding a focusout listener to close the actions when focus leaves the component and implementing onFocus handlers on action containers to ensure they remain open when interacted with. Review feedback focused on improving the robustness of these changes by suggesting the use of x.goal instead of x.get() to avoid redundant animation calls and identifying a missing dependency (props.onClose) in a useEffect hook.
| useEffect(() => { | ||
| if (!props.closeOnTouchOutside) return | ||
| function handle(e: Event) { | ||
| if (x.get() === 0) { | ||
| return | ||
| const root = rootRef.current | ||
| if (!root) return | ||
| function onTouchOutside(e: Event) { | ||
| if (x.get() === 0) return | ||
| if (!root.contains(e.target as Node)) { | ||
| close() | ||
| } | ||
| const root = rootRef.current | ||
| if (root && !root.contains(e.target as Node)) { | ||
| } | ||
| function onFocusOutside(e: FocusEvent) { | ||
| if (x.get() === 0) return | ||
| if (!root.contains(e.relatedTarget as Node)) { | ||
| close() | ||
| } | ||
| } | ||
| document.addEventListener('touchstart', handle) | ||
| document.addEventListener('touchstart', onTouchOutside) | ||
| root.addEventListener('focusout', onFocusOutside) | ||
| return () => { | ||
| document.removeEventListener('touchstart', handle) | ||
| document.removeEventListener('touchstart', onTouchOutside) | ||
| root.removeEventListener('focusout', onFocusOutside) | ||
| } | ||
| }, [props.closeOnTouchOutside]) |
There was a problem hiding this comment.
useEffect 钩子在依赖数组中缺少 props.onClose。由于事件监听器调用了 close()，而 close() 内部使用了 props.onClose，如果 onClose 发生变化，监听器可能会捕获到旧的回调版本。此外，建议在监听器中使用 x.goal 代替 x.get()。x.goal 代表动画的目标值，这可以避免在动画过程中触发冗余的调用，使逻辑更加健壮。
useEffect(() => {
if (!props.closeOnTouchOutside) return
const root = rootRef.current
if (!root) return
function onTouchOutside(e: Event) {
if (x.goal === 0) return
if (!root.contains(e.target as Node)) {
close()
}
}
function onFocusOutside(e: FocusEvent) {
if (x.goal === 0) return
if (!root.contains(e.relatedTarget as Node)) {
close()
}
}
document.addEventListener('touchstart', onTouchOutside)
root.addEventListener('focusout', onFocusOutside)
return () => {
document.removeEventListener('touchstart', onTouchOutside)
root.removeEventListener('focusout', onFocusOutside)
}
}, [props.closeOnTouchOutside, props.onClose])
| onFocus={() => { | ||
| if (x.get() !== getLeftWidth()) open('left') | ||
| }} |
There was a problem hiding this comment.
建议使用 x.goal 代替 x.get() 来判断组件是否已经处于打开或正在打开的状态。这可以避免在动画执行过程中重复触发 open 调用以及 onActionsReveal 回调。
| onFocus={() => { | |
| if (x.get() !== getLeftWidth()) open('left') | |
| }} | |
| onFocus={() => { | |
| if (x.goal !== getLeftWidth()) open('left') | |
| }} |
| onFocus={() => { | ||
| if (x.get() !== -getRightWidth()) open('right') | ||
| }} |
There was a problem hiding this comment.
建议使用 x.goal 代替 x.get()。这可以确保一致性，并避免在同一个操作容器内的元素之间移动焦点时触发不必要的动画启动。
| onFocus={() => { | |
| if (x.get() !== -getRightWidth()) open('right') | |
| }} | |
| onFocus={() => { | |
| if (x.goal !== -getRightWidth()) open('right') | |
| }} |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/swipe-action/swipe-action.tsx`:
- Around line 180-199: The callbacks onTouchOutside and onFocusOutside capture
rootRef.current which TypeScript still treats as possibly null causing TS2531;
fix by reading and asserting the non-null root once before registering handlers
(e.g., const root = rootRef.current! after the early null-return) and use that
local non-null root inside onTouchOutside/onFocusOutside and when removing
listeners, ensuring the same root reference is used for
addEventListener/removeEventListener on document/root and avoiding nullable
access in those closures (referencing rootRef, root, onTouchOutside,
onFocusOutside, x.get(), close).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8c12453-6c17-4f54-90cf-9f2e1a0bb6d4
📒 Files selected for processing (1)
src/components/swipe-action/swipe-action.tsx
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7051 +/- ##
==========================================
+ Coverage 92.94% 92.97% +0.02%
==========================================
Files 337 337
Lines 7386 7398 +12
Branches 1868 1885 +17
==========================================
+ Hits 6865 6878 +13
+ Misses 485 484 -1
Partials 36 36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- 将 `x.get()` 替换为 `x.goal` 进行状态判断 - 修复焦点和触摸外部事件的条件检查
- 测试焦点在操作按钮上时打开滑动动作 - 测试焦点移出时关闭滑动动作
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/swipe-action/tests/swipe-action.test.tsx (1)
299-312: 💤 Low value可选：为测试失败时提供更清晰的错误信息。
当前代码使用类型断言（line 301
as HTMLElement）和非空断言（line 306、312 的!），如果元素未找到，测试失败信息可能不够明确。可以考虑添加显式的存在性断言以便更快定位问题：const rightButton = container.querySelector( `.${classPrefix}-actions-right button` ) expect(rightButton).not.toBeNull() act(() => { rightButton!.focus() }) const track = container.querySelector(`.${classPrefix}-track`) expect(track).not.toBeNull()不过鉴于这是测试代码且元素结构由组件保证，当前写法也是可接受的实践。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/swipe-action/tests/swipe-action.test.tsx` around lines 299 - 312, The test currently uses type/non-null assertions on DOM queries (rightButton, track, root) which can yield unclear failures; add explicit existence assertions before using them: after querying `rightButton`, `track`, and `root` (via container.querySelector with `.${classPrefix}-actions-right button`, `.${classPrefix}-track`, and `.${classPrefix}`) call expect(...).not.toBeNull() (or expect(...).toBeInTheDocument()) and then safely cast/use them (e.g., call focus or check styles) so failures clearly indicate which element was missing; keep the existing assertions for `onActionsReveal` unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/swipe-action/tests/swipe-action.test.tsx`:
- Around line 299-312: The test currently uses type/non-null assertions on DOM
queries (rightButton, track, root) which can yield unclear failures; add
explicit existence assertions before using them: after querying `rightButton`,
`track`, and `root` (via container.querySelector with
`.${classPrefix}-actions-right button`, `.${classPrefix}-track`, and
`.${classPrefix}`) call expect(...).not.toBeNull() (or
expect(...).toBeInTheDocument()) and then safely cast/use them (e.g., call focus
or check styles) so failures clearly indicate which element was missing; keep
the existing assertions for `onActionsReveal` unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 724bb398-4afd-418d-bf64-7a5941e91a75
📒 Files selected for processing (1)
src/components/swipe-action/tests/swipe-action.test.tsx
1 participant
close #6602 修复无障碍聚焦时的不正常打开导致无法关闭的问题
Summary by CodeRabbit
功能改进
测试