feat(ImagePreview): 支持taro下缩放效果。 #2939
Conversation
|
Warning Rate limit exceeded@xiaoyatong has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
步骤概述演练此次更改主要涉及图片预览组件（ 变更
可能相关的问题
可能相关的 PR
建议标签
建议审阅者
诗歌
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/packages/imagepreview/imagepreview.tsx (2)
121-141: 建议将魔法数字提取为常量为了提高代码的可维护性，建议将以下值提取为命名常量：
- 双击时间阈值 (300ms)
- 缩放倍数 (2x, 3x)
- 缩放阈值 (1.1)
+const DOUBLE_TAP_THRESHOLD = 300; // ms +const DEFAULT_SCALE = 1; +const DOUBLE_TAP_SCALE = 2; +const MAX_SCALE = 3; +const MIN_SCALE_THRESHOLD = 1.1; const onTouchStart = (event: TouchEvent) => { const { touches } = event const events = touches[0] const events2 = touches[1] const curTouchTime = Date.now() - if (curTouchTime - lastTouchEndTime < 300) { + if (curTouchTime - lastTouchEndTime < DOUBLE_TAP_THRESHOLD) { const store1 = store - store1.scale = store1.scale === 1 ? 2 : 1 + store1.scale = store1.scale === DEFAULT_SCALE ? DOUBLE_TAP_SCALE : DEFAULT_SCALE scaleNow() }Also applies to: 145-164
266-287: 建议改进类型安全性当前实现中的类型断言可以通过使用类型守卫（Type Guard）来改进：
+type MediaItem = { + type: 'video' | 'image'; + data: VideoOption | ImageOption; +}; +const isVideoOption = (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 })), ] .sort((a, b) => (a.data.index ?? 0) - (b.data.index ?? 0)) .map((item, index) => ( <SwiperItem key={index}> - {item.type === 'video' ? ( + {isVideoOption(item) ? ( <Video - source={(item.data as VideoOption).source} - options={(item.data as VideoOption).options} + source={item.data.source} + options={item.data.options} onClick={closeOnImg} /> ) : ( <Image - src={(item.data as ImageOption).src} + src={item.data.src} draggable={false} onClick={closeOnImg} /> )} </SwiperItem> ))}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/packages/imagepreview/imagepreview.scss(1 hunks)src/packages/imagepreview/imagepreview.taro.tsx(6 hunks)src/packages/imagepreview/imagepreview.tsx(8 hunks)src/packages/notify/Notification.tsx(1 hunks)src/packages/toast/Notification.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/packages/notify/Notification.tsx
- src/packages/toast/Notification.tsx
🧰 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: test
🔇 Additional comments (3)
src/packages/imagepreview/imagepreview.tsx (1)
46-47: 类型定义优化得当！通过添加 index 属性来支持自定义排序是一个很好的改进。
src/packages/imagepreview/imagepreview.taro.tsx (1)
290-292: 平台特定功能处理得当！针对非 Web 环境添加 showMenuByLongpress 支持是很好的做法。
src/packages/imagepreview/imagepreview.scss (1)
65-67: 样式约束设置合理！添加 max-width 约束确保了组件在各种屏幕尺寸下的正确显示。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2939 +/- ##
=============================================
+ Coverage 85.58% 86.06% +0.48%
=============================================
Files 277 277
Lines 18185 18140 -45
Branches 2763 2775 +12
=============================================
+ Hits 15563 15613 +50
+ Misses 2617 2522 -95
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/packages/imagepreview/__test__/imagepreview.spec.tsx (1)
49-62: 很好的测试设置函数实现！通过引入
setup函数统一管理组件渲染，减少了代码重复，提高了测试代码的可维护性。建议考虑以下优化：
const setup = (props = {}) => { - render( + return render( <ImagePreview images={images} videos={videos} visible closeIcon defaultValue={0} onChange={mockOnChange} onClose={mockOnClose} {...props} /> ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/packages/imagepreview/__test__/imagepreview.spec.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
src/packages/imagepreview/__test__/imagepreview.spec.tsx (3)
165-194: 👍 移除了不可靠的计时测试代码移除使用
sleep函数的测试代码是个好的改进。新的autoPlay测试使用了更可靠的事件触发方式。
7-162: 建议补充以下测试场景当前测试用例集合还缺少一些重要场景的覆盖：
- 错误处理测试
- 图片加载失败的情况
- 无效的图片地址
- 可访问性测试
- 键盘导航
- 屏幕阅读器支持
- 边界条件测试
- 空图片数组
- 单张图片时的行为
需要验证这些场景在实际使用中的重要性，是否需要优先添加相关测试用例？
125-150: 🛠️ Refactor suggestion缺少Taro环境特定的缩放测试
根据PR目标，这个功能是为Taro框架添加的，但当前测试用例可能无法完全覆盖Taro环境的特定行为。建议：
- 添加Taro环境特定的测试用例
- 补充更多缩放场景的测试，如：
- 最大/最小缩放限制
- 双击缩放
- 缩放重置
另外，当前的transform断言不够严谨：
-expect((swiperIndicator as HTMLElement).style.transform).toContain('scale(') +expect((swiperIndicator as HTMLElement).style.transform).toMatch(/scale\([\d.]+\)/)
| fireEvent.click(closeIcon as Element) | ||
| // await waitFor(() => expect(mockOnClose).toHaveBeenCalledOnce()) |
There was a problem hiding this comment.
需要补充异步断言验证
mockOnClose 的调用验证被注释掉了，建议添加适当的异步断言：
fireEvent.click(closeIcon as Element)
-// await waitFor(() => expect(mockOnClose).toHaveBeenCalledOnce())
+expect(mockOnClose).toHaveBeenCalledTimes(1)📝 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.
| fireEvent.click(closeIcon as Element) | |
| // await waitFor(() => expect(mockOnClose).toHaveBeenCalledOnce()) | |
| fireEvent.click(closeIcon as Element) | |
| expect(mockOnClose).toHaveBeenCalledTimes(1) |
|
在 H5 中快速滑动图片会触发放大功能 |
🤔 这个变动的性质是？
🔗 相关 Issue
💡 需求背景和解决方案
☑️ 请求合并前的自查清单
Summary by CodeRabbit
样式调整
组件优化
性能提升
Date.now()替代new Date().getTime()测试增强