-
Notifications
You must be signed in to change notification settings - Fork 268
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: imagepreview taro 增加缩放, 优化代码 #2941
base: next
Are you sure you want to change the base?
Conversation
概述演练这个拉取请求主要修改了图像预览组件的样式和代码实现。在 变更
可能相关的问题
可能相关的 PR
建议标签
建议审阅者
诗歌
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 #2941 +/- ##
==========================================
+ Coverage 84.02% 84.51% +0.49%
==========================================
Files 220 220
Lines 17917 17872 -45
Branches 2628 2639 +11
==========================================
+ Hits 15055 15105 +50
+ Misses 2857 2762 -95
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: 0
🧹 Nitpick comments (4)
src/packages/imagepreview/imagepreview.tsx (2)
111-167
: 建议优化缩放性能当前缩放实现逻辑清晰,但建议以下优化:
- 考虑使用
requestAnimationFrame
来优化缩放动画性能- 添加防抖处理,避免频繁的状态更新
- 考虑使用 CSS transform 的
scale3d
来启用硬件加速const scaleNow = () => { if (ref.current) { - ref.current.style.transform = `scale(${store.scale})` + requestAnimationFrame(() => { + ref.current.style.transform = `scale3d(${store.scale}, ${store.scale}, 1)` + }) } }
265-286
: 建议增强类型安全性当前实现的类型判断可以更严谨,建议以下改进:
- 创建统一的联合类型来处理图片和视频项
- 使用类型守卫函数来确保类型安全
+ type MediaItem = + | { type: 'video'; data: VideoOption } + | { type: 'image'; data: ImageOption } + const isVideo = (item: MediaItem): item is { type: 'video'; data: VideoOption } => { + return item.type === 'video' + } {[ - ...videos.map((item) => ({ type: 'video', data: item })), - ...images.map((item) => ({ type: 'image', data: item })), + ...videos.map((item): MediaItem => ({ type: 'video', data: item })), + ...images.map((item): MediaItem => ({ type: 'image', data: item })), ]src/packages/imagepreview/imagepreview.taro.tsx (1)
269-297
: 建议优化平台特定实现针对不同平台的处理已经很好,但还可以进一步优化:
- 建议将平台特定的逻辑抽离到单独的 hooks 中
- 考虑使用条件编译来减少打包体积
- 为小程序平台添加更多的性能优化
+ const usePlatformFeatures = () => { + const isWeb = Taro.getEnv() === 'WEB' + return { + showMenuByLongpress: !isWeb, + imageProps: !isWeb ? { showMenuByLongpress } : {} + } + } - {...(Taro.getEnv() !== 'WEB' && { - showMenuByLongpress, - })} + {...imageProps}src/packages/imagepreview/imagepreview.scss (1)
81-87
: 建议增强响应式设计当前的弹性布局实现良好,建议添加以下响应式优化:
- 添加媒体查询以适应不同屏幕尺寸
- 考虑使用 viewport 单位来优化大屏幕显示
- 添加最小/最大尺寸限制
.nut-image, .nut-video { display: flex; justify-content: center; align-items: center; + min-width: 280px; + max-width: 90vw; + @media (min-width: 768px) { + max-width: 80vw; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/packages/imagepreview/imagepreview.scss
(3 hunks)src/packages/imagepreview/imagepreview.taro.tsx
(8 hunks)src/packages/imagepreview/imagepreview.tsx
(7 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/packages/imagepreview/imagepreview.taro.tsx
[error] 238-240: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 245-246: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/packages/imagepreview/imagepreview.tsx (1)
46-47
: 类型定义优化值得称赞!使用
ImageOption[]
和VideoOption[]
类型别名替代内联类型定义,提高了代码的可维护性和类型安全性。
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: 0
🔭 Outside diff range comments (1)
src/packages/imagepreview/__test__/imagepreview.spec.tsx (1)
Line range hint
1-144
: 缺少缩放功能的测试用例根据 PR 描述,此次改动增加了图片预览的缩放功能,但测试文件中没有相关的测试用例。建议添加以下测试场景:
- 基本缩放功能
- 最大/最小缩放限制
- 缩放手势操作
- 双击缩放
- 缩放状态下的拖动
// 示例测试用例 test('zoom functionality', async () => { const { container } = render( <ImagePreview images={images} visible defaultZoom={1} minZoom={0.5} maxZoom={3} /> ) // 测试缩放相关的交互和状态 })
🧹 Nitpick comments (2)
src/packages/imagepreview/__test__/imagepreview.spec.tsx (2)
100-100
: 建议增加样式自定义的测试用例当前测试仅验证了指示器的颜色和底部位置,建议增加更多样式属性的测试覆盖:
- 指示器的大小
- 指示器的形状
- 指示器的透明度
- 其他可自定义的样式属性
test('customize indicator styles', async () => { const { container } = render( <ImagePreview images={images} visible indicator indicatorColor="red" indicatorSize={20} indicatorShape="square" indicatorOpacity={0.8} /> ) // 添加相应的样式验证 })
Line range hint
1-144
: 建议优化测试结构和组织当前测试文件的组织结构可以进行以下改进:
使用
describe
块按功能模块组织测试:
- 基础功能
- 自动播放
- 指示器
- 关闭按钮
- 缩放功能
- 视频播放
测试描述应更具体,包含:
- 测试场景
- 预期行为
- 边界条件
describe('ImagePreview', () => { describe('Basic Features', () => { test('should render correctly with default props', () => { // ... }) }) describe('Zoom Features', () => { test('should zoom in/out with gesture controls', () => { // ... }) }) // 其他功能模块... })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/imagepreview/__test__/imagepreview.spec.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (1)
src/packages/imagepreview/__test__/imagepreview.spec.tsx (1)
61-61
: 测试稳定性和覆盖率需要改进这个测试用例存在以下问题:
- 移除了 autoPlay 属性但没有相应的测试来验证默认行为
- 延迟时间从 1100ms 增加到 4600ms,超时时间从 2000ms 增加到 5000ms,这可能表明测试不够稳定
- 只验证了初始状态的索引显示(1/4),没有验证自动播放过程中的状态变化
建议添加以下测试场景:
- 验证默认的自动播放行为
- 验证切换过程中的索引变化
- 使用更可靠的方式来等待状态变化,而不是依赖固定的延迟时间
Also applies to: 68-68, 72-73, 76-76
@Alex-huxiyang i'll review the changes in your pr that adds zoom functionality and optimizes the imagepreview component for taro. ✅ Actions performedReview triggered.
|
@@ -75,7 +76,7 @@ const defaultProps = { | |||
closeIcon: false, | |||
closeIconPosition: 'top-right', | |||
showMenuByLongpress: false, | |||
onChange: (value: number) => {}, | |||
onChange: () => {}, |
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
#784
#1255
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
样式更新
组件改进
测试调整