Conversation
dosubot
bot
added
the
size:L
|
😭 Deploy PR Preview 584c9be failed. Build logs
🤖 By surge-preview |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
📝 Walkthrough详细说明此PR为SwipeAction组件引入受控刷卡功能，包括演示页面的新样式布局和组件源码中的初始化逻辑改进，支持defaultSwiped和受控swiped状态模式。 变更
代码审查工作量🎯 3 (Moderate) | ⏱️ ~20 分钟 需要重点关注的区域：
可能相关的PR
建议标签
诗歌
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
Summary of ChangesHello @rayhomie, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 enhances the SwipeAction component by adding support for both controlled (swiped prop) and uncontrolled (defaultSwiped prop) modes. The implementation correctly handles the component's lifecycle to reflect the swiped state. My review focuses on improving the robustness of the asynchronous rendering logic and enhancing type safety, which will make the component more reliable and maintainable.
| setTimeout(() => { | ||
| this.initWidth((maxSwipe: any) => { | ||
| if (maxSwipe) { | ||
| const direction = initialSwiped === 'left' ? 'left' : 'right'; | ||
| this.setData({ | ||
| swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1), | ||
| swipedR: direction === 'right', | ||
| swipedL: direction === 'left', | ||
| }); | ||
| } | ||
| }); | ||
| }, 50); |
There was a problem hiding this comment.
The use of a fixed setTimeout(..., 50) to wait for component rendering before calculating widths is fragile and can lead to race conditions on slower devices or under heavy load. This could result in the swipe action not initializing to the correct defaultSwiped state. Please consider a more robust approach, such as using my.nextTick (if available on your target platform) or another mechanism that guarantees execution after the render cycle is complete.
| setTimeout(() => { | ||
| this.initWidth((maxSwipe: any) => { | ||
| if (maxSwipe) { | ||
| const direction = swiped === 'left' ? 'left' : 'right'; | ||
| this.setData({ | ||
| swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1), | ||
| swipedR: direction === 'right', | ||
| swipedL: direction === 'left', | ||
| tapTypeL: '', | ||
| tapTypeR: '', | ||
| }); | ||
| } | ||
| }); | ||
| }, 50); |
There was a problem hiding this comment.
Similar to the implementation in didMount, using a fixed setTimeout(..., 50) here is fragile and can lead to race conditions when the swiped prop is updated. This could cause the component to not open correctly. A more robust post-render callback mechanism like my.nextTick should be preferred to ensure reliability across different devices and load conditions.
| if (initialSwiped) { | ||
| // 需要等待按钮宽度设置完成后再初始化展开状态 | ||
| setTimeout(() => { | ||
| this.initWidth((maxSwipe: any) => { |
There was a problem hiding this comment.
The maxSwipe parameter is typed as any. For better type safety and code clarity, it should be explicitly typed as number, since it represents a width value obtained from initWidth.
| this.initWidth((maxSwipe: any) => { | |
| this.initWidth((maxSwipe: number) => { |
| // swiped 变为 true/'left'/'right',需要打开 | ||
| // 需要等待下一个事件循环,确保按钮宽度已经渲染 | ||
| setTimeout(() => { | ||
| this.initWidth((maxSwipe: any) => { |
There was a problem hiding this comment.
The maxSwipe parameter is typed as any. It should be typed as number to improve type safety and make the code easier to understand, as it represents a width value.
| this.initWidth((maxSwipe: any) => { | |
| this.initWidth((maxSwipe: number) => { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/SwipeAction/index.ts (1)
123-173: 定时器清理缺失和条件逻辑需要验证
Line 144 的
setTimeout同样缺少清理机制。didMount(Line 109) 和didUpdate(Line 144) 中的定时器都未保存到全局的myTimeOut变量，且组件中不存在detached生命周期方法来清理这些定时器。当组件卸载时，如果 50ms 的定时器仍在挂起状态，会导致内存泄漏。建议在定时器分配时保存 ID 并在组件卸载时清除。Lines 131-172 的条件分支逻辑确实存在问题。当
swipedChanged为true时，会进入 lines 132-158 的分支，导致 lines 159-167 的else if分支被跳过。这意味着如果swiped和elasticity同时变化，elasticity 导致的状态重置 (Lines 161-167) 会被意外跳过，虽然 Line 171 的inertiaWidth更新仍会执行。请确认这是否为预期设计，还是应该在swipedChanged分支中也处理elasticity变化。
🧹 Nitpick comments (2)
src/SwipeAction/index.ts (2)
109-109: 建议使用常量替代魔法数字50ms 的延迟时间在代码中出现两次 (Lines 109, 144, 以及 Line 120 和 157)，应该提取为具名常量以提高可读性和可维护性。
+const SWIPE_INIT_DELAY = 50; // 等待按钮宽度计算完成的延迟时间 + Component( transformOptions({ // ... didMount() { // ... if (initialSwiped) { setTimeout(() => { // ... - }, 50); + }, SWIPE_INIT_DELAY); } }, didUpdate(prevProp) { // ... if (swipedChanged) { // ... } else { setTimeout(() => { // ... - }, 50); + }, SWIPE_INIT_DELAY); } } },Also applies to: 144-144
110-119: 建议添加错误处理在
initWidth的回调中，如果maxSwipe为假值（undefined、null、0等），当前代码会静默失败。虽然有if (maxSwipe)检查，但建议添加日志或警告，帮助开发者排查按钮宽度计算失败的问题。this.initWidth((maxSwipe: any) => { if (maxSwipe) { const direction = swiped === 'left' ? 'left' : 'right'; this.setData({ swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1), swipedR: direction === 'right', swipedL: direction === 'left', tapTypeL: '', tapTypeR: '', }); + } else { + console.warn('[SwipeAction] 无法获取按钮宽度，初始化滑动状态失败'); } });Also applies to: 145-156
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
demo/pages/SwipeAction/index.axml(1 hunks)demo/pages/SwipeAction/index.less(1 hunks)demo/pages/SwipeAction/index.ts(2 hunks)src/SwipeAction/index.ts(2 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). (3)
- GitHub Check: preview
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
demo/pages/SwipeAction/index.less (1)
62-71: 样式实现合理为演示用例添加的控制按钮样式结构清晰，使用了项目现有的 LESS 变量和主题系统，实现符合预期。
demo/pages/SwipeAction/index.ts (2)
3-3: 演示代码实现正确添加
controlledSwipe数据属性用于演示受控模式，实现简洁明了。
103-108: 事件处理逻辑清晰
onControlSwipe方法正确地从事件对象中读取方向参数并更新状态，符合小程序的事件处理模式。demo/pages/SwipeAction/index.axml (1)
333-382: 演示用例完整新增的演示部分清晰地展示了
defaultSwiped非受控模式和swiped受控模式的用法，有助于开发者理解两种使用模式的区别。
| didMount() { | ||
| const { defaultSwiped, elasticity } = this.getProps(); | ||
| this.setButtonItemWidth(); | ||
| const { defaultSwiped, swiped, elasticity } = this.getProps(); | ||
| this.setData({ inertiaWidth: !isOldVersion && elasticity ? 20 : 0 }); | ||
| if (defaultSwiped) { | ||
| this.initWidth((maxSwipe: any) => { | ||
| maxSwipe && | ||
| this.setData({ | ||
| swipeX: (maxSwipe + 0.01) * (defaultSwiped === 'right' ? -1 : 1), | ||
| swipedR: defaultSwiped === 'right', | ||
| swipedL: defaultSwiped === 'left', | ||
| }); | ||
| }); | ||
|
|
||
| // 优先使用 swiped 属性(受控),其次使用 defaultSwiped(非受控) | ||
| const initialSwiped = swiped || defaultSwiped; | ||
|
|
||
| // 先设置按钮宽度,然后在回调中处理初始展开状态 | ||
| this.setButtonItemWidth(); | ||
|
|
||
| if (initialSwiped) { | ||
| // 需要等待按钮宽度设置完成后再初始化展开状态 | ||
| setTimeout(() => { | ||
| this.initWidth((maxSwipe: any) => { | ||
| if (maxSwipe) { | ||
| const direction = initialSwiped === 'left' ? 'left' : 'right'; | ||
| this.setData({ | ||
| swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1), | ||
| swipedR: direction === 'right', | ||
| swipedL: direction === 'left', | ||
| }); | ||
| } | ||
| }); | ||
| }, 50); | ||
| } | ||
| }, |
There was a problem hiding this comment.
缺少定时器清理可能导致状态更新异常
didMount 中的 setTimeout (Line 109) 没有保存返回的 timer ID，也没有在组件卸载或更新时清理。如果组件在 50ms 内被销毁或 swiped 属性再次变化，可能导致过期的状态更新。
建议将 timer ID 保存到实例变量，并在 didUnmount 和相关更新逻辑中清理：
+let initTimer = null;
+
Component(
transformOptions({
props: SwipeActionDefaultProps,
didMount() {
const { defaultSwiped, swiped, elasticity } = this.getProps();
this.setData({ inertiaWidth: !isOldVersion && elasticity ? 20 : 0 });
const initialSwiped = swiped || defaultSwiped;
this.setButtonItemWidth();
if (initialSwiped) {
- setTimeout(() => {
+ initTimer = setTimeout(() => {
this.initWidth((maxSwipe: any) => {
if (maxSwipe) {
const direction = initialSwiped === 'left' ? 'left' : 'right';
this.setData({
swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1),
swipedR: direction === 'right',
swipedL: direction === 'left',
});
}
});
}, 50);
}
},
+ didUnmount() {
+ if (initTimer) {
+ clearTimeout(initTimer);
+ initTimer = null;
+ }
+ },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/SwipeAction/index.ts around lines 97 to 122, the setTimeout started at
line 109 isn’t storing its timer ID and therefore isn’t cleared on unmount or
when props change; save the timeout ID to an instance property (e.g.,
this._initTimeout), clear it with clearTimeout(this._initTimeout) and null it in
didUnmount, and also clear any existing timeout before creating a new one when
reacting to prop updates (like swiped changes) to prevent stale callbacks from
updating state after the component is destroyed or reconfigured.
| this.initWidth((maxSwipe: any) => { | ||
| if (maxSwipe) { | ||
| const direction = initialSwiped === 'left' ? 'left' : 'right'; | ||
| this.setData({ | ||
| swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1), | ||
| swipedR: direction === 'right', | ||
| swipedL: direction === 'left', | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
重复的初始化逻辑应该提取为独立方法
Lines 110-119 (在 didMount 中) 和 Lines 145-156 (在 didUpdate 中) 包含几乎完全相同的初始化逻辑。这违反了 DRY 原则，增加了维护成本和出错风险。
建议提取为独立的辅助方法：
+ applySwipedState(swiped, delay = 50) {
+ setTimeout(() => {
+ this.initWidth((maxSwipe: any) => {
+ if (maxSwipe) {
+ const direction = swiped === 'left' ? 'left' : 'right';
+ this.setData({
+ swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1),
+ swipedR: direction === 'right',
+ swipedL: direction === 'left',
+ tapTypeL: '',
+ tapTypeR: '',
+ });
+ }
+ });
+ }, delay);
+ },
didMount() {
const { defaultSwiped, swiped, elasticity } = this.getProps();
this.setData({ inertiaWidth: !isOldVersion && elasticity ? 20 : 0 });
const initialSwiped = swiped || defaultSwiped;
this.setButtonItemWidth();
if (initialSwiped) {
- setTimeout(() => {
- this.initWidth((maxSwipe: any) => {
- if (maxSwipe) {
- const direction = initialSwiped === 'left' ? 'left' : 'right';
- this.setData({
- swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1),
- swipedR: direction === 'right',
- swipedL: direction === 'left',
- });
- }
- });
- }, 50);
+ this.applySwipedState(initialSwiped);
}
},
didUpdate(prevProp) {
// ... 省略其他代码
if (swipedChanged) {
if (!swiped) {
// ... 重置逻辑
} else {
- setTimeout(() => {
- this.initWidth((maxSwipe: any) => {
- if (maxSwipe) {
- const direction = swiped === 'left' ? 'left' : 'right';
- this.setData({
- swipeX: (maxSwipe + 0.01) * (direction === 'right' ? -1 : 1),
- swipedR: direction === 'right',
- swipedL: direction === 'left',
- tapTypeL: '',
- tapTypeR: '',
- });
- }
- });
- }, 50);
+ this.applySwipedState(swiped);
}
}
},Also applies to: 145-156
🤖 Prompt for AI Agents
In src/SwipeAction/index.ts around lines 110-119 and 145-156, the same
initialization code that reads maxSwipe and sets swipeX, swipedR, swipedL is
duplicated; extract that logic into a single private/helper method (e.g.,
initSwipeState(initialSwiped?: string)) that calls this.initWidth and in its
callback computes direction, swipeX and sets the data, then replace the
duplicated blocks in didMount and didUpdate with calls to this new helper
(ensure correct this binding and preserve the same behavior when initialSwiped
is undefined).
1 participant
Summary by CodeRabbit
发版说明
✏️ Tip: You can customize this high-level summary in your review settings.