feat(imagepreview): harmony adaptation #2791
Conversation
Walkthrough此拉取请求对 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint (1.23.1)
packages/nutui-taro-demo/src/app.config.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "taro/react" to extend from. Please check that the name of the config is correct. The config "taro/react" was referenced from the config file in "/packages/nutui-taro-demo/.eslintrc". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 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 @@
## feat_v3.x #2791 +/- ##
==========================================
Coverage 83.99% 83.99%
==========================================
Files 220 220
Lines 17885 17885
Branches 2679 2679
==========================================
Hits 15023 15023
Misses 2857 2857
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (15)
src/packages/imagepreview/demos/taro/demo1.tsx (2)
7-16: 建议优化图片加载性能当前实现存在以下可以改进的地方：
- 使用了不同的CDN源（jsdelivr和360buyimg），建议统一使用同一个CDN以减少DNS查询
- 图片尺寸差异较大，建议标准化图片尺寸
- 可以考虑添加图片懒加载或使用响应式图片
建议参考以下优化方案：
const images = [ { - src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + src: 'https://m.360buyimg.com/nutui/apple-4.jpeg', + loading: 'lazy', + srcset: 'https://m.360buyimg.com/nutui/apple-4-small.jpeg 300w, + https://m.360buyimg.com/nutui/apple-4.jpeg 750w' }, // 其他图片也进行类似优化 ]
Line range hint
22-30: 建议增加错误处理机制当前的图片预览组件缺少错误处理机制，建议添加
onError回调来处理图片加载失败的情况。<ImagePreview autoPlay images={images} visible={showPreview} onClose={() => setShowPreview(false)} + onError={(err) => { + console.error('图片加载失败:', err); + // 在此处理错误情况 + }} />src/packages/imagepreview/demos/taro/demo5.tsx (2)
7-16: 图片URL更新提升了安全性，但建议优化CDN使用URLs更改为HTTPS协议是一个很好的安全性改进。不过目前使用了多个不同的CDN源（jsdelivr.net和360buyimg.com），这可能会影响加载性能。
建议考虑以下优化：
- 统一使用同一个CDN服务
- 考虑使用更短的URL路径
- 为图片添加合适的alt属性以提升可访问性
const images = [ { - src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + alt: '苹果图片' }, { - src: 'https://m.360buyimg.com/mobilecms/s750x366_jfs/t1/26597/30/4870/174583/5c35c5d2Ed55eedc6/50e27870c25e7a82.png', + src: 'https://fastly.jsdelivr.net/npm/@nutui/images/demo1.png', + alt: '示例图片1' }, // ... 其他图片也做类似更改
Line range hint
21-31: 建议增强组件的健壮性和可访问性当前实现缺少图片加载状态处理和错误处理机制，建议增加相关功能以提升用户体验。
建议添加以下功能：
- 图片加载状态处理
- 加载失败的容错处理
- 键盘导航支持
<ImagePreview autoPlay images={images} visible={showPreview} indicator indicatorColor="red" + onImageLoad={(index) => console.log(`图片${index}加载完成`)} + onImageError={(index) => console.log(`图片${index}加载失败`)} + fallback="加载失败显示的默认图片URL" onClose={() => setShowPreview(false)} /> -<Cell title="设置轮播指示器及颜色" onClick={() => setShowPreview(true)} /> +<Cell + title="设置轮播指示器及颜色" + onClick={() => setShowPreview(true)} + role="button" + tabIndex={0} + onKeyPress={(e) => e.key === 'Enter' && setShowPreview(true)} +/>src/packages/imagepreview/demos/taro/demo7.tsx (2)
4-19: 建议添加图片加载状态处理建议为图片预览添加加载状态处理，以提升用户体验。可以考虑添加 loading 占位图或骨架屏。
const Demo7 = () => { const images = [ { src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + loading: 'lazy', + alt: '苹果图片', }, // ... 其他图片同样添加 loading 和 alt 属性 ] const [showPreview, setShowPreview] = useState(false) + const [loading, setLoading] = useState(false) return ( <> <ImagePreview autoPlay={false} images={images} visible={showPreview} closeIcon closeIconPosition="bottom" onClose={() => setShowPreview(false)} + onImageLoad={() => setLoading(false)} + beforePreview={() => setLoading(true)} /> <Cell title="关闭按钮" onClick={() => setShowPreview(true)} /> </> ) }
图片资源已经过验证，建议添加错误处理机制
建议在 ImagePreview 组件中添加 onError 事件处理器来处理图片加载失败的情况，示例代码:
<ImagePreview autoPlay={false} images={images} visible={showPreview} closeIcon closeIconPosition="bottom" onClose={() => setShowPreview(false)} onError={(err) => { console.log('图片加载失败:', err) // 在这里添加错误处理逻辑 }} />🔗 Analysis chain
Line range hint
22-31: 验证图片资源的可访问性需要确保所有图片资源都能正常访问，建议添加错误处理机制。
Let me analyze the code in the ImagePreview component demo7.tsx to verify the image resources.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 验证图片资源的可访问性 # 提取并检查所有图片 URL rg -o 'https://[^"]+\.(jpeg|jpg|png)' | while read -r url; do echo "Checking: $url" curl -sI "$url" | head -n 1 doneLength of output: 154281
Script:
#!/bin/bash # Check the images array content in demo7.tsx rg -U "const images = \[.*?\]" src/packages/imagepreview/demos/taro/demo7.tsxLength of output: 79
Script:
#!/bin/bash # 查看完整的 demo7.tsx 文件内容 cat src/packages/imagepreview/demos/taro/demo7.tsxLength of output: 1074
src/packages/imagepreview/demos/taro/demo3.tsx (1)
7-7: 改进：使用 HTTPS 协议提升安全性将图片 URL 从协议相对格式更改为显式 HTTPS 格式是一个很好的安全改进，可以防止协议降级攻击。
建议：统一 CDN 来源并优化图片
目前使用了多个不同的 CDN（jsdelivr 和 360buyimg），建议：
- 考虑统一使用同一个 CDN 以提高性能和可维护性
- 确保所有图片都经过适当的移动端优化
建议修改如下：
- src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + src: 'https://storage.360buyimg.com/nutui-static/apple-4.jpeg',Also applies to: 10-10, 13-13, 16-16
src/packages/imagepreview/demos/taro/demo4.tsx (2)
7-7: 图片URL更新和建议优化图片URL从协议相对格式更改为HTTPS格式是个很好的改进。不过建议考虑以下优化点：
- 建议统一使用同一个CDN服务，避免多个域名导致的额外DNS查询
- 建议简化URL路径，当前部分URL过长
- 考虑添加图片加载失败的容错处理
建议参考以下改进方案：
const images = [ { - src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + src: 'https://img.xxx.com/demo/apple-4.jpeg', + fallback: '默认图片URL' }, // ... 其他图片同样处理 ]Also applies to: 10-10, 13-13, 16-16
Line range hint
21-40: 组件实现建议优化当前实现存在以下可以改进的地方：
init状态使用了any类型，建议明确类型定义- 生产环境代码中包含
console.log- 缺少图片加载错误处理机制
建议按照以下方式优化：
- const [init, setInit] = useState<any>(1) + const [init, setInit] = useState<number>(1) return ( <> <ImagePreview autoPlay images={images} visible={showPreview} value={init} defaultValue={init} indicator onChange={(value) => { - console.log('demo onChange', value) setInit(value) }} + onError={(err) => { + console.error('图片加载失败:', err) + // 处理加载失败逻辑 + }} onClose={() => setShowPreview(false)} /> </> )src/packages/imagepreview/demos/taro/demo2.tsx (1)
26-35: 组件结构优化建议虽然当前实现是可行的，但建议以下优化：
- 考虑将图片尺寸提取为常量或配置项
- 可以添加加载状态处理
建议的代码改进：
+ const IMAGE_SIZE = 30 return ( <> <Cell style={{ position: 'relative' }}> {images.map((image, index) => ( <View key={image.src} onClick={() => { setShowPreview(true) setInit(index + 1) }} style={{ marginRight: '10px' }} > - <Image width={30} height={30} src={image.src} /> + <Image + width={IMAGE_SIZE} + height={IMAGE_SIZE} + src={image.src} + loading="lazy" + onError={() => console.warn(`图片加载失败: ${image.src}`)} + /> </View> ))}src/packages/imagepreview/demos/taro/demo6.tsx (1)
Line range hint
1-65: 建议优化混合媒体预览的用户体验当前实现在处理图片和视频的混合预览时可以考虑以下改进：
- 为视频添加预览缩略图（poster）
- 考虑添加媒体类型标识，帮助用户区分图片和视频
建议按如下方式优化代码：
const videos = [ { source: { src: 'https://storage.jd.com/about/big-final.mp4?Expires=3730193075&AccessKey=3LoYX1dQWa6ZXzQl&Signature=ViMFjz%2BOkBxS%2FY1rjtUVqbopbJI%3D', type: 'video/mp4', }, options: { muted: true, controls: true, + poster: 'https://example.com/video-thumbnail.jpg', }, + type: 'video', index: 0, }, // ... 其他视频配置 ] const images = [ { src: 'https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg', + type: 'image', index: 1, }, // ... 其他图片配置 ]packages/nutui-taro-demo/src/app.config.ts (3)
40-50: 建议合并数据输入相关的子包目前数据输入相关的组件分散在
dentry和dataentry两个子包中，建议考虑合并这两个子包以提高代码组织的一致性。建议将这两个子包合并为一个名为
form-components的子包，例如：{ - "root": "dentry", + "root": "form-components", "pages": [ "pages/address/index", // ... 其他表单相关组件 ] }, - { - "root": "dataentry", - "pages": [ - // ... 当前 dataentry 中的组件 - ] - }Also applies to: 54-67
93-111: 展示类组件的分类需要优化在
exhibition子包中，有一些组件（如virtuallist和swiper）可能更适合放在其他类别中：
virtuallist更适合放在性能优化相关的子包中swiper可以考虑放在导航组件子包中建议重新评估这些组件的分类，可以考虑：
- 创建新的性能优化子包
- 将部分组件移动到更合适的现有子包中
116-123: 业务组件子包的命名建议
business这个命名可能过于笼统，建议根据具体业务场景重命名，以提高代码的可维护性和可理解性。可以考虑更具体的命名，例如：
{ - "root": "business", + "root": "enhanced-components", "pages": [ // ... 现有组件 ] }src/config.json (1)
版本升级需要补充迁移指南和变更说明
从检查结果来看，ImagePreview 组件升级到 3.0.0 版本，但在 CHANGELOG.md 和组件文档中都没有找到相关的重大更改说明和迁移指南。为确保用户顺利升级：
建议：
- 在 CHANGELOG.md 中补充 3.0.0 版本的变更记录
- 在组件文档中添加版本升级指南章节，说明与 2.x 版本的差异
- 明确标注任何不兼容的 API 变更
🔗 Analysis chain
Line range hint
1005-1013: 请确认版本升级的兼容性影响ImagePreview 组件从 2.0.0 升级到 3.0.0 是一个主版本升级，这表明可能存在破坏性更改。
建议：
- 确保已经记录了所有破坏性更改
- 提供从 2.x 到 3.x 的迁移指南
- 更新相关的示例代码和文档
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述：验证是否存在迁移指南和重大更改文档 # 测试：检查是否存在迁移指南 fd -t f "MIGRATION|BREAKING-CHANGES|UPGRADE" -E md # 测试：检查 ImagePreview 相关的文档 rg -l "ImagePreview" docs/Length of output: 125
Script:
#!/bin/bash # 检查仓库根目录下的文档目录结构 fd -t d "doc|docs" # 检查仓库中所有与 ImagePreview 相关的文档文件 fd -t f . -E node_modules -E dist -E build | rg -i "imagepreview|image.?preview" # 检查 changelog 或版本历史相关文件 fd -t f "CHANGELOG|HISTORY|RELEASES" -E node_modules # 检查 ImagePreview 组件的源代码，寻找重大更改 rg -l "ImagePreview" src/Length of output: 2454
Script:
#!/bin/bash # 检查 CHANGELOG.md 中关于 ImagePreview 的更新记录 rg -A 5 -B 5 "ImagePreview" CHANGELOG.md # 检查组件文档中的重大更改说明 cat src/packages/imagepreview/doc.md # 对比新旧版本的组件实现差异 rg -A 5 "export (default |class |interface )" src/packages/imagepreview/imagepreview.tsx rg -A 5 "export (default |class |interface )" src/packages/imagepreview/imagepreview.taro.tsxLength of output: 2334
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
packages/nutui-taro-demo/src/app.config.ts(1 hunks)src/config.json(1 hunks)src/packages/imagepreview/demos/taro/demo1.tsx(1 hunks)src/packages/imagepreview/demos/taro/demo2.tsx(2 hunks)src/packages/imagepreview/demos/taro/demo3.tsx(1 hunks)src/packages/imagepreview/demos/taro/demo4.tsx(1 hunks)src/packages/imagepreview/demos/taro/demo5.tsx(1 hunks)src/packages/imagepreview/demos/taro/demo6.tsx(1 hunks)src/packages/imagepreview/demos/taro/demo7.tsx(1 hunks)src/packages/imagepreview/imagepreview.scss(1 hunks)
🔇 Additional comments (7)
src/packages/imagepreview/demos/taro/demo1.tsx (1)
7-7: 图片URL更新为HTTPS协议是个好的改进！
将图片URL从协议相对格式更改为显式HTTPS格式提高了安全性，确保了内容始终通过加密连接加载。
Also applies to: 10-10, 13-13, 16-16
src/packages/imagepreview/demos/taro/demo7.tsx (1)
7-7: 图片 URL 更新为 HTTPS 协议，提高了安全性！
将图片 URL 从协议相对路径更改为显式 HTTPS 协议是一个很好的改进，这样可以确保图片始终通过安全连接加载。
Also applies to: 10-10, 13-13, 16-16
src/packages/imagepreview/demos/taro/demo2.tsx (3)
2-3: 导入声明更新符合 Taro 最佳实践
使用 @nutui/nutui-react-taro 的 Image 组件和 @tarojs/components 的 View 组件替换原生 HTML 元素是正确的做法，这样能确保更好的跨平台兼容性。
8-8: 图片URL更新提高了安全性
将图片URL从协议相对路径（//）更改为明确的HTTPS协议是一个很好的安全改进。这样可以：
- 确保始终使用加密连接加载图片
- 防止中间人攻击
- 提高资源加载的可靠性
Also applies to: 11-11, 14-14, 17-17
Line range hint 38-45: ImagePreview配置验证
请确认以下几点：
autoPlay功能是否需要提供时间间隔配置defaultValue使用init是否会在某些情况下导致不同步
建议添加相关单元测试验证这些场景。需要我协助编写测试用例吗？
src/packages/imagepreview/demos/taro/demo6.tsx (1)
7-7: 图片 URL 更新为 HTTPS 协议的改动是正确的
将图片 URL 从协议相对格式更改为显式的 HTTPS 格式提高了安全性和可靠性。
运行以下脚本验证图片的可访问性：
Also applies to: 11-11, 15-15, 19-19
✅ Verification successful
所有图片资源均可正常访问，URL 更新是安全且有效的
验证结果显示所有图片资源均返回 HTTP 200 状态码，确认：
- jsdelivr.net CDN 上的苹果图片可访问
- 360buyimg.com 上的所有图片资源均可访问
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证所有图片 URL 的可访问性
# 测试每个图片 URL 的响应状态
for url in \
"https://fastly.jsdelivr.net/npm/@vant/assets/apple-4.jpeg" \
"https://m.360buyimg.com/mobilecms/s750x366_jfs/t1/26597/30/4870/174583/5c35c5d2Ed55eedc6/50e27870c25e7a82.png" \
"https://m.360buyimg.com/mobilecms/s750x366_jfs/t1/9542/17/12873/201687/5c3c4362Ea9eb757d/60026b40a9d60d85.jpg" \
"https://m.360buyimg.com/mobilecms/s750x366_jfs/t1/30042/36/427/82951/5c3bfdabE3faf2f66/9adca782661c988c.jpg"
do
curl -sI "$url" | head -n 1
done
Length of output: 2156
src/packages/imagepreview/imagepreview.scss (1)
42-44: 样式更改符合设计规范！
图标颜色设置与关闭按钮的整体样式保持一致，增强了组件的视觉统一性。
运行以下脚本验证 $white 变量的定义：
✅ Verification successful
样式变更已确认无误！
$white 变量已在 src/styles/variables.scss 中正确定义为 #ffffff，可以安全使用。样式设置合理且符合设计规范。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查 $white 变量是否在样式文件中定义
# 搜索可能包含变量定义的文件
rg -l '\$white' | grep -i 'variables\|config\|theme'
Length of output: 268
Script:
#!/bin/bash
# 检查主要的变量文件中 $white 变量的定义
rg '\$white\s*:' src/styles/variables.scss -A 1
Length of output: 106
| "pages/elevator/index", | ||
| "pages/fixednav/index", | ||
| "pages/hoverbutton/index", | ||
| "pages/navbar/index", | ||
| "pages/sidenavbar/index", | ||
| "pages/tabbar/index", | ||
| "pages/tabs/index" | ||
| ] | ||
| }, | ||
| { | ||
| "root": "dentry", | ||
| "pages": [] | ||
| "pages": [ | ||
| "pages/address/index", | ||
| "pages/calendar/index", | ||
| "pages/calendarcard/index", | ||
| "pages/cascader/index", | ||
| "pages/checkbox/index", | ||
| "pages/datepicker/index", | ||
| "pages/form/index", | ||
| "pages/input/index", | ||
| "pages/inputnumber/index" | ||
| ] | ||
| }, | ||
| { | ||
| "root": "dataentry", | ||
| "pages": [] | ||
| "pages": [ | ||
| "pages/menu/index", | ||
| "pages/numberkeyboard/index", | ||
| "pages/picker/index", | ||
| "pages/radio/index", | ||
| "pages/range/index", | ||
| "pages/rate/index", | ||
| "pages/searchbar/index", | ||
| "pages/shortpassword/index", | ||
| "pages/signature/index", | ||
| "pages/switch/index", | ||
| "pages/textarea/index", | ||
| "pages/uploader/index" | ||
| ] | ||
| }, | ||
| { | ||
| "root": "feedback", | ||
| "pages": [] | ||
| "pages": [ | ||
| "pages/actionsheet/index", | ||
| "pages/badge/index", | ||
| "pages/dialog/index", | ||
| "pages/drag/index", | ||
| "pages/empty/index", | ||
| "pages/infiniteloading/index", | ||
| "pages/loading/index", | ||
| "pages/noticebar/index", | ||
| "pages/notify/index", | ||
| "pages/popover/index", | ||
| "pages/popup/index", | ||
| "pages/pulltorefresh/index", | ||
| "pages/resultpage/index", | ||
| "pages/skeleton/index", | ||
| "pages/swipe/index", | ||
| "pages/toast/index" | ||
| ] | ||
| }, | ||
| { | ||
| "root": "exhibition", | ||
| "pages": [ | ||
| "pages/countdown/index" | ||
| "pages/animate/index", | ||
| "pages/animatingnumbers/index", | ||
| "pages/avatar/index", | ||
| "pages/circleprogress/index", | ||
| "pages/collapse/index", | ||
| "pages/countdown/index", | ||
| "pages/ellipsis/index", | ||
| "pages/imagepreview/index", | ||
| "pages/indicator/index", | ||
| "pages/pagination/index", | ||
| "pages/price/index", | ||
| "pages/progress/index", | ||
| "pages/steps/index", | ||
| "pages/swiper/index", | ||
| "pages/table/index", | ||
| "pages/tag/index", | ||
| "pages/tour/index", | ||
| "pages/video/index", | ||
| "pages/virtuallist/index" | ||
| ] | ||
| }, | ||
| { | ||
| "root": "business", | ||
| "pages": [] | ||
| "pages": [ | ||
| "pages/avatarcropper/index", | ||
| "pages/barrage/index", | ||
| "pages/card/index", | ||
| "pages/timeselect/index", | ||
| "pages/trendarrow/index", | ||
| "pages/watermark/index" | ||
| ] |
There was a problem hiding this comment.
💡 Codebase verification
配置文件中的页面路径尚未实现
根据验证结果，当前 app.config.ts 中定义的子包结构和页面尚未在项目中实际创建：
- 源码目录下只有基本的应用配置文件和一个
pages目录 - 未找到配置中定义的任何组件页面文件
- 子包目录结构（如 base、layout、nav 等）也尚未创建
建议：
- 创建配置文件中定义的所有子包目录结构
- 实现对应的组件页面
- 或者先将配置文件中未实现的页面路径注释掉，避免运行时报错
🔗 Analysis chain
子包结构清晰，但建议验证路径完整性
子包的组织结构按功能类别划分合理，便于维护和按需加载。不过建议验证所有页面路径的有效性，以确保没有遗漏或错误的路径。
Let me explain: The previous script failed because we were searching in the wrong directory. Since the file we're reviewing is in packages/nutui-taro-demo/src/app.config.ts, we should search for the page files under that directory structure. This new script will help us verify if the pages mentioned in the configuration actually exist in the codebase.
我们需要进一步验证目录结构和组件文件。由于配置文件中定义了多个子包和页面，但我们只找到了一个 index 页面，我们需要确认：
- 子包目录结构是否已创建
- 相关的组件文件是否存在（以基础组件为例进行搜索）
这将帮助我们理解当前的实现状态。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证所有页面路径是否存在对应的文件
# 检查每个子包中定义的页面是否都有对应的文件
for root in base layout nav dentry dataentry feedback exhibition business; do
echo "检查 $root 包中的页面..."
fd -t f "index\.(tsx|jsx)$" "$root/pages"
done
Length of output: 2151
Script:
#!/bin/bash
# 检查正确的路径结构
# 首先找到所有的页面文件
fd -t f "index\.(tsx|jsx)$" "packages/nutui-taro-demo/src"
Length of output: 109
Script:
#!/bin/bash
# 检查子包目录结构是否存在
ls -la packages/nutui-taro-demo/src/
# 检查是否有相关的组件文件
fd -t f -e tsx -e jsx "button|cell|icon|image|overlay" packages/nutui-taro-demo/src/
Length of output: 674
🤔 这个变动的性质是？
Summary by CodeRabbit
新功能
** bug 修复**
文档