-
Notifications
You must be signed in to change notification settings - Fork 267
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: 修改jd小程序高版本弹幕不滚动问题 #2612
fix: 修改jd小程序高版本弹幕不滚动问题 #2612
Conversation
Walkthrough此次更改涉及 Changes
Suggested labels
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 @@
## next #2612 +/- ##
=======================================
Coverage 82.95% 82.95%
=======================================
Files 219 219
Lines 17908 17908
Branches 2547 2547
=======================================
Hits 14856 14856
Misses 3047 3047
Partials 5 5 ☔ 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: 1
Outside diff range and nitpick comments (1)
src/packages/barrage/barrage.taro.tsx (1)
138-141
: 注意事件监听器的清理以防止内存泄漏在为
el
添加animationend
事件监听器后,没有在适当的时候移除监听器。虽然浏览器通常会在元素被移除时清理事件监听器,但为了保险起见,建议在回调函数中手动移除事件监听器,防止可能的内存泄漏。可以在回调函数中移除事件监听器:
const onAnimationEnd = () => { (barrageContainer.current as HTMLDivElement).removeChild(el) el.removeEventListener('animationend', onAnimationEnd) } el.addEventListener('animationend', onAnimationEnd)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/packages/barrage/barrage.taro.tsx (4 hunks)
- src/packages/barrage/demos/taro/demo1.tsx (1 hunks)
Additional comments not posted (1)
src/packages/barrage/demos/taro/demo1.tsx (1)
23-28
: 请确认新添加的interval
属性的影响新增的
interval={100}
属性可能会影响弹幕的滚动速度或显示频率。请确保:
- 这个值(100)是经过测试和验证的,能够解决高版本弹幕不滚动的问题。
- 考虑添加注释说明这个属性的作用和选择这个值的原因。
- 更新相关文档,说明这个新属性的用途和默认值。
为了验证这个改动的影响,可以运行以下脚本:
这将帮助我们了解是否需要在其他地方也添加
interval
属性。Verification successful
interval
属性添加已确认无其他影响通过检查代码库,确定此次在
demo1.tsx
中新增的interval={100}
属性仅影响该示例,不会对其他部分产生影响。Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查 Barrage 组件的其他使用情况,确保改动不会影响其他地方 # 测试:搜索 Barrage 组件的其他使用实例 rg --type typescript --type tsx 'Barrage' -g '!**/demo1.tsx'Length of output: 95
Script:
#!/bin/bash # 描述:检查 Barrage 组件的其他使用情况,确保改动不会影响其他地方 # 测试:搜索 Barrage 组件的其他使用实例 rg --type ts --type tsx 'Barrage' -g '!**/demo1.tsx'Length of output: 80
Script:
#!/bin/bash # 描述:检查 Barrage 组件的其他使用情况,确保改动不会影响其他地方 # 测试:搜索 Barrage 组件的其他使用实例,排除 demo1.tsx rg 'Barrage' -g '*.ts' -g '*.tsx' -g '!**/demo1.tsx'Length of output: 2133
const setStyle = async (el: HTMLElement, currentIndex: number) => { | ||
try { | ||
if (el) { | ||
const refe = await getRectByTaro(el) | ||
const width = refe.width | ||
const height = refe.height | ||
el.classList.add('move') | ||
const elScrollDuration = Math.round( | ||
(width / barrageCWidth.current) * duration | ||
) | ||
times.current[currentIndex] = elScrollDuration | ||
el.style.animationDuration = `${duration + elScrollDuration}ms` | ||
el.style.top = `${currentIndex * (height + gapY) + 20}px` | ||
el.style.width = `${width}px` | ||
} | ||
} catch (error) { | ||
console.log('异常自动流转到下一个', error) | ||
;(barrageContainer.current as HTMLDivElement).removeChild(el) | ||
} | ||
el.addEventListener('animationend', () => { | ||
;(barrageContainer.current as HTMLDivElement).removeChild(el) | ||
}) | ||
} |
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.
建议在 React 组件中避免直接操作 DOM
在 setStyle
函数中,直接对 DOM 元素进行了操作,例如使用 el.classList.add
和直接修改 el.style
。在 React 中,直接操作 DOM 可能导致组件状态和界面不同步,增加维护难度。建议使用 React 的状态和属性来控制组件的渲染,或者使用 ref
与 useEffect
相结合的方式安全地操作 DOM。
可以考虑将需要的样式和类名通过状态管理,并在渲染时使用 className
和 style
属性进行控制,这样可以保持 React 的声明式编程风格,提高代码的可读性和可维护性。
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新功能
Barrage
组件中新增了interval
属性,设置为 100,指定了弹幕效果的时间间隔。改进
InternalBarrage
组件的定时和动画处理,增强了代码结构和清晰度。