feat(Video): web h5 下支持 ref 调用 #2852
Conversation
Walkthrough此次更改涉及多个文件，主要更新了视频组件的功能和文档。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant VideoComponent
participant VideoElement
User->>VideoComponent: 调用 play()
VideoComponent->>VideoElement: 播放视频
User->>VideoComponent: 调用 pause()
VideoComponent->>VideoElement: 暂停视频
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2852 +/- ##
=======================================
Coverage 84.02% 84.02%
=======================================
Files 220 220
Lines 17907 17917 +10
Branches 2625 2628 +3
=======================================
+ Hits 15046 15055 +9
- Misses 2856 2857 +1
Partials 5 5 ☔ View full report in Codecov by Sentry. |
![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
🧹 Outside diff range and nitpick comments (2)
src/packages/video/demos/h5/demo2.tsx (1)
18-26: 建议优化 ref 控制示例的实现方式当前使用
setTimeout来演示 ref 的控制功能可能不是最佳选择，因为：
- 定时器可能会在组件卸载后继续执行
- 硬编码的延迟时间不够灵活
建议改为使用按钮触发的方式：
- setTimeout(() => { - rootRef?.current?.pause() - }, 2000) - - setTimeout(() => { - rootRef?.current?.play() - }, 4000) + const handlePause = () => { + rootRef?.current?.pause() + } + + const handlePlay = () => { + rootRef?.current?.play() + } + + return ( + <> + <div> + <button onClick={handlePause}>暂停</button> + <button onClick={handlePlay}>播放</button> + </div> + <Cell>src/packages/video/doc.md (1)
99-104: 建议补充 Ref 方法的详细说明当前的 Ref 方法文档虽然简洁明了，但建议补充以下内容：
- 方法的返回值说明
- 使用示例代码
- 可能的异常情况处理
建议扩展文档内容如下：
### Ref | 方法名 | 说明 | 参数 | | --- | --- | --- | -| play | 播放 | `-` | -| pause | 暂停 | `-` | +| play | 播放视频。返回一个 Promise，当视频开始播放时 resolve。 | `-` | +| pause | 暂停视频。返回一个 Promise，当视频暂停时 resolve。 | `-` | + +```tsx +// 示例代码 +const videoRef = useRef<HTMLVideoElement>(null) + +// 播放视频 +const handlePlay = async () => { + try { + await videoRef.current?.play() + console.log('视频开始播放') + } catch (error) { + console.error('播放失败:', error) + } +} + +// 暂停视频 +const handlePause = async () => { + try { + await videoRef.current?.pause() + console.log('视频已暂停') + } catch (error) { + console.error('暂停失败:', error) + } +} +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
src/packages/video/demo.tsx(3 hunks)src/packages/video/demos/h5/demo2.tsx(2 hunks)src/packages/video/doc.en-US.md(1 hunks)src/packages/video/doc.md(1 hunks)src/packages/video/doc.zh-TW.md(1 hunks)src/packages/video/video.tsx(4 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/video/video.tsx
[warning] 98-99: src/packages/video/video.tsx#L98-L99
Added lines #L98 - L99 were not covered by tests
[warning] 101-102: src/packages/video/video.tsx#L101-L102
Added lines #L101 - L102 were not covered by tests
[warning] 105-106: src/packages/video/video.tsx#L105-L106
Added lines #L105 - L106 were not covered by tests
🔇 Additional comments (7)
src/packages/video/demos/h5/demo2.tsx (1)
31-31: ref 属性的使用正确
ref 属性的传递实现正确，能够正常获取到视频元素的引用。
src/packages/video/demo.tsx (1)
16-16: 翻译更新准确反映了新功能
各语言版本的翻译更新准确地描述了新增的 ref 控制功能，保持了良好的一致性。
Also applies to: 25-25, 34-34
src/packages/video/doc.zh-TW.md (1)
98-104: 文档结构清晰，内容完整！
新增的 Ref 部分很好地说明了 play 和 pause 方法的用法。
src/packages/video/doc.en-US.md (1)
98-104: 英文文档与中文文档保持一致！
Ref 部分的翻译准确，与中文版本内容一致。
src/packages/video/video.tsx (3)
39-42: 类型定义清晰明确！
VideoRef 类型定义简洁且符合组件需求。
45-49: 组件类型定义合理！
使用 React.forwardRef 并正确处理了类型继承关系，通过 Omit 避免了事件处理器的类型冲突。
104-107: useImperativeHandle 使用正确！
正确使用 useImperativeHandle 暴露必要的方法。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 105-106: src/packages/video/video.tsx#L105-L106
Added lines #L105 - L106 were not covered by tests
| const pause = () => { | ||
| rootRef?.current?.pause() | ||
| } | ||
| const play = () => { | ||
| rootRef?.current?.play() | ||
| } | ||
|
|
||
| React.useImperativeHandle(ref, () => ({ | ||
| pause, | ||
| play, | ||
| })) |
There was a problem hiding this comment.
需要添加单元测试！
新增的 ref 方法缺少测试覆盖，建议添加相应的单元测试用例。
建议添加以下测试场景：
- 测试
play方法调用 - 测试
pause方法调用 - 测试错误处理场景
需要我帮您生成测试用例代码吗？
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 98-99: src/packages/video/video.tsx#L98-L99
Added lines #L98 - L99 were not covered by tests
[warning] 101-102: src/packages/video/video.tsx#L101-L102
Added lines #L101 - L102 were not covered by tests
[warning] 105-106: src/packages/video/video.tsx#L105-L106
Added lines #L105 - L106 were not covered by tests
| const pause = () => { | ||
| rootRef?.current?.pause() | ||
| } | ||
| const play = () => { | ||
| rootRef?.current?.play() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议增加错误处理！
play 和 pause 方法应该添加错误处理，因为 HTMLVideoElement 的这些方法会返回 Promise。
建议修改实现如下：
- const pause = () => {
- rootRef?.current?.pause()
- }
- const play = () => {
- rootRef?.current?.play()
- }
+ const pause = async () => {
+ try {
+ await rootRef?.current?.pause()
+ } catch (error) {
+ console.error('视频暂停失败:', error)
+ }
+ }
+ const play = async () => {
+ try {
+ await rootRef?.current?.play()
+ } catch (error) {
+ console.error('视频播放失败:', error)
+ }
+ }📝 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 pause = () => { | |
| rootRef?.current?.pause() | |
| } | |
| const play = () => { | |
| rootRef?.current?.play() | |
| } | |
| const pause = async () => { | |
| try { | |
| await rootRef?.current?.pause() | |
| } catch (error) { | |
| console.error('视频暂停失败:', error) | |
| } | |
| } | |
| const play = async () => { | |
| try { | |
| await rootRef?.current?.play() | |
| } catch (error) { | |
| console.error('视频播放失败:', error) | |
| } | |
| } |
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 98-99: src/packages/video/video.tsx#L98-L99
Added lines #L98 - L99 were not covered by tests
[warning] 101-102: src/packages/video/video.tsx#L101-L102
Added lines #L101 - L102 were not covered by tests
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/packages/video/demos/h5/demo2.tsx (2)
1-1: 建议使用更严格的类型定义建议为
source状态添加明确的接口定义，以提高代码的可维护性和类型安全性。+interface VideoSource { + src: string; + type: string; +} -const [source] = useState({ +const [source] = useState<VideoSource>({ src: 'https://storage.360buyimg.com/nutui/video/video_NutUI.mp4', type: 'video/mp4', })Also applies to: 5-5
31-31: 建议添加注释说明 ref 的用途为了提高代码可读性，建议添加注释说明 ref 的作用和使用场景。
<Video + // ref 用于控制视频的播放和暂停 ref={rootRef}src/packages/video/__tests__/video.spec.tsx (1)
31-70: 建议增加更多测试场景当前测试用例只覆盖了视频暂停功能，建议添加以下测试场景：
- 测试 play() 方法
- 测试 ref 为 null 时的错误处理
- 测试视频加载失败的情况
需要我帮您生成这些额外的测试用例吗？
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/packages/video/__tests__/video.spec.tsx(2 hunks)src/packages/video/demos/h5/demo2.tsx(2 hunks)
🔇 Additional comments (1)
src/packages/video/__tests__/video.spec.tsx (1)
2-2: 导入声明看起来没有问题！
新增的 fireEvent 和 Button 导入对于测试视频暂停功能是必要的。
Also applies to: 6-6
| const rootRef = useRef<HTMLVideoElement>(null) | ||
| setTimeout(() => { | ||
| rootRef?.current?.pause() | ||
| }, 2000) | ||
|
|
||
| setTimeout(() => { | ||
| rootRef?.current?.play() | ||
| }, 4000) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议优化定时器实现
当前实现存在以下几个问题：
- 没有清理定时器
- 硬编码的时间值不易维护
- 缺少错误处理
建议按照以下方式重构：
+const PAUSE_DELAY = 2000;
+const PLAY_DELAY = 4000;
const rootRef = useRef<HTMLVideoElement>(null);
+
+useEffect(() => {
+ const pauseTimer = setTimeout(() => {
+ try {
+ rootRef?.current?.pause();
+ } catch (error) {
+ console.error('视频暂停失败:', error);
+ }
+ }, PAUSE_DELAY);
-setTimeout(() => {
- rootRef?.current?.pause()
-}, 2000)
+ const playTimer = setTimeout(() => {
+ try {
+ rootRef?.current?.play();
+ } catch (error) {
+ console.error('视频播放失败:', error);
+ }
+ }, PLAY_DELAY);
-setTimeout(() => {
- rootRef?.current?.play()
-}, 4000)
+ return () => {
+ clearTimeout(pauseTimer);
+ clearTimeout(playTimer);
+ };
+}, []);📝 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 rootRef = useRef<HTMLVideoElement>(null) | |
| setTimeout(() => { | |
| rootRef?.current?.pause() | |
| }, 2000) | |
| setTimeout(() => { | |
| rootRef?.current?.play() | |
| }, 4000) | |
| const PAUSE_DELAY = 2000; | |
| const PLAY_DELAY = 4000; | |
| const rootRef = useRef<HTMLVideoElement>(null); | |
| useEffect(() => { | |
| const pauseTimer = setTimeout(() => { | |
| try { | |
| rootRef?.current?.pause(); | |
| } catch (error) { | |
| console.error('视频暂停失败:', error); | |
| } | |
| }, PAUSE_DELAY); | |
| const playTimer = setTimeout(() => { | |
| try { | |
| rootRef?.current?.play(); | |
| } catch (error) { | |
| console.error('视频播放失败:', error); | |
| } | |
| }, PLAY_DELAY); | |
| return () => { | |
| clearTimeout(pauseTimer); | |
| clearTimeout(playTimer); | |
| }; | |
| }, []); |
| test('video props', () => { | ||
| const pause = vi.fn() | ||
| const App = () => { | ||
| const source = { | ||
| src: 'xxx.mp4', | ||
| type: 'video/mp4', | ||
| } | ||
| const itemRef = React.useRef<HTMLVideoElement>(null) | ||
| const options = { | ||
| controls: true, | ||
| autoplay: true, | ||
| playsinline: true, | ||
| loop: true, | ||
| poster: | ||
| 'https://img12.360buyimg.com/ling/s345x208_jfs/t1/168105/33/8417/54825/603df06dEfcddc4cb/21f9f5d0a1b3dad4.jpg.webp', | ||
| } | ||
|
|
||
| return ( | ||
| <> | ||
| <Video | ||
| ref={itemRef} | ||
| source={source} | ||
| options={options} | ||
| onPause={pause} | ||
| /> | ||
| <Button | ||
| size="small" | ||
| data-testid="emit-click" | ||
| onClick={() => itemRef?.current?.pause()} | ||
| > | ||
| 暂停一下 | ||
| </Button> | ||
| </> | ||
| ) | ||
| } | ||
|
|
||
| const { getByTestId } = render(<App />) | ||
| fireEvent.click(getByTestId('emit-click')) | ||
| expect(pause).toBeCalled() | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议完善测试用例的健壮性
当前的测试用例可以通过以下几点来增强：
- 需要在测试后清理 mock 函数
- 应该验证 video ref 是否正确获取到了 video 元素
- 测试描述可以更具体，说明测试目的
建议按照以下方式修改：
-test('video props', () => {
+test('视频组件应该正确响应 ref.pause() 调用', () => {
const pause = vi.fn()
+ // 确保在每个测试后清理 mock
+ afterEach(() => {
+ pause.mockClear()
+ })
const App = () => {
// ... existing code ...
}
const { getByTestId } = render(<App />)
+ const videoElement = itemRef.current
+ expect(videoElement).not.toBeNull()
fireEvent.click(getByTestId('emit-click'))
expect(pause).toBeCalled()
+ expect(pause).toHaveBeenCalledTimes(1)
})Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/packages/empty/utils.ts (1)
1-1: 建议添加类型导出为了提高代码的可维护性，建议将
EmptyAction类型也导出。-import { EmptyAction } from './types' +export { EmptyAction } from './types'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/packages/empty/utils.ts(1 hunks)src/packages/video/__tests__/video.spec.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/packages/video/tests/video.spec.tsx
src/packages/empty/utils.ts
Outdated
| export const getButtonType = (actions: Array<EmptyAction>, index: number) => { | ||
| if (!actions || actions.length === 0) return 'default' | ||
| const action = actions[index] | ||
| if (!actions || actions.length === 0) return 'default' | ||
| if (action.type) return action.type | ||
| return actions.length > 1 && index === 0 ? 'default' : 'primary' | ||
| actions.length > 1 && index === 0 ? 'default' : 'primary' | ||
| } |
There was a problem hiding this comment.
缺少 return 语句导致函数返回 undefined
函数在某些情况下会返回 undefined，这可能导致意外行为。同时，函数的防御性编程可以更强。
建议按照以下方式修改代码：
export const getButtonType = (actions: Array<EmptyAction>, index: number) => {
+ if (!actions?.length) return 'default'
const action = actions[index]
- if (!actions || actions.length === 0) return 'default'
+ if (!action) return 'default'
if (action.type) return action.type
- actions.length > 1 && index === 0 ? 'default' : 'primary'
+ return actions.length > 1 && index === 0 ? 'default' : 'primary'
}修改说明：
- 修复了缺少 return 语句的问题
- 增加了对
action是否存在的检查 - 优化了空数组检查的位置
- 使用可选链操作符简化了代码
📝 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.
| export const getButtonType = (actions: Array<EmptyAction>, index: number) => { | |
| if (!actions || actions.length === 0) return 'default' | |
| const action = actions[index] | |
| if (!actions || actions.length === 0) return 'default' | |
| if (action.type) return action.type | |
| return actions.length > 1 && index === 0 ? 'default' : 'primary' | |
| actions.length > 1 && index === 0 ? 'default' : 'primary' | |
| } | |
| export const getButtonType = (actions: Array<EmptyAction>, index: number) => { | |
| if (!actions?.length) return 'default' | |
| const action = actions[index] | |
| if (!action) return 'default' | |
| if (action.type) return action.type | |
| return actions.length > 1 && index === 0 ? 'default' : 'primary' | |
| } |
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
新特性
play和pause，支持外部控制视频播放。文档