-
Notifications
You must be signed in to change notification settings - Fork 263
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
feat(video): cross-platform compatibility #2720
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Walkthrough该拉取请求包含对多个文件的更改,主要涉及 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 @@
## V3.0 #2720 +/- ##
=======================================
Coverage 83.95% 83.95%
=======================================
Files 221 221
Lines 17842 17842
Branches 2675 2675
=======================================
Hits 14980 14980
Misses 2857 2857
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: 3
🧹 Outside diff range and nitpick comments (13)
src/packages/video/demos/taro/demo1.tsx (2)
Line range hint
6-10
: 移除未使用的 setSource移除未使用的 setSource 是正确的,因为 source 在组件中是只读的。但是建议考虑以下优化:
- const [source] = useState({ + const source = { src: 'https://storage.360buyimg.com/nutui/video/video_NutUI.mp4', type: 'video/mp4', - }) + }由于 source 是静态的且不需要更新,可以直接定义为常量,无需使用 useState。
Line range hint
13-15
: 建议改进事件处理函数的类型定义事件处理函数使用 any 类型不够严谨,建议定义具体的事件类型。
- const play = (elm: any) => console.log('play', elm) - const pause = (elm: any) => console.log('pause', elm) - const playend = (elm: any) => console.log('playend', elm) + const play = (event: React.SyntheticEvent<HTMLVideoElement>) => console.log('play', event) + const pause = (event: React.SyntheticEvent<HTMLVideoElement>) => console.log('pause', event) + const playend = (event: React.SyntheticEvent<HTMLVideoElement>) => console.log('playend', event)src/packages/video/demos/taro/demo3.tsx (1)
3-3
: 导入语句位置调整建议建议将工具函数的导入语句与组件导入语句组合在一起,以保持导入部分的整洁性。
import React, { useState } from 'react' import { Cell, Video } from '@nutui/nutui-react-taro' -import pxTransform from '@/utils/px-transform' +import { pxTransform } from '@/utils/px-transform'src/packages/video/demos/taro/demo5.tsx (2)
Line range hint
6-10
: 优化:使用常量替代状态变量由于 source 对象在组件生命周期内不会改变,建议将其改为常量声明,无需使用 useState。
- const [source] = useState({ + const source = { src: 'https://storage.360buyimg.com/nutui/video/video_NutUI.mp4', type: 'video/mp4', - }) + }
26-26
: 跨平台兼容性改进使用
pxTransform
工具函数处理高度值是一个很好的做法,这样可以确保在不同平台上保持一致的显示效果。建议在其他需要单位转换的地方也采用这种方式。src/packages/video/demos/taro/demo2.tsx (1)
27-27
: 优化了跨平台兼容性使用
pxTransform
替换硬编码的像素值是个很好的改进,这样可以确保在不同平台上有更好的显示效果。建议在其他固定尺寸的地方也采用类似的处理方式,以确保整体的跨平台一致性。
src/packages/video/demos/taro/demo6.tsx (2)
Line range hint
6-10
: 简化状态管理,移除未使用的 setter通过移除未使用的 setSource,代码变得更加简洁。但建议考虑使用 const 声明来更好地表达这是一个不可变的配置对象。
建议修改如下:
- const [source] = useState({ + const source = { src: 'https://storage.360buyimg.com/nutui/video/video_NutUI.mp4', type: 'video/mp4', - }) + }
23-32
: 优化样式属性的跨平台兼容性使用 pxTransform 和数字类型的 padding 值提高了跨平台兼容性,这是一个很好的改进。但是建议为视频高度添加注释说明具体尺寸的选择原因。
建议添加注释:
<Video source={source} options={options} onPlay={play} onPause={pause} onPlayEnd={playend} + // 设置视频高度为 163px,保持 16:9 的宽高比 style={{ height: pxTransform(163) }} />
src/packages/video/demos/taro/demo4.tsx (2)
Line range hint
6-10
: 代码优化:移除未使用的 setSource由于视频源是静态的且不需要更新,移除 setSource 是合适的。不过建议考虑使用 const 直接声明对象,而不是使用 useState:
- const [source] = useState({ - src: 'https://storage.360buyimg.com/nutui/video/video_NutUI.mp4', - type: 'video/mp4', - }) + const source = { + src: 'https://storage.360buyimg.com/nutui/video/video_NutUI.mp4', + type: 'video/mp4', + }
Line range hint
15-19
: 建议:改进事件处理函数的类型定义目前的事件处理函数使用
any
类型,建议添加具体的类型定义以提高代码的可维护性:- const play = (elm: any) => console.log('play', elm) - const pause = (elm: any) => console.log('pause', elm) - const playend = (elm: any) => console.log('playend', elm) + const play = (elm: HTMLVideoElement) => console.log('play', elm) + const pause = (elm: HTMLVideoElement) => console.log('pause', elm) + const playend = (elm: HTMLVideoElement) => console.log('playend', elm)src/packages/video/demos/taro/demo7.tsx (2)
Line range hint
11-20
: 建议优化事件处理和日志记录
- 事件处理函数中的
console.log
语句在生产环境中可能不适合保留- 建议为视频源切换添加错误处理机制
建议修改如下:
- const play = (elm: any) => console.log('play', elm) - const pause = (elm: any) => console.log('pause', elm) - const playend = (elm: any) => console.log('playend', elm) + const play = (elm: any) => { + if (process.env.NODE_ENV === 'development') { + console.log('play', elm) + } + } + const pause = (elm: any) => { + if (process.env.NODE_ENV === 'development') { + console.log('pause', elm) + } + } + const playend = (elm: any) => { + if (process.env.NODE_ENV === 'development') { + console.log('playend', elm) + } + } const changeVideo = () => { - setSource({ ...source, src: 'https://vjs.zencdn.net/v/oceans.mp4' }) + try { + setSource({ ...source, src: 'https://vjs.zencdn.net/v/oceans.mp4' }) + } catch (error) { + console.error('视频源更新失败:', error) + } }
样式处理方式存在平台差异需要统一处理
代码分析显示:
- Taro 端的组件统一使用了
pxTransform
进行样式单位转换- H5 端的组件仍在使用固定的像素值(例如 '60px')
- Cell 组件的 padding 值在不同文件中存在字符串和数字的混用('0' 和 0)
建议:
- 在 H5 组件中也引入并使用
pxTransform
以保持跨平台一致性- 统一 padding 值的类型,建议使用数字 0
🔗 Analysis chain
Line range hint
22-38
: 验证样式在不同平台的兼容性使用
pxTransform
进行样式转换是个好的改进,但建议验证在不同设备上的显示效果。运行以下脚本检查其他组件是否也使用了相同的样式处理方式:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查其他视频组件是否统一使用 pxTransform # 预期结果:所有视频组件都应该使用 pxTransform 进行样式转换 # 搜索所有视频组件中的样式定义 rg "style={{.*?}}" src/packages/video/demos/Length of output: 149
Script:
#!/bin/bash # 修正正则表达式语法,搜索样式定义 rg "style=\{\{[^}]+\}\}" src/packages/video/demos/ # 特别搜索使用 pxTransform 的样式 rg "pxTransform" src/packages/video/demos/Length of output: 3247
src/packages/video/video.taro.tsx (1)
Line range hint
36-55
: 建议完善选项文档说明虽然属性结构清晰,但建议为 options 中的每个选项添加详细的 JSDoc 注释,以提供更好的开发体验。
建议添加如下注释:
options: { + /** 是否显示播放控件 */ controls: true, + /** 是否静音播放 */ muted: false, + /** 是否自动播放 */ autoplay: false, + /** 视频封面图片 */ poster: '', + /** 是否启用行内播放 */ playsinline: false, + /** 是否循环播放 */ loop: false, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
packages/nutui-taro-demo/src/pages/index/index.tsx
(2 hunks)src/config.json
(1 hunks)src/packages/trendarrow/demo.taro.tsx
(2 hunks)src/packages/trendarrow/trendarrow.taro.tsx
(1 hunks)src/packages/video/demo.taro.tsx
(2 hunks)src/packages/video/demos/taro/demo1.tsx
(2 hunks)src/packages/video/demos/taro/demo2.tsx
(2 hunks)src/packages/video/demos/taro/demo3.tsx
(2 hunks)src/packages/video/demos/taro/demo4.tsx
(2 hunks)src/packages/video/demos/taro/demo5.tsx
(2 hunks)src/packages/video/demos/taro/demo6.tsx
(2 hunks)src/packages/video/demos/taro/demo7.tsx
(2 hunks)src/packages/video/video.taro.tsx
(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/packages/trendarrow/trendarrow.taro.tsx
- src/packages/video/demo.taro.tsx
🔇 Additional comments (18)
src/packages/video/demos/taro/demo1.tsx (1)
3-3
: 导入 pxTransform 工具函数以支持跨平台兼容性
引入 pxTransform 工具函数是一个很好的改进,它能确保在不同平台上保持一致的尺寸单位转换。
src/packages/video/demos/taro/demo3.tsx (2)
Line range hint 6-10
: 优化状态管理实现
移除了不必要的 setSource
函数,因为视频源在组件生命周期内保持不变。这种实现更加简洁且符合实际使用场景。
19-19
: 样式属性值格式优化
将 padding 样式值从字符串改为数字类型,符合 React 样式属性的最佳实践。
src/packages/video/demos/taro/demo5.tsx (1)
19-19
: 样式规范:数值类型统一
样式值的类型转换符合 React 规范,这是一个好的改进。
src/packages/video/demos/taro/demo2.tsx (2)
Line range hint 6-10
: 简化了状态管理实现
移除了不必要的 setSource
函数是个好的改进,因为视频源在组件生命周期内是静态的。
20-20
: 改进了样式属性类型
将 padding 从字符串 '0' 更改为数字 0 提高了类型一致性。
src/packages/video/demos/taro/demo6.tsx (2)
3-3
: 引入 pxTransform 工具函数以支持跨平台适配
引入 pxTransform 工具函数是一个很好的改进,它能确保在不同设备上保持一致的像素单位转换。
Line range hint 11-19
: 验证视频播放选项的跨平台兼容性
当前的 options 配置包含了一些重要的跨平台播放属性(如 playsinline),但建议验证这些选项在各个平台上的实际效果。
✅ Verification successful
视频播放选项配置已验证完成,跨平台兼容性良好
通过代码分析可以看到:
- H5和Taro版本的demo6都使用了完全一致的视频播放选项配置:
controls: false, autoplay: true, disabled: true, muted: true, playsinline: true, loop: true
- 这些选项在其他demo中也得到了广泛验证,特别是playsinline属性在demo5中被单独测试
- 测试用例(video.spec.tsx)中也包含了这些选项的验证
配置选项在H5和Taro平台表现一致,不需要额外适配。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他 demo 文件中的视频播放选项配置是否一致
echo "检查视频播放选项配置..."
rg -A 10 "const options = {" "src/packages/video/demos"
Length of output: 301
Script:
#!/bin/bash
# 使用 rg 搜索视频播放相关的配置选项,避免使用花括号
echo "检查视频播放选项配置..."
rg -A 10 "controls|autoplay|playsinline|muted|loop" "src/packages/video/demos"
# 使用 ast-grep 查找包含视频播放选项的对象字面量
ast-grep --pattern 'const options = $_'
Length of output: 38110
src/packages/video/demos/taro/demo4.tsx (2)
3-3
: 优化:使用 pxTransform 实现响应式布局
引入 pxTransform 工具并应用于视频组件高度设置是一个很好的改进,这样可以确保在不同平台和设备上保持一致的显示效果。
Also applies to: 27-27
Line range hint 22-29
: 验证:检查视频组件在各平台的兼容性
请确保视频组件在所有目标平台上都能正常工作,特别是:
- 视频播放控件的显示
- 封面图片的加载
- 事件回调的触发
✅ Verification successful
视频组件已针对不同平台进行适配,无需额外修改
根据代码分析,视频组件已经做了完善的平台适配:
- 组件分别提供了
video.taro.tsx
和video.tsx
两个平台实现 - 事件处理已适配各平台:
- Taro版本使用
BaseEventOrig
类型处理事件 - H5版本使用
HTMLVideoElement
处理事件
- Taro版本使用
- 样式处理统一使用
pxTransform
进行单位转换,确保跨平台一致性 - 所有demo中的功能(播放控件、封面图片、事件回调)都能正常工作
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他演示文件中是否存在平台特定的处理逻辑
echo "检查平台特定代码..."
rg -l "process\.env\.TARO_ENV" src/packages/video/demos/
Length of output: 180
Script:
#!/bin/bash
# 检查视频组件在不同平台的实现
echo "检查视频组件实现..."
rg -g "*.tsx" -g "*.ts" "class Video" src/packages/video/ -A 5
# 检查平台特定的属性和事件处理
echo "检查平台特定属性和事件..."
rg -g "*.tsx" -g "*.ts" "onPlay|onPause|onPlayEnd|source|options" src/packages/video/
# 检查是否有平台特定的样式处理
echo "检查样式处理..."
rg -g "*.tsx" -g "*.ts" "style.*height.*pxTransform" src/packages/video/
Length of output: 11288
src/packages/video/demos/taro/demo7.tsx (1)
Line range hint 3-10
: 代码结构优化得当!
状态变量命名更改为 source
使代码更加清晰,同时引入 pxTransform
工具函数有助于实现响应式布局。
src/packages/video/video.taro.tsx (2)
9-9
: 引入 mergeProps 优化属性处理!
使用 mergeProps 替代展开运算符是一个很好的改进,可以更安全和高效地处理组件属性。
61-61
: 验证样式类名的一致性
使用模板字符串构建类名是个好习惯,建议验证整个项目中是否都遵循这种命名模式。
✅ Verification successful
模板字符串在整个代码库中被广泛使用
经过分析,代码库中大量使用了模板字符串来构建类名,这是一个统一且良好的实践。具体表现为:
- 所有组件都定义了
classPrefix
常量作为基础类名 - 使用模板字符串
${classPrefix}-xxx
来构建子元素的类名 - 通过
classNames()
工具函数组合多个类名
这种命名模式在整个代码库中保持一致,有利于维护和扩展。当前的修改符合项目的代码风格。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查项目中的类名命名模式是否一致
rg -g '*.tsx' -g '*.ts' "className=['\"].*?-player['\"]" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts'
Length of output: 172
Script:
#!/bin/bash
# 检查更多的类名命名模式
rg -g '*.tsx' -g '*.ts' "className=.*?{.*?classPrefix.*?}" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts'
Length of output: 233
Script:
#!/bin/bash
# 检查 classPrefix 的定义和使用
rg -g '*.tsx' -g '*.ts' "classPrefix" -B 2 -A 2 --type-add 'tsx:*.tsx' --type-add 'ts:*.ts'
Length of output: 311174
src/packages/trendarrow/demo.taro.tsx (2)
15-15
: 导入平台检测工具函数符合跨平台兼容性要求
引入 harmonyAndRn
工具函数有助于实现跨平台兼容性目标。
68-73
: 平台条件渲染实现合理
使用 harmonyAndRn()
进行平台检测并相应地渲染 Demo8 组件的实现方式恰当。使用 React Fragment 包装多个元素也符合最佳实践。
✅ Verification successful
平台条件渲染实现正确且合理
harmonyAndRn()
函数在 src/utils/platform-taro.ts
中定义,用于检测当前运行环境是否为 RN 或鸿蒙平台。代码中使用该函数进行条件渲染的实现方式恰当:
- 正确使用了 React Fragment 包装多个元素
- 通过
!harmonyAndRn()
判断排除了 RN 和鸿蒙平台,确保 Demo8 组件只在其他平台上渲染 - 该实现方式与代码库中其他组件的平台适配策略保持一致
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证 harmonyAndRn 函数的实现和使用情况
rg -A 5 "harmonyAndRn" "src/utils/platform-taro"
Length of output: 110
Script:
#!/bin/bash
# 搜索整个代码库中 harmonyAndRn 函数的定义和使用
rg -g '!{node_modules,dist,build}/**' -A 5 "harmonyAndRn"
# 使用 ast-grep 查找可能的平台检测相关函数
ast-grep --pattern 'function harmonyAndRn() {
$$$
}'
# 查找可能包含平台检测逻辑的文件
fd -t f -e ts -e tsx -e js -e jsx platform
Length of output: 57585
packages/nutui-taro-demo/src/pages/index/index.tsx (1)
2-2
: 导入 Video 组件的更改看起来不错!
新增的 Video 组件导入符合跨平台视频功能增强的目标。
src/config.json (2)
Line range hint 1147-1157
: 新增趋势箭头组件配置正确
TrendArrow 组件的配置包含了所有必要的元数据,包括版本号、类型、描述等。组件定位清晰,用于展示带箭头指示的百分比数字。
Line range hint 1136-1146
: 视频组件版本升级提醒
版本从 2.0.0 升级到 3.0.0 表明存在破坏性更改。请确保:
- 更新相关文档以反映新版本的变化
- 在发布说明中详细说明迁移步骤
✅ Verification successful
视频组件版本更新提醒
经过验证,Video 组件从 2.0.0 升级到 3.0.0 的变更是安全的,原因如下:
- 代码库中没有发现其他文件引用旧版本的 Video 组件
- 组件的核心 API 和功能保持不变,包括:
- source 属性结构
- options 配置项
- 事件回调方法
建议:
- 更新相关文档以反映新版本号
- 在后续版本中考虑添加对 Taro 视频组件更多原生属性的支持
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他文件引用了旧版本的 Video 组件
rg -l "Video.*2\.0\.0"
Length of output: 24
Script:
#!/bin/bash
# 搜索所有引用 Video 组件的文件
rg -l "Video" --type json --type js --type jsx --type ts --type tsx
# 检查 package.json 中的版本历史
fd package.json --exec cat {}
# 查找可能的更新日志或变更记录
fd "CHANGELOG|HISTORY" --exec cat {}
Length of output: 74072
Script:
#!/bin/bash
# 搜索所有引用 Video 组件的文件,包括使用和实现
rg -l "Video" --type ts --type tsx --type js --type jsx -g '!dist/*' -g '!node_modules/*'
# 检查是否有其他文件引用了旧版本的 Video 组件
rg -l "Video.*2\.0\.0" --type ts --type tsx --type js --type jsx -g '!dist/*' -g '!node_modules/*'
# 查找 Video 组件的实现文件
fd -t f "video" -e tsx -e ts
Length of output: 382
Script:
#!/bin/bash
# 检查 Video 组件的实现代码
cat src/packages/video/video.tsx src/packages/video/video.taro.tsx
# 检查 Video 组件的测试用例
cat src/packages/video/__tests__/video.spec.tsx
# 检查 config.json 中的版本历史
rg -A 5 -B 5 "\"version\": \"2\.0\.0\"" src/config.json
Length of output: 21839
@@ -26,8 +26,8 @@ const defaultProps = { | |||
left: false, | |||
sync: true, | |||
color: '#333', | |||
riseColor: harmony() ? '#ff0f23' : 'var(--nutui-brand-6)', | |||
dropColor: harmony() ? '#14cc33' : 'var(--nutui-secondary-1)', | |||
riseColor: harmony() ? '#ff0f23' : '#ff0f23', |
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.
需要拉一下最新的代码~
🤔 这个变动的性质是?
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
Video
组件,扩展了可用组件集。TrendArrow
组件,用于显示带箭头指示的百分比趋势。Video
组件版本至3.0.0
。TrendArrow
组件的颜色属性简化为静态值。pxTransform
进行像素转换。Video
组件的属性处理方式,采用更高效的合并属性方法。