-
Notifications
You must be signed in to change notification settings - Fork 273
fix(noticebar): the speed and duration attributes behave incorrectly in vertical mode #3252
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
base: feat_v3.x
Are you sure you want to change the base?
Conversation
Walkthrough本次变更主要围绕 NoticeBar 组件的纵向滚动逻辑进行了重构和增强,包括统一滚动动画时长计算、用 ref 替换局部变量以保证状态一致性、优化初始化和内容动态变更时的处理流程,并补充了相关的单元测试和文档描述。Demo 示例也同步支持了纵向内容的动态增删。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Demo
participant NoticeBar
User->>Demo: 点击“添加最后一项”按钮
Demo->>Demo: 更新 list state
Demo->>NoticeBar: 传递新的 list
NoticeBar->>NoticeBar: 重新计算高度与偏移,重置动画
User->>Demo: 点击“删除最后一项”按钮
Demo->>Demo: 更新 list state
Demo->>NoticeBar: 传递新的 list
NoticeBar->>NoticeBar: 重新计算高度与偏移,重置动画
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3252 +/- ##
=============================================
+ Coverage 87.59% 87.92% +0.33%
=============================================
Files 290 290
Lines 19102 19129 +27
Branches 2930 2954 +24
=============================================
+ Hits 16732 16819 +87
+ Misses 2365 2305 -60
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 3
🧹 Nitpick comments (9)
src/packages/noticebar/doc.en-US.md (1)
91-91
: 建议:统一英文文档中的标点此处标题使用了中文逗号 “,”,建议改为英文逗号 “,”,以保持英文文档的标点一致性。
- ### Vertical Scroll Custom Style, Dynamic Content Updates + ### Vertical Scroll Custom Style, Dynamic Content Updatessrc/packages/noticebar/demos/h5/demo11.tsx (1)
5-5
: 建议:组件命名与文件名对齐当前文件
demo11.tsx
中组件命名为Demo10
,易造成混淆,建议改为Demo11
。- const Demo10 = () => { + const Demo11 = () => {src/packages/noticebar/demos/taro/demo10.tsx (2)
52-60
: 建议添加删除操作的边界检查。删除功能应该检查列表是否为空,避免意外删除所有项目。
<Button size="small" onClick={() => { - setList((prev) => prev.slice(0, -1)) + setList((prev) => prev.length > 0 ? prev.slice(0, -1) : prev) }} + disabled={list.length === 0} > 删除最后一项 </Button>
44-47
: 动态添加实现可以优化。添加项目时使用更有意义的内容而不是简单的数字。
<Button size="small" onClick={() => { - setList((prev) => [...prev, `${prev.length + 1}`]) + setList((prev) => [...prev, `新增项目 ${prev.length + 1}`]) }} >src/packages/noticebar/demos/h5/demo10.tsx (1)
52-60
: 建议添加删除操作的边界检查。与 Taro 版本相同,删除功能应该检查列表是否为空。
<Button size="small" onClick={() => { - setList((prev) => prev.slice(0, -1)) + setList((prev) => prev.length > 0 ? prev.slice(0, -1) : prev) }} + disabled={list.length === 0} >src/packages/noticebar/__test__/noticebar.spec.tsx (1)
264-353
: 动态内容更新测试设计优秀测试全面覆盖了动态更新场景:
- 初始状态验证
- 轮播进行中的状态
- 内容更新后的重置行为
- 样式和结构的正确更新
测试逻辑清晰,断言准确。
建议在第269行的组件声明中添加类型注解以提高代码可读性:
- const TestComponent = () => { + const TestComponent: React.FC = () => {src/packages/noticebar/noticebar.taro.tsx (1)
331-353
: 状态更新使用函数式 setState 是最佳实践正确使用了函数式 setState 更新
childOffset
,确保了:
- 避免并发更新问题
- 遵循 React 的不可变性原则
- 基于最新状态值进行更新
逻辑正确处理了循环滚动时首尾元素的位置调整。
静态分析工具建议使用可选链,可以简化第383-386行的代码:
- _offset = - moveOffset + Number(activeRef.current === childCount - 1 && val / 2) + _offset = + moveOffset + Number(activeRef.current === childCount - 1 ? val / 2 : 0)src/packages/noticebar/noticebar.tsx (2)
375-393
: setStyle 函数实现正确函数正确处理了可选的 rect 参数,支持在初始化时传入测量结果,避免了重复测量。
建议使用条件运算符使代码更清晰(第385-386行):
- _offset = - moveOffset + Number(activeRef.current === childCount - 1 && val / 2) + _offset = + moveOffset + (activeRef.current === childCount - 1 ? val / 2 : 0)🧰 Tools
🪛 Biome (1.9.4)
[error] 383-386: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
409-416
: getActive 函数逻辑正确函数正确使用了
activeRef.current
获取当前活动索引。静态分析显示第415行(
pace
为 0 的情况)未被测试覆盖。虽然这是边界情况,建议在测试中覆盖此分支以提高代码质量。🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 415-415: src/packages/noticebar/noticebar.tsx#L415
Added line #L415 was not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/packages/noticebar/__test__/noticebar.spec.tsx
(3 hunks)src/packages/noticebar/demo.taro.tsx
(2 hunks)src/packages/noticebar/demo.tsx
(2 hunks)src/packages/noticebar/demos/h5/demo10.tsx
(3 hunks)src/packages/noticebar/demos/h5/demo11.tsx
(1 hunks)src/packages/noticebar/demos/taro/demo10.tsx
(4 hunks)src/packages/noticebar/doc.en-US.md
(1 hunks)src/packages/noticebar/doc.md
(1 hunks)src/packages/noticebar/doc.taro.md
(1 hunks)src/packages/noticebar/doc.zh-TW.md
(1 hunks)src/packages/noticebar/noticebar.taro.tsx
(13 hunks)src/packages/noticebar/noticebar.tsx
(14 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/packages/noticebar/noticebar.taro.tsx (1)
src/hooks/use-memo.ts (1)
useMemo
(8-24)
src/packages/noticebar/noticebar.tsx (2)
src/hooks/use-memo.ts (1)
useMemo
(8-24)src/utils/get-rect.ts (1)
getRect
(16-48)
🪛 Biome (1.9.4)
src/packages/noticebar/noticebar.taro.tsx
[error] 383-386: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/packages/noticebar/noticebar.tsx
[error] 383-386: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Check: codecov/patch
src/packages/noticebar/noticebar.tsx
[warning] 211-212: src/packages/noticebar/noticebar.tsx#L211-L212
Added lines #L211 - L212 were not covered by tests
[warning] 415-415: src/packages/noticebar/noticebar.tsx#L415
Added line #L415 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (15)
src/packages/noticebar/doc.md (1)
99-99
: 批准:更新标题以强调动态滚动内容该标题已从 “自定义右侧图标” 扩展为 “自定义右侧图标,动态变更滚动内容”,与 Demo11 和最新测试覆盖保持一致。
src/packages/noticebar/demos/h5/demo11.tsx (1)
19-19
: 确认:延迟展示时长更新
duration
从默认值更新为3000ms
,与文档和测试中的示例保持一致,符合预期。src/packages/noticebar/doc.taro.md (1)
99-99
: 批准:更新标题以强调动态滚动内容标题已从 “自定义右侧图标” 更新为 “自定义右侧图标,动态变更滚动内容”,与 Demo11、文档及测试保持一致。
src/packages/noticebar/demo.taro.tsx (1)
32-32
: 翻译字符串更新准确反映新功能。翻译更新正确地描述了新的动态内容变更功能,中英文表述都很准确。
Also applies to: 45-45
src/packages/noticebar/demo.tsx (1)
28-28
: 翻译更新与 Taro 版本保持一致。翻译字符串正确地反映了动态内容变更功能,与 Taro 版本保持一致性。
Also applies to: 41-41
src/packages/noticebar/demos/taro/demo10.tsx (1)
1-2
: 状态管理实现正确。正确引入了
useState
和相关组件,状态管理方式合适。Also applies to: 12-12
src/packages/noticebar/demos/h5/demo10.tsx (1)
20-20
: 时长调整改善用户体验。将 duration 从 1000ms 增加到 3000ms 为用户提供了更好的阅读体验。
src/packages/noticebar/__test__/noticebar.spec.tsx (3)
170-173
: 测试断言更新合理将断言从检查有样式改为检查无样式,正确验证了垂直列表模式下初始状态不应有内联动画样式,符合修复初始显示立即触发下一项的问题。
225-262
: 新增的容器高度计算测试覆盖全面测试正确验证了垂直模式下使用 children 时的容器高度计算逻辑。
(childCount + 1) * height
的公式确保了无缝滚动效果。
355-473
: 时序逻辑测试设计精准使用
vi.useFakeTimers()
精确控制时间流逝,有效验证了:
- 统一的滚动动画时间计算 (
scrollAnimationTimeMs
)- list 模式下的样式和内容轮换
- children 模式下的 transform 动画
测试准确捕捉了 PR 修复的核心问题:将 duration 作为暂停时间,而动画时间由 height/speed 计算得出。
src/packages/noticebar/noticebar.taro.tsx (5)
82-82
: 使用 useRef 管理活动索引是正确的选择将
active
局部变量替换为activeRef
,避免了闭包中的过期值问题,确保异步操作(如定时器回调)始终访问到最新的活动索引。
193-216
: 滚动动画时间计算逻辑完善
scrollAnimationTimeMs
的实现优点:
- 使用
useMemo
避免重复计算- 完善的边界条件处理(speed或height为0)
- 特殊处理小于1秒的情况,确保动画流畅
/4
因子的使用与回退机制合理这解决了原代码中
100 * speed
计算不明确的问题,统一了动画时序。
391-391
: 容器高度计算支持无缝滚动设置
height: ${Number(height) * (childCount + 1)}px
是实现垂直无缝滚动的关键,多出的一个高度确保了滚动循环的平滑过渡。
457-462
: 使用 memoized 值优化渲染性能
hasVerticalContent
的实现合理:
- 避免每次渲染时重复判断
- 逻辑清晰,正确处理了 children 和 list 两种数据源
这种优化模式值得在其他组件中推广。
464-467
: 容器引用回调处理得当使用
useCallback
创建稳定的回调函数,避免了不必要的重新渲染。通过isContainerReady
状态确保初始化时序正确,解决了组件挂载时的竞态条件。
@@ -96,7 +96,7 @@ import { NoticeBar } from '@nutui/nutui-react' | |||
|
|||
::: | |||
|
|||
### 縱嚮模式:自定義右側圖標 | |||
### 縱嚮模式:自定義右側圖標,動態變更滾動內容 |
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.
修复:繁体中文标题中的错别字
縱嚮模式
中的 “嚮” 为错字,应使用 “向”。
- ### 縱嚮模式:自定義右側圖標,動態變更滾動內容
+ ### 縱向模式:自定義右側圖標,動態變更滾動內容
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
### 縱嚮模式:自定義右側圖標,動態變更滾動內容 | |
### 縱向模式:自定義右側圖標,動態變更滾動內容 |
🤖 Prompt for AI Agents
In src/packages/noticebar/doc.zh-TW.md at line 99, the title contains a typo
where "縱嚮模式" uses the incorrect character "嚮". Replace "嚮" with the correct
character "向" to fix the typo, changing the phrase to "縱向模式".
import React from 'react' | ||
import { NoticeBar } from '@nutui/nutui-react' | ||
import React, { useState } from 'react' | ||
import { Button, NoticeBar, Space } from '@nutui/nutui-react' | ||
|
||
const Demo9 = () => { |
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.
🛠️ Refactor suggestion
组件命名不一致。
组件名为 Demo9
但文件名为 demo10.tsx
,这可能会造成混淆。
-const Demo9 = () => {
+const Demo10 = () => {
-export default Demo9
+export default Demo10
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const Demo9 = () => { | |
const Demo10 = () => { | |
// existing implementation | |
}; | |
export default Demo10; |
🤖 Prompt for AI Agents
In src/packages/noticebar/demos/h5/demo10.tsx at line 4, the component is named
Demo9, which is inconsistent with the file name demo10.tsx. Rename the component
from Demo9 to Demo10 to match the file name and avoid confusion.
|
||
// 滚动时间间隔 | ||
const time = | ||
height / speed / 4 < 1 | ||
? Number((height / speed / 4).toFixed(1)) * 1000 | ||
: ~~(height / speed / 4) * 1000 | ||
const scrollAnimationTimeMs = useMemo(() => { | ||
if (Number(speed) <= 0 || Number(height) <= 0) return 0 | ||
|
||
// 用于计算时间(秒到毫秒,并处理 <1 秒的情况) | ||
const calculateTimeInMs = (baseSeconds: number): number => { | ||
if (baseSeconds < 1) { | ||
// 例如 0.04s -> "0.0" -> 0ms. 0.05s -> "0.1" -> 100ms. | ||
return Number(baseSeconds.toFixed(1)) * 1000 | ||
} | ||
return Math.floor(baseSeconds) * 1000 | ||
} | ||
|
||
// 尝试使用 /4 因子计算 | ||
const timeWithFactor4 = calculateTimeInMs( | ||
Number(height) / Number(speed) / 4 | ||
) | ||
|
||
if (timeWithFactor4 === 0) { | ||
// 如果带 /4 因子的时间为0,则回退到不带 /4 因子的计算 | ||
return calculateTimeInMs(Number(height) / Number(speed)) | ||
} | ||
return timeWithFactor4 | ||
}, [height, speed]) | ||
|
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.
💡 Verification agent
🧩 Analysis chain
Web 版本实现与 Taro 版本保持一致
scrollAnimationTimeMs
的计算逻辑与 Taro 版本完全一致,确保了跨平台行为的统一性。
静态分析显示第211-212行未被测试覆盖。这些行处理了动画时间为0时的回退逻辑,建议补充相关测试用例。
运行以下脚本检查现有测试是否覆盖了边界情况:
🏁 Script executed:
#!/bin/bash
# 检查测试文件中是否有测试 speed 或 height 为极小值的情况
rg -A 5 -B 5 "speed.*[0-9]" src/packages/noticebar/__test__/noticebar.spec.tsx | grep -E "(speed|height).*[=:].*[0-5]\b"
Length of output: 432
请补充测试以覆盖「动画时间为0时的回退逻辑」
静态分析和现有测试结果显示,以下分支尚无覆盖:
- 当
speed
或height
为0
时,scrollAnimationTimeMs
应直接返回0
- 当
height/speed/4
计算后小于1s
(触发toFixed(1)
四舍五入),且结果为0
时,应进入不带/4
因子的回退计算分支
建议在 src/packages/noticebar/__test__/noticebar.spec.tsx
中新增至少三个测试用例,分别验证上述场景:
speed={0}
或height={0}
speed
和height
组合使得height/speed/4 < 1
且四舍五入后等于0
- 对比带
/4
因子与不带因子的计算结果是否符合预期(回退分支)
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 211-212: src/packages/noticebar/noticebar.tsx#L211-L212
Added lines #L211 - L212 were not covered by tests
🤖 Prompt for AI Agents
In src/packages/noticebar/__test__/noticebar.spec.tsx, add tests to cover the
scrollAnimationTimeMs calculation edge cases from lines 190 to 215 in
src/packages/noticebar/noticebar.tsx. Specifically, create tests where speed or
height is zero to verify the function returns 0 immediately, tests where
height/speed/4 results in a value less than 1 second that rounds to 0 to trigger
the fallback calculation without the /4 factor, and tests comparing the timing
results with and without the /4 factor to confirm the fallback logic works as
intended.
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
问题描述
2025-06-04.10.49.43.mov
问题原因
可以看到:
duration应该是停留时长,但此处却被设置为动画过渡时间
这里的100 * speed 意义不明,应该为duration + 动画过渡时间
关键问题代码:
可以看到一开始滚动就立刻调用showhorseLamp()去展示下一项,第一项没有停留。
解决方案:
☑️ 请求合并前的自查清单
Summary by CodeRabbit