Skip to content
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

refactor(uploader): Upoader 3.0 重构 & 多端适配 #2660

Open
wants to merge 40 commits into
base: V3.0
Choose a base branch
from

Conversation

Alex-huxiyang
Copy link
Collaborator

@Alex-huxiyang Alex-huxiyang commented Oct 24, 2024

🤔 这个变动的性质是?

  • 新特性提交
  • 日常 bug 修复
  • 站点、文档改进
  • 演示代码改进
  • 组件样式/交互改进
  • TypeScript 定义更新
  • 包体积优化
  • 性能优化
  • 功能增强
  • 国际化改进
  • 重构
  • 代码风格优化
  • 测试用例
  • 分支合并
  • 其他改动(是关于什么的改动?)

🔗 相关 Issue

💡 需求背景和解决方案

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • fork仓库代码是否为最新避免文件冲突
  • Files changed 没有 package.json lock 等无关文件

Summary by CodeRabbit

版本更新说明

  • 新功能

    • 更新了多个组件的版本,新增了多个组件(如EmptyResultPageNotifyPopupPullToRefreshSkeletonSwipe等)。
    • 引入了新的上传逻辑,支持更灵活的文件上传管理。
    • 增加了对文件大小和数量的限制功能。
    • 组件的上传功能现已支持动态状态管理,提升了用户交互体验。
    • 新增了upload方法,简化了上传配置。
  • 文档

    • 更新了上传组件的文档,增强了清晰度和可读性。
    • 修改了文档中的属性描述,增加了新属性的说明。
  • 样式

    • 增加了新的进度条样式,改善了上传体验。
    • 更新了组件的样式以确保一致性,并增强了对RTL支持。

Copy link

coderabbitai bot commented Oct 24, 2024

Walkthrough

此拉取请求主要对src/config.json文件进行了版本更新,多个组件的版本均已升级至3.0.0,包括UploaderAvatarPagination等。此外,新增了多个组件,如EmptyNotifyPopup等。部分组件的可见性已被修改,show属性设置为false。同时,多个测试用例和演示文件中的逻辑也进行了相应的调整,以适应新的组件结构和功能。

Changes

文件路径 变更摘要
src/config.json 更新多个组件版本至3.0.0,新增组件包括EmptyNotify等,部分组件的show属性设置为false
src/packages/uploader/__tests__/uploader.spec.tsx 修改UploaderPreview组件的测试用例,改变list变量类型,添加url属性,更新点击事件处理选择器。
src/packages/uploader/demo.taro.tsx 更新翻译对象,调整JSX结构,修改ScrollView类名,简化翻译内容。
src/packages/uploader/demo.tsx 移除多个演示组件的导入,更新翻译字符串,调整JSX结构,改变演示组件的顺序。
src/packages/uploader/demos/h5/demo1.tsx 引入useState管理状态,移除静态uploadUrlonStart函数,更新Uploader组件的props。
src/packages/uploader/demos/h5/demo10.tsx 新增Demo10组件,使用useState管理文件列表,定义upload函数模拟文件上传。
src/packages/uploader/demos/h5/demo11.tsx 删除Demo11组件。
src/packages/uploader/demos/h5/demo12.tsx 删除Demo12组件。
src/packages/uploader/demos/h5/demo13.tsx 删除Demo13组件。
src/packages/uploader/demos/h5/demo14.tsx 删除Demo14组件。
src/packages/uploader/demos/h5/demo2.tsx 重命名组件,更新内部逻辑,添加defaultListonDelete函数。
src/packages/uploader/demos/h5/demo3.tsx 更新文件上传逻辑,使用useState管理文件列表,添加upload函数。
src/packages/uploader/demos/h5/demo4.tsx 更新状态管理,添加onOversizeupload函数,调整JSX结构。
src/packages/uploader/demos/h5/demo5.tsx 引入状态管理,添加beforeUpload函数,更新上传逻辑。
src/packages/uploader/demos/h5/demo6.tsx 更新组件结构,设置Uploader为禁用状态。
src/packages/uploader/demos/h5/demo7.tsx 更新组件结构,允许多个Uploader实例,定义defaultFileList
src/packages/uploader/demos/h5/demo8.tsx 更新上传逻辑,添加uploadsleep函数。
src/packages/uploader/demos/h5/demo9.tsx 更新上传逻辑,添加uploadsleep函数,使用useState管理文件列表。
src/packages/uploader/demos/taro/demo1.tsx 更新状态管理,删除静态uploadUrl,添加uploaduploadFail函数。
src/packages/uploader/demos/taro/demo10.tsx 删除Demo10组件。
src/packages/uploader/demos/taro/demo11.tsx 删除Demo11组件。
src/packages/uploader/demos/taro/demo12.tsx 删除Demo12组件。
src/packages/uploader/demos/taro/demo13.tsx 删除Demo13组件。
src/packages/uploader/demos/taro/demo14.tsx 删除Demo14组件。
src/packages/uploader/demos/taro/demo2.tsx 更新组件逻辑,添加defaultListonDelete函数。
src/packages/uploader/demos/taro/demo3.tsx 更新状态管理,添加uploadsleep函数。
src/packages/uploader/demos/taro/demo4.tsx 更新状态管理,添加onOversizeupload函数。
src/packages/uploader/demos/taro/demo5.tsx 更新上传逻辑,添加beforeUploadupload函数。
src/packages/uploader/demos/taro/demo6.tsx 更新组件结构,设置Uploader为禁用状态。
src/packages/uploader/demos/taro/demo7.tsx 更新组件结构,允许多个Uploader实例,定义defaultFileList
src/packages/uploader/demos/taro/demo8.tsx 更新上传逻辑,添加uploadsleep函数。
src/packages/uploader/demos/taro/demo9.tsx 更新上传逻辑,添加uploadsleep函数,使用useState管理文件列表。
src/packages/uploader/doc.en-US.md 更新文档标题和内容,修改属性表,增加新属性。
src/packages/uploader/doc.md 更新文档结构和内容,修改属性表,增加新属性。
src/packages/uploader/doc.taro.md 更新文档结构和内容,修改属性表,增加新属性。
src/packages/uploader/doc.zh-TW.md 更新文档结构和内容,修改属性表,增加新属性。
src/packages/uploader/file-item.taro.ts 删除FileItem类及其相关类型。
src/packages/uploader/file-item.ts FileItem类修改为接口,更新属性为可选。
src/packages/uploader/index.taro.ts 更新类型导出路径。
src/packages/uploader/preview.taro.tsx 更新导入路径,改进错误处理逻辑,简化代码。
src/packages/uploader/preview.tsx 更新状态检查和解构逻辑,改进类型安全。
src/packages/uploader/uploader.harmony.css 新增进度组件及相关样式,更新现有样式。
src/packages/uploader/uploader.taro.tsx 修改UploaderProps接口,新增upload方法,移除多个属性。
src/packages/uploader/uploader.tsx 修改UploaderProps接口,新增upload方法,移除多个属性。

Possibly related PRs

Suggested reviewers

  • oasis-cloud
  • xiaoyatong

兔子在草地上跳跃,
版本更新真是妙,
新组件齐上阵,
上传功能更流畅。
让我们一起欢庆,
代码更美好,
兔子乐陶陶! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b3267a0 and 5959362.

📒 Files selected for processing (1)
  • package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

🛑 Comments failed to post (58)
src/packages/uploader/demos/h5/demo6.tsx (1)

4-11: 🛠️ Refactor suggestion

Enhance demo clarity and user experience.

As a demo component, this implementation could be improved in several ways:

  1. Add explanatory text within the Cell to describe the purpose of a disabled uploader
  2. Include a default file list or preview to demonstrate how disabled state affects existing uploads
  3. Consider adding props like maxCount or previewType to showcase more features even in disabled state

Consider this enhancement:

 const Demo6 = () => {
   return (
-    <Cell style={{ flexWrap: 'wrap' }}>
-      <Uploader disabled />
+    <Cell style={{ flexWrap: 'wrap' }}>
+      <Cell.Description>Disabled uploader state demonstration</Cell.Description>
+      <Uploader
+        disabled
+        defaultFileList={[
+          {
+            name: 'example.png',
+            url: 'https://example.com/example.png',
+            status: 'success'
+          }
+        ]}
+        maxCount={3}
+        previewType="list"
+      />
     </Cell>
   )
 }
📝 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 Demo6 = () => {
  return (
    <Cell style={{ flexWrap: 'wrap' }}>
      <Cell.Description>Disabled uploader state demonstration</Cell.Description>
      <Uploader
        disabled
        defaultFileList={[
          {
            name: 'example.png',
            url: 'https://example.com/example.png',
            status: 'success'
          }
        ]}
        maxCount={3}
        previewType="list"
      />
    </Cell>
  )
}
export default Demo6
src/packages/uploader/demos/taro/demo6.tsx (1)

6-8: 🛠️ Refactor suggestion

Consider enhancing the UX for disabled state.

The disabled uploader might benefit from additional visual cues or explanatory text to inform users why uploads are not allowed.

 return (
   <Cell style={{ flexWrap: 'wrap' }}>
-    <Uploader disabled />
+    <Uploader 
+      disabled 
+      placeholder="Uploads are currently disabled"
+    />
   </Cell>
 )
📝 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.

    <Cell style={{ flexWrap: 'wrap' }}>
      <Uploader 
        disabled 
        placeholder="Uploads are currently disabled"
      />
    </Cell>
src/packages/uploader/demos/h5/demo8.tsx (2)

12-17: ⚠️ Potential issue

Add error handling and cleanup URL object references.

The current implementation has potential memory leaks and lacks error handling.

Consider implementing these improvements:

   async function upload(file: File) {
-    await sleep(2000)
-    return {
-      url: URL.createObjectURL(file),
-    }
+    try {
+      await sleep(2000)
+      const objectUrl = URL.createObjectURL(file)
+      // Consider adding this cleanup function to component's cleanup logic
+      // window.addEventListener('unload', () => URL.revokeObjectURL(objectUrl))
+      return {
+        url: objectUrl,
+      }
+    } catch (error) {
+      console.error('Upload failed:', error)
+      throw new Error('Upload failed')
+    }
   }

Committable suggestion was skipped due to low confidence.


19-21: 🛠️ Refactor suggestion

Enhance user experience with loading and error states.

The current implementation lacks visual feedback during upload and error handling.

Consider these improvements:

   <Cell style={{ flexWrap: 'wrap' }}>
-    <Uploader capture upload={(file: File) => upload(file)} />
+    <Uploader
+      capture
+      upload={(file: File) => upload(file)}
+      onProgress={(event) => {
+        // Show upload progress
+        console.log('Upload progress:', event.percent)
+      }}
+      onError={(error) => {
+        // Handle and display upload errors
+        console.error('Upload error:', error)
+      }}
+    />
   </Cell>

Also, consider adding a comment explaining the purpose of the capture prop for better documentation.

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demos/taro/demo8.tsx (4)

5-11: 🛠️ Refactor suggestion

Consider improving type safety of the sleep utility function.

The sleep function could benefit from better type definitions and error handling.

-  function sleep(time: number) {
-    return new Promise<void>((resolve) => {
+  function sleep(time: number): Promise<void> {
+    if (time < 0) throw new Error('Sleep time cannot be negative')
+    return new Promise<void>((resolve) => {
       setTimeout(() => {
         resolve()
       }, time)
     })
   }
📝 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.

  function sleep(time: number): Promise<void> {
    if (time < 0) throw new Error('Sleep time cannot be negative')
    return new Promise<void>((resolve) => {
      setTimeout(() => {
        resolve()
      }, time)
    })
  }

19-21: 🛠️ Refactor suggestion

Improve type safety and documentation of component props.

Several improvements could be made to enhance type safety and maintainability:

-    <Cell style={{ flexWrap: 'wrap' }}>
-      <Uploader capture upload={(file: File) => upload(file)} />
+    <Cell style={{ flexWrap: 'wrap' as const }}>
+      <Uploader
+        capture // Consider documenting the purpose of this prop
+        upload={(file: File): Promise<{ url: string }> => upload(file)}
+      />

Committable suggestion was skipped due to low confidence.


12-17: ⚠️ Potential issue

Address potential memory leak in upload function.

The URL.createObjectURL creates a new object URL that should be revoked when no longer needed to prevent memory leaks. Also, consider adding proper typing for the return value.

-  async function upload(file: File) {
+  async function upload(file: File): Promise<{ url: string }> {
     await sleep(2000)
+    const url = URL.createObjectURL(file)
+    // In a real implementation, you might want to store this URL to revoke it later
+    // Consider implementing a cleanup mechanism or documenting the need to call URL.revokeObjectURL
     return {
-      url: URL.createObjectURL(file),
+      url,
     }
   }

Committable suggestion was skipped due to low confidence.


1-22: 💡 Codebase verification

Based on the documentation files and demo content reviewed, I can now provide the final response:

Documentation improvements needed for Demo8's capture functionality

The review comment is valid. While the documentation does explain the capture prop in the Props table, the demo itself lacks proper documentation. Here's why this needs attention:

  1. The documentation shows that Demo8 specifically demonstrates "直接调起摄像头" (Directly activate the camera) functionality, but the demo code lacks comments explaining this important use case.
  2. The current implementation with a 2-second delay is indeed not representative of real upload scenarios and lacks error handling.
  3. Other demos in the codebase also lack proper documentation, making this a consistent issue across demo files.

Suggested improvements:

  • Add comments explaining that the capture prop is used to directly activate the device camera on mobile devices
  • Include error handling in the upload function to demonstrate proper error management
  • Consider adding a more realistic upload simulation or at least a comment explaining that the delay is for demonstration purposes only
🔗 Analysis chain

Consider enhancing this demo with additional documentation and error handling.

As this is a demo file showcasing the Uploader 3.0 functionality:

  1. Add comments explaining the purpose of the capture prop and its use cases
  2. Consider showing error handling scenarios
  3. The 2-second delay might not be representative of real upload scenarios

Let's check if other demo files include proper documentation:


Let me modify the search pattern to correctly check for documentation in demo files.

This will help us:

  1. Identify all demo files in the directory
  2. Look for both single-line and multi-line comments in these files to understand the documentation patterns

Let me try a different approach to check the content of these demo files, focusing on their structure and any existing documentation.


Let me check the documentation files to understand how the demos are documented in the component's documentation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check documentation patterns in other demo files
rg -l "\/\*\*|\* @" "src/packages/uploader/demos/taro/"

Length of output: 133


Script:

#!/bin/bash
# Check documentation patterns in other demo files
# First, let's see all demo files
fd "demo\d+\.tsx$" "src/packages/uploader/demos/taro/"

# Then check for comments in these files
rg -U "(/\*\*|//)" "src/packages/uploader/demos/taro/"

Length of output: 2713


Script:

#!/bin/bash
# Let's examine the content of a few demo files to understand their structure
echo "=== Demo1 Content ==="
cat src/packages/uploader/demos/taro/demo1.tsx

echo -e "\n=== Demo8 Content (the one under review) ==="
cat src/packages/uploader/demos/taro/demo8.tsx

# Let's also check if there's any documentation in the parent directory
echo -e "\n=== Checking for documentation files in parent directory ==="
ls src/packages/uploader/

Length of output: 2746


Script:

#!/bin/bash
# Check the documentation files for any mention of the capture prop and demo documentation
echo "=== Checking doc.taro.md ==="
cat src/packages/uploader/doc.taro.md

echo -e "\n=== Checking doc.en-US.md ==="
cat src/packages/uploader/doc.en-US.md

Length of output: 12935

src/packages/uploader/demos/h5/demo7.tsx (1)

24-29: 🛠️ Refactor suggestion

Consider enhancing the demo with different configurations.

Both uploaders currently share identical defaultValue. Consider differentiating them to better showcase the component's versatility.

 <Uploader
-  defaultValue={defaultFileList}
+  defaultValue={[{
+    name: 'custom-icon-demo-2.jpg',
+    url: 'https://m.360buyimg.com/babel/different-image-url.jpg',
+    message: '使用自定义图标上传',
+  }]}
   uploadIcon={<Star />}
   deleteIcon={<Dongdong />}
 />
📝 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.

      <Uploader
        defaultValue={[{
          name: 'custom-icon-demo-2.jpg',
          url: 'https://m.360buyimg.com/babel/different-image-url.jpg',
          message: '使用自定义图标上传',
        }]}
        uploadIcon={<Star />}
        deleteIcon={<Dongdong />}
      />
    </Cell>
src/packages/uploader/demos/taro/demo7.tsx (1)

15-29: 🛠️ Refactor suggestion

Consider enhancing the demo's educational value.

While the implementation effectively demonstrates icon customization, consider these improvements:

  1. Each Uploader instance could use different defaultValue data to better demonstrate independence
  2. Add comments explaining the purpose of each configuration
 <Cell style={{ flexWrap: 'wrap' }}>
+  {/* Example: Custom upload and delete icons */}
   <Uploader
-    defaultValue={defaultFileList}
+    defaultValue={[{ name: 'custom-upload-icon.png', url: '...', message: 'Using Dongdong icon' }]}
     uploadIcon={<Dongdong />}
     deleteIcon={<Star />}
     style={{
       marginInlineEnd: '10px',
     }}
   />
+  {/* Example: Reversed icon configuration */}
   <Uploader
-    defaultValue={defaultFileList}
+    defaultValue={[{ name: 'custom-delete-icon.png', url: '...', message: 'Using Star icon' }]}
     uploadIcon={<Star />}
     deleteIcon={<Dongdong />}
   />
 </Cell>

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demos/h5/demo4.tsx (2)

24-33: 🛠️ Refactor suggestion

Enhance component configuration and accessibility.

Consider these improvements:

  1. Extract inline styles to constants or CSS
  2. Define maxFileSize as a named constant
  3. Add accessibility attributes

Here's the suggested implementation:

+ const MAX_FILE_SIZE = 1024 * 50 // 50KB
+ const cellStyles = { flexWrap: 'wrap' as const }

  return (
-   <Cell style={{ flexWrap: 'wrap' }}>
+   <Cell style={cellStyles}>
      <Uploader
        value={list}
        onChange={setList}
        upload={(file: File) => upload(file)}
        multiple
-       maxFileSize={1024 * 50}
+       maxFileSize={MAX_FILE_SIZE}
        onOversize={onOversize}
+       aria-label="File uploader"
+       role="button"
      />
    </Cell>
  )

Committable suggestion was skipped due to low confidence.


10-22: 🛠️ Refactor suggestion

Improve helper functions implementation.

Consider these enhancements:

  1. Add JSDoc comments for better documentation
  2. Clean up object URLs to prevent memory leaks
  3. Consider reducing the delay time for better demo experience

Here's the suggested implementation:

+ /**
+  * Utility function to create a delayed promise
+  * @param time - Delay time in milliseconds
+  * @returns Promise that resolves after the specified time
+  */
  function sleep(time: number) {
    return new Promise<void>((resolve) => {
      setTimeout(() => {
        resolve()
      }, time)
    })
  }

+ /**
+  * Simulates file upload with artificial delay
+  * @param file - File to be uploaded
+  * @returns Promise with upload result containing URL
+  */
  async function upload(file: File) {
-   await sleep(2000)
+   await sleep(1000) // Reduced delay for better demo experience
+   const url = URL.createObjectURL(file)
    return {
-     url: URL.createObjectURL(file),
+     url,
+     cleanup: () => URL.revokeObjectURL(url), // Add cleanup function
    }
  }
📝 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.

  /**
   * Utility function to create a delayed promise
   * @param time - Delay time in milliseconds
   * @returns Promise that resolves after the specified time
   */
  function sleep(time: number) {
    return new Promise<void>((resolve) => {
      setTimeout(() => {
        resolve()
      }, time)
    })
  }

  /**
   * Simulates file upload with artificial delay
   * @param file - File to be uploaded
   * @returns Promise with upload result containing URL
   */
  async function upload(file: File) {
    await sleep(1000) // Reduced delay for better demo experience
    const url = URL.createObjectURL(file)
    return {
      url,
      cleanup: () => URL.revokeObjectURL(url), // Add cleanup function
    }
  }
src/packages/uploader/demos/taro/demo4.tsx (1)

17-22: ⚠️ Potential issue

Add cleanup and error handling to upload function.

The current implementation might lead to memory leaks due to uncleaned object URLs.

Consider this improvement:

   async function upload(file: File) {
-    await sleep(2000)
-    return {
-      url: URL.createObjectURL(file),
-    }
+    try {
+      await sleep(2000)
+      const url = URL.createObjectURL(file)
+      // Add cleanup when the URL is no longer needed
+      setTimeout(() => URL.revokeObjectURL(url), 0)
+      return { url }
+    } catch (error) {
+      console.error('Upload failed:', error)
+      throw 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.

  async function upload(file: File) {
    try {
      await sleep(2000)
      const url = URL.createObjectURL(file)
      // Add cleanup when the URL is no longer needed
      setTimeout(() => URL.revokeObjectURL(url), 0)
      return { url }
    } catch (error) {
      console.error('Upload failed:', error)
      throw error
    }
  }
src/packages/uploader/demos/taro/demo3.tsx (2)

7-10: 🛠️ Refactor suggestion

Consider moving the style object outside the component.

The style object is static and doesn't depend on component state or props. Moving it outside the component would prevent unnecessary recreation on each render.

+const uploaderStyle = {
+  marginInlineEnd: '10px',
+  marginBottom: '10px',
+}

const Demo3 = () => {
  const [list, setList] = useState<FileItem[]>([])
-  const uploaderStyle = {
-    marginInlineEnd: '10px',
-    marginBottom: '10px',
-  }

Committable suggestion was skipped due to low confidence.


18-22: ⚠️ Potential issue

Handle URL.createObjectURL cleanup and add error handling.

The current implementation has two potential issues:

  1. URLs created by URL.createObjectURL should be revoked when no longer needed to prevent memory leaks
  2. The upload function lacks error handling for failed uploads

Consider implementing this improved version:

 async function upload(file: File) {
+  try {
     await sleep(2000)
+    const url = URL.createObjectURL(file)
+    // Add cleanup logic when the URL is no longer needed
+    // You might want to store these URLs in a ref and clean them up when files are removed
     return {
-      url: URL.createObjectURL(file),
+      url,
     }
+  } catch (error) {
+    console.error('Upload failed:', error)
+    throw new Error('Upload failed')
+  }
 }

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demos/h5/demo5.tsx (3)

30-37: 🛠️ Refactor suggestion

Enhance component with loading and error states.

The current implementation could benefit from better user feedback and accessibility:

  1. Add loading state handling
  2. Add error state handling
  3. Improve accessibility

Here's a suggested improvement:

  <Cell style={{ flexWrap: 'wrap' }}>
    <Uploader
      value={list}
      onChange={setList}
      beforeUpload={beforeUpload}
      upload={(file: File) => upload(file)}
+     aria-label="Upload PNG images"
+     onError={(error) => alert(error.message)}
+     loading={isUploading}
    />
  </Cell>

Committable suggestion was skipped due to low confidence.


6-15: ⚠️ Potential issue

Remove debug logs and enhance user feedback in beforeUpload.

The beforeUpload implementation has several areas for improvement:

  1. Remove console.log statements from production code
  2. Add user feedback when files are rejected
  3. Make the allowed file types more maintainable

Here's a suggested improvement:

  const [list, setList] = useState<FileItem[]>([])
+ const ALLOWED_TYPES = ['image/png']
  const beforeUpload = async (files: File[]) => {
-   console.log('bbb')
    const allowedTypes = ['image/png']
    const filteredFiles = Array.from(files).filter((file) =>
-     allowedTypes.includes(file.type)
+     ALLOWED_TYPES.includes(file.type)
    )
-   console.log(filteredFiles)
+   if (filteredFiles.length < files.length) {
+     alert('Only PNG images are allowed')
+   }
    return filteredFiles
  }
📝 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 [list, setList] = useState<FileItem[]>([])
  const ALLOWED_TYPES = ['image/png']
  const beforeUpload = async (files: File[]) => {
    const allowedTypes = ['image/png']
    const filteredFiles = Array.from(files).filter((file) =>
      ALLOWED_TYPES.includes(file.type)
    )
    if (filteredFiles.length < files.length) {
      alert('Only PNG images are allowed')
    }
    return filteredFiles
  }

16-27: ⚠️ Potential issue

Add error handling and cleanup to upload function.

The current implementation has potential memory leaks and lacks error handling:

  1. Object URLs should be revoked when no longer needed
  2. No file size validation
  3. Missing error handling for URL.createObjectURL

Here's a suggested improvement:

+ const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB
  async function upload(file: File) {
+   if (file.size > MAX_FILE_SIZE) {
+     throw new Error('File size exceeds 5MB limit');
+   }
    await sleep(2000)
+   try {
+     const url = URL.createObjectURL(file)
      return {
-       url: URL.createObjectURL(file),
+       url,
+       cleanup: () => URL.revokeObjectURL(url),
      }
+   } catch (error) {
+     throw new Error('Failed to process file');
+   }
  }
📝 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 MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB
  function sleep(time: number) {
    return new Promise<void>((resolve) => {
      setTimeout(() => {
        resolve()
      }, time)
    })
  }
  async function upload(file: File) {
    if (file.size > MAX_FILE_SIZE) {
      throw new Error('File size exceeds 5MB limit');
    }
    await sleep(2000)
    try {
      const url = URL.createObjectURL(file)
      return {
        url,
        cleanup: () => URL.revokeObjectURL(url),
      }
    } catch (error) {
      throw new Error('Failed to process file');
    }
  }
src/packages/uploader/demos/taro/demo5.tsx (1)

23-28: ⚠️ Potential issue

Add error handling and cleanup for Object URLs.

The current implementation might leak memory as Object URLs aren't being revoked. Also, error handling should be added.

Here's a suggested improvement:

 async function upload(file: File) {
   await sleep(2000)
+  try {
+    const url = URL.createObjectURL(file)
+    // In a real implementation, you would want to revoke the URL after it's no longer needed
+    // URL.revokeObjectURL(url)
     return {
-      url: URL.createObjectURL(file),
+      url,
     }
+  } catch (error) {
+    console.error('Failed to create object URL:', error)
+    throw new Error('Failed to process file')
+  }
 }

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demos/h5/demo9.tsx (5)

31-53: 🛠️ Refactor suggestion

Consider improvements for maintainability and internationalization.

A few suggestions to enhance the code:

  1. Consider extracting inline styles to a styled-component or CSS module
  2. Text content should be internationalized for better maintainability

Example approach:

const messages = {
  upload: '执行上传',
  clear: '手动清空上传'
}

const StyledCell = styled(Cell)`
  display: flex;
  flex-direction: column;
`

const ButtonContainer = styled.div`
  margin-bottom: 10px;
`

const UploadButton = styled(Button)`
  margin-inline-end: 10px;
`

18-23: ⚠️ Potential issue

Add cleanup for object URLs to prevent memory leaks.

The URL.createObjectURL creates a new object URL for each file, which should be revoked when no longer needed to prevent memory leaks.

Consider implementing cleanup:

 async function upload(file: File) {
   await sleep(2000)
+  const objectUrl = URL.createObjectURL(file)
+  // Add cleanup logic when the URL is no longer needed
+  setTimeout(() => URL.revokeObjectURL(objectUrl), 0)
   return {
-    url: URL.createObjectURL(file),
+    url: objectUrl,
   }
 }
📝 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.

  async function upload(file: File) {
    await sleep(2000)
    const objectUrl = URL.createObjectURL(file)
    // Add cleanup logic when the URL is no longer needed
    setTimeout(() => URL.revokeObjectURL(objectUrl), 0)
    return {
      url: objectUrl,
    }
  }

32-39: ⚠️ Potential issue

Convert maxCount to number type.

The maxCount prop is passed as a string but should be a number for better type safety.

 <Uploader
-  maxCount="5"
+  maxCount={5}
   multiple
   autoUpload={false}
   ref={uploadRef}
   upload={(file: File) => upload(file)}
   style={{ marginBottom: 10 }}
 />
📝 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.

      <Uploader
        maxCount={5}
        multiple
        autoUpload={false}
        ref={uploadRef}
        upload={(file: File) => upload(file)}
        style={{ marginBottom: 10 }}
      />

24-29: 🛠️ Refactor suggestion

Add null checks for safer ref handling.

The current implementation uses type assertion which could be unsafe. Consider adding null checks for better type safety.

 const submitUpload = () => {
-  ;(uploadRef.current as uploadRefState).submit()
+  if (uploadRef.current) {
+    uploadRef.current.submit()
+  }
 }

 const clearUpload = () => {
-  ;(uploadRef.current as uploadRefState).clear()
+  if (uploadRef.current) {
+    uploadRef.current.clear()
+  }
 }
📝 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 submitUpload = () => {
    if (uploadRef.current) {
      uploadRef.current.submit()
    }
  }
  const clearUpload = () => {
    if (uploadRef.current) {
      uploadRef.current.clear()
    }
  }

11-17: 🛠️ Refactor suggestion

Consider moving utility functions outside component.

The sleep utility function is recreated on each render. Consider moving it outside the component or using useCallback if component-specific state is needed.

+const sleep = (time: number) => {
+  return new Promise<void>((resolve) => {
+    setTimeout(resolve, time)
+  })
+}

 const Demo9 = () => {
-  function sleep(time: number) {
-    return new Promise<void>((resolve) => {
-      setTimeout(() => {
-        resolve()
-      }, time)
-    })
-  }
📝 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 sleep = (time: number) => {
  return new Promise<void>((resolve) => {
    setTimeout(resolve, time)
  })
}

const Demo9 = () => {
src/packages/uploader/demos/taro/demo9.tsx (3)

31-53: 🛠️ Refactor suggestion

Consider these improvements for better maintainability.

  1. The maxCount prop should be a number instead of a string
  2. Consider extracting inline styles to a stylesheet
  3. Button text should be internationalized

Here's how you can improve it:

-      <Uploader
-        maxCount="5"
+      <Uploader
+        maxCount={5}
         multiple
         autoUpload={false}
         ref={uploadRef}
         upload={(file: File) => upload(file)}
-        style={{ marginBottom: 10 }}
+        className="demo-uploader"
       />

Consider creating a separate stylesheet:

.demo-uploader {
  margin-bottom: 10px;
}

And use i18n for button text:

-          执行上传
+          {t('upload.submit')}
-          手动清空上传
+          {t('upload.clear')}

24-29: ⚠️ Potential issue

Implement safer ref handling.

The current type assertions could throw runtime errors if the ref is null.

Consider adding null checks:

 const submitUpload = () => {
-  ;(uploadRef.current as uploadRefState).submit()
+  if (uploadRef.current) {
+    uploadRef.current.submit()
+  }
 }
 const clearUpload = () => {
-  ;(uploadRef.current as uploadRefState).clear()
+  if (uploadRef.current) {
+    uploadRef.current.clear()
+  }
 }
📝 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 submitUpload = () => {
    if (uploadRef.current) {
      uploadRef.current.submit()
    }
  }
  const clearUpload = () => {
    if (uploadRef.current) {
      uploadRef.current.clear()
    }
  }

18-23: ⚠️ Potential issue

Add error handling and cleanup for URL.createObjectURL.

The upload function has two potential issues:

  1. No error handling for failed uploads
  2. URL.createObjectURL creates a memory leak if not revoked

Consider implementing this improved version:

 async function upload(file: File) {
-  await sleep(2000)
-  return {
-    url: URL.createObjectURL(file),
-  }
+  try {
+    await sleep(2000)
+    const objectUrl = URL.createObjectURL(file)
+    // Consider revoking the URL when the upload is cleared or component unmounts
+    return {
+      url: objectUrl,
+      onRemove: () => URL.revokeObjectURL(objectUrl),
+    }
+  } catch (error) {
+    console.error('Upload failed:', error)
+    throw new Error('Upload failed')
+  }
 }
📝 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.

  async function upload(file: File) {
    try {
      await sleep(2000)
      const objectUrl = URL.createObjectURL(file)
      // Consider revoking the URL when the upload is cleared or component unmounts
      return {
        url: objectUrl,
        onRemove: () => URL.revokeObjectURL(objectUrl),
      }
    } catch (error) {
      console.error('Upload failed:', error)
      throw new Error('Upload failed')
    }
  }
src/packages/uploader/demos/h5/demo1.tsx (2)

17-23: 🛠️ Refactor suggestion

Consider moving the sleep utility to a shared utilities file.

The sleep function is a generic utility that could be useful across multiple demos. Consider moving it to a shared utilities file to promote code reuse.

+// src/packages/shared/utils.ts
+export function sleep(time: number): Promise<void> {
+  return new Promise<void>((resolve) => {
+    setTimeout(resolve, time)
+  })
+}

-  function sleep(time: number) {
-    return new Promise<void>((resolve) => {
-      setTimeout(() => {
-        resolve()
-      }, time)
-    })
-  }

Committable suggestion was skipped due to low confidence.


24-32: ⚠️ Potential issue

Address potential memory leak and improve error handling.

  1. The URL.createObjectURL creates a URL that should be revoked when no longer needed to prevent memory leaks.
  2. The error message in uploadFail could be more descriptive.
  3. Consider adding JSDoc comments to document the artificial delays.
+/**
+ * Simulates a successful file upload with a 2-second delay
+ * @param file The file to upload
+ * @returns A promise that resolves with the file URL
+ */
 async function upload(file: File) {
   await sleep(2000)
+  const url = URL.createObjectURL(file)
+  // TODO: Implement cleanup to revoke the URL when it's no longer needed
   return {
-    url: URL.createObjectURL(file),
+    url,
   }
 }

+/**
+ * Simulates a failed file upload with a 2-second delay
+ * @param file The file that failed to upload
+ * @throws Error with detailed message
+ */
 async function uploadFail(file: File): Promise<FileItem> {
   await sleep(2000)
-  throw new Error('Fail to upload')
+  throw new Error(`Failed to upload file: ${file.name}. Please try again.`)
 }
📝 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.

  /**
   * Simulates a successful file upload with a 2-second delay
   * @param file The file to upload
   * @returns A promise that resolves with the file URL
   */
  async function upload(file: File) {
    await sleep(2000)
    const url = URL.createObjectURL(file)
    // TODO: Implement cleanup to revoke the URL when it's no longer needed
    return {
      url,
    }
  }

  /**
   * Simulates a failed file upload with a 2-second delay
   * @param file The file that failed to upload
   * @throws Error with detailed message
   */
  async function uploadFail(file: File): Promise<FileItem> {
    await sleep(2000)
    throw new Error(`Failed to upload file: ${file.name}. Please try again.`)
src/packages/uploader/demos/taro/demo1.tsx (2)

24-31: 🛠️ Refactor suggestion

Improve upload function implementation.

Several improvements can be made to the upload function:

  1. The created Blob is never used
  2. Console.log should be removed from production code
  3. The hardcoded URL makes the Blob creation unnecessary
  async function upload(file: File) {
    await sleep(2000)
-   const blob = new Blob([file], { type: 'image/png' })
-   const url = URL.createObjectURL(blob)
-   console.log(url)
    return {
      url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
    }
  }
📝 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.

  async function upload(file: File) {
    await sleep(2000)
    return {
      url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
    }

40-46: 🛠️ Refactor suggestion

Enhance user feedback and accessibility.

The Uploader component could benefit from additional props to improve user experience:

  1. Add error message display
  2. Show loading states during upload
  3. Include accessibility attributes
  <Uploader
    uploadLabel="商品主图"
    value={list}
    onChange={setList}
    upload={(file: File) => upload(file)}
    style={uploaderStyle}
+   aria-label="Upload product main image"
+   onError={(err) => console.error(err)}
+   loading={false}
  />

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demos/h5/demo2.tsx (1)

46-51: 🛠️ Refactor suggestion

Consider enhancing the demo with more interactive features.

While the current implementation shows various file states, it could be more educational by demonstrating:

  1. Actual file upload handling
  2. Error validation
  3. Size/type restrictions

Consider adding these features:

 <Uploader
   defaultValue={defaultList}
   onDelete={onDelete}
   uploadIcon={<Dongdong />}
   maxCount={6}
+  maxSize={5 * 1024 * 1024}
+  accept="image/*"
+  onOversize={(file) => console.log('File too large:', file)}
+  onUpload={(file) => {
+    // Demo upload simulation
+    return new Promise((resolve) => {
+      setTimeout(() => resolve({ url: URL.createObjectURL(file) }), 1000)
+    })
+  }}
 />
📝 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.

    <Uploader
      defaultValue={defaultList}
      onDelete={onDelete}
      uploadIcon={<Dongdong />}
      maxCount={6}
      maxSize={5 * 1024 * 1024}
      accept="image/*"
      onOversize={(file) => console.log('File too large:', file)}
      onUpload={(file) => {
        // Demo upload simulation
        return new Promise((resolve) => {
          setTimeout(() => resolve({ url: URL.createObjectURL(file) }), 1000)
        })
      }}
    />
src/packages/uploader/demos/taro/demo2.tsx (2)

46-51: 🛠️ Refactor suggestion

Consider showcasing more Uploader features.

While the current demo effectively shows file status handling, consider enhancing it with additional features:

  • File type restrictions (accept prop)
  • File size limits (maxSize prop)
  • Custom validation rules

Example enhancement:

 <Uploader
   defaultValue={defaultList}
   onDelete={onDelete}
   uploadIcon={<Dongdong />}
   maxCount={6}
+  accept="image/*"
+  maxSize={5 * 1024 * 1024}
+  onOversize={(files) => console.log('File too large:', files)}
 />

Committable suggestion was skipped due to low confidence.


7-41: 💡 Codebase verification

Based on the search results, I can now provide a final response:

Consider using local assets from the repository for the demo

The search reveals that:

  1. The specific image URL 360buyimg.com is widely used across multiple demo files
  2. The repository already has a dedicated assets directory at src/sites/assets/images/ with many local images

Recommendations:

  • Store demo images in src/sites/assets/images/ alongside other local assets
  • Create a reusable demo assets constant file to maintain URLs in one place
  • Use relative imports to reference these local assets instead of external URLs

This will:

  • Prevent broken demos if external resources become unavailable
  • Improve load times and reliability
  • Follow the existing pattern of local asset usage seen in other parts of the codebase
🔗 Analysis chain

Consider using local assets for the demo.

The demo currently uses external image URLs which could lead to broken demos if the external resource becomes unavailable.

Let's check if these image URLs are used elsewhere in the codebase:

Consider:

  1. Using local demo assets to prevent external dependencies
  2. Creating a demo assets constant file to maintain URLs in one place
  3. Implementing proper error handling for image loading failures

Let me try a different search approach to verify the usage of these image URLs.

I'm searching for both the URL pattern across all files and checking if there are any local image assets already present in the repository. This will help us:

  1. Verify if these external URLs are used elsewhere
  2. Check if there are existing local assets we could potentially use instead
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the hardcoded image URL usage
rg "360buyimg.com" --type tsx --type ts

Length of output: 67


Script:

#!/bin/bash
# Search for the image URL pattern without file type restrictions
rg "360buyimg\.com"

# Also search for any demo-related image files in the repository
fd -e png -e gif -e jpg -e jpeg -e webp

Length of output: 73275

src/packages/uploader/demos/h5/demo10.tsx (3)

7-40: 🛠️ Refactor suggestion

Consider improving type safety and demo clarity.

A few suggestions to enhance the demo:

  1. Define an enum for file status values to ensure type safety
  2. Use more generic/English file names for better international understanding
  3. Consider using different mock URLs for each file to better demonstrate variety

Here's a suggested improvement:

enum FileStatus {
  SUCCESS = 'success',
  ERROR = 'error',
  UPLOADING = 'uploading'
}

const [list, setList] = useState<FileItem[]>([
  {
    name: 'document1.png',
    url: 'https://example.com/sample1.gif',
    status: FileStatus.SUCCESS,
    message: 'Upload successful',
  },
  // ... other items with different URLs and English text
])

58-70: ⚠️ Potential issue

Enhance component props and accessibility.

A few suggestions for improvement:

  1. maxCount should be a number, not a string
  2. Consider adding error handling feedback
  3. Add accessibility attributes for better user experience

Here's the suggested improvement:

 <Uploader
   upload={(file: File) => upload(file)}
   value={list}
   onChange={setList}
-  maxCount="10"
+  maxCount={10}
   multiple
   previewType="list"
   style={{ marginBottom: 20 }}
+  aria-label="File upload"
+  onError={(error) => console.error('Upload failed:', error)}
 >
-  <Button type="success" size="small">
+  <Button type="success" size="small" aria-label="Upload files">
     上传文件
   </Button>
 </Uploader>
📝 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.

    <Uploader
      upload={(file: File) => upload(file)}
      value={list}
      onChange={setList}
      maxCount={10}
      multiple
      previewType="list"
      style={{ marginBottom: 20 }}
      aria-label="File upload"
      onError={(error) => console.error('Upload failed:', error)}
    >
      <Button type="success" size="small" aria-label="Upload files">
        上传文件
      </Button>
    </Uploader>

41-55: ⚠️ Potential issue

Improve upload simulation reliability and resource management.

Several improvements could be made to the upload simulation:

  1. The 50% failure rate might be too high for demonstration purposes
  2. The error message could be more descriptive
  3. URL.createObjectURL creates a memory leak if not revoked

Here's the suggested fix:

 async function upload(file: File) {
   await sleep(2000)
-  if (Math.random() < 0.5) {
+  if (Math.random() < 0.2) { // 20% failure rate
+    throw new Error(`Failed to upload ${file.name}: Network error`)
+  }
+  const objectUrl = URL.createObjectURL(file)
+  // Add cleanup function to component
   return {
-    url: URL.createObjectURL(file),
+    url: objectUrl,
+    cleanup: () => URL.revokeObjectURL(objectUrl),
   }
-  }
-  throw new Error('Fail to upload')
 }

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demos/taro/demo10.tsx (2)

58-66: 🛠️ Refactor suggestion

Consider type improvements and consistent styling approach.

  1. The maxCount prop should be a number instead of a string
  2. Consider moving inline styles to a styling system for better maintainability
 <Uploader
   upload={(file: File) => upload(file)}
   value={list}
   onChange={setList}
-  maxCount="10"
+  maxCount={10}
   multiple
   previewType="list"
-  style={{ marginBottom: 20 }}
+  className="demo-uploader"
 >

Committable suggestion was skipped due to low confidence.


48-56: ⚠️ Potential issue

Fix potential memory leak with URL.createObjectURL.

The URL.createObjectURL creates a new object URL that persists until the document is unloaded. To prevent memory leaks, you should revoke the URL when it's no longer needed.

 async function upload(file: File) {
   await sleep(2000)
   if (Math.random() < 0.5) {
+    const objectUrl = URL.createObjectURL(file)
+    // Consider revoking the URL when the file is removed or component unmounts
     return {
-      url: URL.createObjectURL(file),
+      url: objectUrl,
     }
   }
-  throw new Error('Fail to upload')
+  throw new Error(`Failed to upload ${file.name}: Simulated random failure`)
 }

Committable suggestion was skipped due to low confidence.

src/packages/uploader/demo.taro.tsx (1)

57-78: 🛠️ Refactor suggestion

Consider using semantic HTML structure.

While the current structure works, using semantic HTML elements would improve accessibility and document outline.

-      <div className="demo bg-w">
+      <section className="demo bg-w">
-        <h2>{translated.basic}</h2>
+        <h2 id="basic">{translated.basic}</h2>
         <Demo1 />
+        <section aria-labelledby="basic">
+          <h3>{translated.uploadStatus}</h3>
           <Demo2 />
+        </section>
         /* ... other demos ... */
-      </div>
+      </section>

This change would:

  • Improve document structure with section elements
  • Add ARIA landmarks for better accessibility
  • Create a clearer content hierarchy

Committable suggestion was skipped due to low confidence.

src/packages/uploader/upload.ts (2)

49-58: ⚠️ Potential issue

Fix duplicate success callback and add proper error handling.

There are several issues in the XHR response handling:

  1. The success callback is incorrectly called twice (lines 49 and 53)
  2. Missing timeout error handling
  3. No cleanup of event listeners

Apply this diff to fix these issues:

-    options.onSuccess?.(xhr.responseText, options)
     xhr.onreadystatechange = () => {
       if (xhr.readyState === 4) {
         if (xhr.status === options.xhrState) {
           options.onSuccess?.(xhr.responseText, options)
         } else {
           options.onFailure?.(xhr.responseText, options)
         }
       }
     }
+    xhr.ontimeout = () => {
+      options.onFailure?.('Upload timeout', options)
+    }
+    xhr.onerror = () => {
+      options.onFailure?.('Network error', options)
+    }
📝 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.

    xhr.onreadystatechange = () => {
      if (xhr.readyState === 4) {
        if (xhr.status === options.xhrState) {
          options.onSuccess?.(xhr.responseText, options)
        } else {
          options.onFailure?.(xhr.responseText, options)
        }
      }
    }
    xhr.ontimeout = () => {
      options.onFailure?.('Upload timeout', options)
    }
    xhr.onerror = () => {
      options.onFailure?.('Network error', options)
    }

61-68: 🛠️ Refactor suggestion

Consider improving type safety and hook interface.

The current implementation could benefit from better type safety and a more structured hook interface.

Consider these improvements:

-  headers = {}
+  headers: Record<string, string> = {}

-  beforeXhrUpload?: any
+  beforeXhrUpload?: (xhr: XMLHttpRequest, options: UploadOptions) => boolean | void

   // In the upload method
-  if (options.beforeXhrUpload) {
-    options.beforeXhrUpload(xhr, options)
+  if (options.beforeXhrUpload && options.beforeXhrUpload(xhr, options) !== false) {
+    xhr.send(options.formData)
   } else {
     xhr.send(options.formData)
   }

This change:

  1. Adds proper typing for headers
  2. Improves the beforeXhrUpload hook interface with proper types
  3. Allows the hook to control whether the upload should proceed

Committable suggestion was skipped due to low confidence.

src/packages/uploader/preview.tsx (3)

17-17: 🛠️ Refactor suggestion

Consider using TypeScript enum or constants for status values.

Using string literals for status checks can be error-prone. Consider defining an enum or constants for status values to improve type safety and maintainability.

// Create a new file: src/packages/uploader/constants.ts
export enum UploaderStatus {
  ERROR = 'error',
  SUCCESS = 'success',
  READY = 'ready'
}

Then update the status check:

-if (item.status === 'error') {
+if (item.status === UploaderStatus.ERROR) {

49-62: 🛠️ Refactor suggestion

Extract progress rendering logic into a separate component.

The nested conditional rendering makes the code harder to read and maintain. Consider extracting this logic into a dedicated component.

interface ProgressProps {
  status: string
  message: string
  item: FileItem
}

const ProgressOverlay: React.FC<ProgressProps> = ({ status, message, item }) => {
  if (status === 'ready') {
    return (
      <div className="nut-uploader-preview-progress">
        <div className="nut-uploader-preview-progress-msg">{message}</div>
      </div>
    )
  }
  
  if (status !== 'success') {
    return (
      <div className="nut-uploader-preview-progress">
        {renderIcon(item)}
        <div className="nut-uploader-preview-progress-msg">{message}</div>
      </div>
    )
  }
  
  return null
}

Then use it in the main component:

-{status === 'ready' ? (
-  <div className="nut-uploader-preview-progress">
-    <div className="nut-uploader-preview-progress-msg">
-      {message}
-    </div>
-  </div>
-) : (
-  status !== 'success' && (
-    <div className="nut-uploader-preview-progress">
-      {renderIcon(item)}
-      <div className="nut-uploader-preview-progress-msg">
-        {message}
-      </div>
-    </div>
-  )
-)}
+<ProgressOverlay status={status} message={message} item={item} />

109-113: 🛠️ Refactor suggestion

Improve className handling and component styling.

Two suggestions for improvement:

  1. Validate status before using it in className to prevent invalid CSS classes.
  2. Consider moving the spacing after LinkIcon to CSS.
-className={`nut-uploader-preview-img-file-name ${status}`}
+className={`nut-uploader-preview-img-file-name ${status || ''}`}

-<LinkIcon />
-<span>&nbsp;{name}</span>
+<LinkIcon className="nut-uploader-preview-img-file-icon" />
+<span>{name}</span>

Add to your CSS:

.nut-uploader-preview-img-file-icon {
  margin-right: 0.25em;
}
src/packages/uploader/preview.taro.tsx (2)

8-10: 🛠️ Refactor suggestion

Add proper TypeScript types for component props.

The component is using any type which reduces type safety. Consider creating a proper interface for the props:

interface PreviewProps {
  fileList: FileItem[]
  previewType: 'picture' | 'list'
  deletable?: boolean
  onDeleteItem: (item: FileItem, index: number) => void
  handleItemClick: (item: FileItem, index: number) => void
  previewUrl?: string
  deleteIcon?: React.ReactNode
  children?: React.ReactNode
}

export const Preview: React.FunctionComponent<PreviewProps> = ({...})

Also applies to: 13-20


23-23: 🛠️ Refactor suggestion

Consider using an enum for file status.

Using string literals for status values could lead to typos. Consider defining an enum:

export enum FileStatus {
  Ready = 'ready',
  Success = 'success',
  Error = 'error'
}

// Then use it in the FileItem interface and comparisons:
interface FileItem {
  status: FileStatus
  // ...other properties
}

// In the component:
if (item.status === FileStatus.Error)

Also applies to: 34-41

src/packages/uploader/doc.taro.md (1)

109-109: ⚠️ Potential issue

Fix markdown formatting in URLs.

There are some formatting issues in the documentation links:

  1. Line 109: Remove the extra quotation mark in the capture property link
  2. Line 114: Fix the URL formatting in the accept property description

Apply these changes:

-[选取模式](https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/input#htmlattrdefcapture")
+[选取模式](https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/input#htmlattrdefcapture)
-[详细说明]("https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/Input/file#%E9%99%90%E5%88%B6%E5%85%81%E8%AE%B8%E7%9A%84%E6%96%87%E4%BB%B6%E7%B1%BB%E5%9E%8B")
+[详细说明](https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/Input/file#%E9%99%90%E5%88%B6%E5%85%81%E8%AE%B8%E7%9A%84%E6%96%87%E4%BB%B6%E7%B1%BB%E5%9E%8B)

Also applies to: 114-114

🧰 Tools
🪛 LanguageTool

[typographical] ~109-~109: Símbolo desemparellado: Parece que falta «)»
Context: ...tring|picture` | | capture | 图片[选取模式](https://developer.mozilla.org/zh-CN/docs...

(UNPAIRED_BRACKETS)

src/packages/uploader/__tests__/uploader.spec.tsx (1)

209-209: ⚠️ Potential issue

Maintain type safety by using FileItem interface.

Using any type reduces type safety and could mask potential type-related issues. This contradicts TypeScript's benefits.

Replace with proper typing:

-  const list: any = {}
+  const list: FileItem = {
+    name: '文件1.png',
+    url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
+    status: 'ready',
+    message: '准备上传',
+    type: 'image',
+    uid: '12'
+  }
📝 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 list: FileItem = {
    name: '文件1.png',
    url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
    status: 'ready',
    message: '准备上传',
    type: 'image',
    uid: '12'
  }
src/packages/uploader/doc.en-US.md (4)

141-145: ⚠️ Potential issue

Fix Methods table formatting.

The table has formatting issues and redundant column separators.

Apply these corrections:

| MethodName | Description | Parameters | ReturnValues |
-| --- | --- | --- | --- | --- | submit | Manual upload mode
+| --- | --- | --- | --- |
| submit | Manual upload mode, performs the upload operation | `-` | `-` |
📝 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.

The Uploader instance can be retrieved by ref and the instance methods called.
| MethodName | Description | Parameters | ReturnValues |
| --- | --- | --- | --- |
| submit | Manual upload mode, performs the upload operation | `-` | `-` |
| clear | Clear the queue of selected files (this method is usually used when uploading in manual mode) | `index` | `-` |

151-151: ⚠️ Potential issue

Fix broken ConfigProvider component link.

The link to the ConfigProvider component is broken.

Update the link to use the correct fragment:

-The component provides the following CSS variables that can be used to customize styles, see [ConfigProvider component](#/zh-CN/component/configprovider).
+The component provides the following CSS variables that can be used to customize styles, see [ConfigProvider component](/zh-CN/component/configprovider).
📝 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.

The component provides the following CSS variables that can be used to customize styles, see [ConfigProvider component](/zh-CN/component/configprovider).
🧰 Tools
🪛 Markdownlint

151-151: null
Link fragments should be valid

(MD051, link-fragments)


129-137: ⚠️ Potential issue

Clean up FileItem interface table.

There are duplicated entries and formatting issues in the table:

Remove duplicated content:

-| uid | Unique identifier of the file | `-` | name | File name.
-| name | File name | `-` | url | Path to file
-| url | file path | `-` | uid | unique identifier for the file | `-` | name | file name | `-` | url | file path | `-` | type | file type
+| uid | Unique identifier of the file | `-` |
+| name | File name | `-` |
+| url | File path | `-` |
+| type | File type | `image` |
📝 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.

| Name | Description | Default Value |
| --- | --- | --- | status | File status value.
| status | File status value, optionally 'ready,uploading,success,error' | `ready` |
| uid | Unique identifier of the file | `-` |
| name | File name | `-` |
| url | File path | `-` |
| type | File type | `image` |
| loadingIcon | Loading Icon | `-` |
| failIcon | failureIcon | `-` |

94-123: ⚠️ Potential issue

Improve props table grammar and clarity.

Several property descriptions need grammatical improvements:

  1. Replace "If or not" with "Whether" for consistency (autoUpload, clearInput)
  2. Improve clarity of the upload property description
  3. Add missing prepositions and fix punctuation

Apply these corrections:

-| autoUpload | If or not the upload will be done immediately after the file is selected, if false, you need to manually execute the ref submit method to upload | `Boolean` | `true` |
+| autoUpload | Whether to upload immediately after file selection. If false, manual execution of the ref submit method is required | `Boolean` | `true` |

-| upload | Upload method, input is the file object to be uploaded, after asynchronous processing, return the upload result | `(file: File) => Promise<FileItem>` | `-` |
+| upload | Upload method that takes a file object as input and returns a Promise with the upload result after processing | `(file: File) => Promise<FileItem>` | `-` |

-| clearInput | If or not you want to clear the `input` content, set it to `true` to support selecting the same file to upload over and over again | `boolean` | `true` |
+| clearInput | Whether to clear the `input` content. When true, allows repeated selection and upload of the same file | `boolean` | `true` |
📝 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.

| --- | --- | --- | --- | autoUpload | Whether to upload the file immediately after selecting it.
| autoUpload | Whether to upload immediately after file selection. If false, manual execution of the ref submit method is required | `Boolean` | `true` | upload | The upload method, the input parameter is the file to be uploaded.
| upload | Upload method that takes a file object as input and returns a Promise with the upload result after processing | `(file: File) => Promise<FileItem>` | `-` |
| name | The name of the `input` tag `name`, the name of the file parameter sent to the backend | `string` | `file` |
| defaultValue | Default list of files already uploaded | `FileItem[]` | `[]` |
| value | List of files that have been uploaded | `FileItem[]` | `-` |
| preview | Whether or not to show the preview image after a successful upload | `boolean` | `true` |
| previewUrl | Default image address when uploading non-image ('image') formats | `string` | `-` |
| deletable | Whether or not to show the delete button | `boolean` | `true` |
| method | The http method for the upload request | `string` | `post` | | previewType
| previewType | The built-in style of the uploaded list, two basic styles are supported picture, list | `string` | `picture` | picture [capture
| capture | Picture [selection mode] (https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/input#htmlattrdefcapture"), directly bring up the camera | `string` | `false` | maxFileSize
| maxFileSize | You can set the maximum file size (in bytes) for uploading | `number` \| `string` | `Number.MAX_VALUE` |
| maxCount | File upload count limit | `number` \| `string` | `1` |
| fit | Picture fill mode | `contain` \| `cover` \| `fill` \| `none` \| `scale-down` | `cover` |
| clearInput | Whether to clear the `input` content. When true, allows repeated selection and upload of the same file | `boolean` | `true` |
| accept | Allowed file types to be uploaded, [Details] ("https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/Input/file#%E9%99%90%E5%88%B6%E5%85%81%E8%AE% B8%E7%9A%84%E6%96%87%E4%BB%B6%E7%B1%BB%E5%9E%8B") | `string` | `*` |
| uploadIcon | uploadRegion <a href="#/zh-CN/icon">Icon Name</a> | `React.ReactNode` | `-` |
| deleteIcon | Delete the icon name of the region | `React.ReactNode` | `-` |
| uploadLabel | Text below the image in the upload area | `React.
| multiple | Whether to support file multi-selection |`boolean`|`false`|
| disabled | Whether to disable file uploading |`boolean`|`false`|
| beforeUpload | The beforeUpload function needs to return a`Promise`object |`(file: File[]) => Promise<File[] \| boolean>`|`-`|
| beforeDelete | Callback when deleting a file, does not remove it if the return value is false. Supports returning a`Promise`object, which is not removed when resolve(false) or reject |`(file: FileItem, files: FileItem[]) => boolean`|`-`|
| onOversize | Triggered when file size exceeds limit |`(file: File[]) => void`|`-`|
| onOverCount | Triggered when the number of files exceeds the limit |`(count: number) => void`|`-`|
| onChange | Triggered when the list of uploaded files changes |`(files: FileItem[]) => void`|`-`|
| onDelete | Triggered when clicked to delete a file |`(file: FileItem, files: FileItem[]) => void`|`-`|
| onFileItemClick | Triggered when a file is uploaded successfully |`(file: FileItem, index: number) => void`|`-`|
| onUploadQueueChange | Triggered when the image upload queue changes |`(tasks: FileItem[]) => void`|`-` |
🧰 Tools
🪛 LanguageTool

[grammar] ~95-~95: This is a non-standard phrase. Did you mean “whether or not”?
Context: ...tely after selecting it. | autoUpload | If or not the upload will be done immediately aft...

(IF_OR_NOT)


[grammar] ~95-~95: Possibly, a comma is missing after an introductory clause, the verb inflection is not correct, or a question mark is missing. Did you mean “ref, submit” or “ref submits”?
Context: ...false, you need to manually execute the ref submit method to upload | Boolean | true |...

(IF_DT_NN_VBZ)


[uncategorized] ~95-~95: Possible missing preposition found.
Context: ... upload | Boolean | true | upload | The upload method, the input parameter is t...

(AI_HYDRA_LEO_MISSING_IN)


[style] ~100-~100: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...aded | FileItem[] | - | | preview | Whether or not to show the preview image after a succe...

(WHETHER)


[style] ~102-~102: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ormats | string | - | | deletable | Whether or not to show the delete button | boolean |...

(WHETHER)


[grammar] ~109-~109: This is a non-standard phrase. Did you mean “whether or not”?
Context: ...scale-down | cover | | clearInput | If or not you want to clear the input content, ...

(IF_OR_NOT)

src/packages/uploader/demos/h5/demo3.tsx (1)

18-22: ⚠️ Potential issue

Potential memory leak due to unrevoked object URLs.

Using URL.createObjectURL(file) creates object URLs that can consume memory if not released. Consider revoking the object URLs when they are no longer needed to prevent memory leaks.

You can modify the code to revoke the object URL after it's used:

 async function upload(file: File) {
   await sleep(2000)
-  return {
-    url: URL.createObjectURL(file),
-  }
+  const url = URL.createObjectURL(file)
+  // Revoke the object URL when it's no longer needed
+  URL.revokeObjectURL(url)
+  return {
+    url: url,
+  }
 }

Alternatively, ensure that URL.revokeObjectURL(url) is called when the file is removed from the list.

Committable suggestion was skipped due to low confidence.

src/packages/uploader/uploader.tsx (2)

190-200: ⚠️ Potential issue

Fix overwriting of _files after beforeUpload

In the fileChange function, after assigning _files from beforeUpload, you overwrite it with filterFiles(new Array<File>().slice.call(files)), which ignores any modifications made by beforeUpload. This means that the filtered files from beforeUpload are not used.

To ensure that the files returned by beforeUpload are correctly processed, modify the code to pass _files to filterFiles:

let _files: File[] = []
if (beforeUpload) {
  _files = await beforeUpload(Array.from(files))
  if (!_files.length) $el.value = ''
} else {
  _files = Array.from(files)
}
-_files = filterFiles(new Array<File>().slice.call(files))
+_files = filterFiles(_files)
📝 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 fileChange = async (event: React.ChangeEvent<HTMLInputElement>) => {
    if (disabled) return
    const $el = event.target
    const { files } = $el
    let _files: File[] = []
    if (beforeUpload) {
      _files = await beforeUpload(Array.from(files))
      if (!_files.length) $el.value = ''
    } else {
      _files = Array.from(files)
    }
    _files = filterFiles(_files)


201-227: ⚠️ Potential issue

Ensure fileList is updated for non-image files

Currently, fileList is only updated for image files when preview is true. Non-image files are not added to fileList, which means they won't appear in the upload list.

To fix this, update fileList for all files:

const tasks = _files.map((file) => {
  const info: any = {
    uid: idCountRef.current++,
    status: autoUpload ? 'uploading' : 'ready',
    file,
    message: autoUpload
      ? locale.uploader.uploading
      : locale.uploader.waitingUpload,
    name: file.name,
    type: file.type,
  }
  if (preview && file.type?.includes('image')) {
    const reader = new FileReader()
    reader.onload = (event: ProgressEvent<FileReader>) => {
      const fileItem = {
        ...info,
        url: (event.target as FileReader).result as string,
      }
      fileListRef.current = [...fileListRef.current, fileItem]
      setFileList(fileListRef.current)
    }
    reader.readAsDataURL(file)
  } else {
+   const fileItem = {
+     ...info,
+     url: '',
+   }
+   fileListRef.current = [...fileListRef.current, fileItem]
+   setFileList(fileListRef.current)
  }
  return info
})
📝 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 tasks = _files.map((file) => {
      const info: any = {
        uid: idCountRef.current++,
        status: autoUpload ? 'uploading' : 'ready',
        file,
        message: autoUpload
          ? locale.uploader.uploading
          : locale.uploader.waitingUpload,
        name: file.name,
        type: file.type,
      }
      if (preview && file.type?.includes('image')) {
        const reader = new FileReader()
        reader.onload = (event: ProgressEvent<FileReader>) => {
          const fileItem = {
            ...info,
            url: (event.target as FileReader).result as string,
          }
          fileListRef.current = [...fileListRef.current, fileItem]
          setFileList(fileListRef.current)
        }
        reader.readAsDataURL(file)
      } else {
        const fileItem = {
          ...info,
          url: '',
        }
        fileListRef.current = [...fileListRef.current, fileItem]
        setFileList(fileListRef.current)
      }
      return info
    })
src/packages/uploader/uploader.taro.tsx (2)

377-386: ⚠️ Potential issue

Correct the usage of processed files after beforeUpload in onChangeImage

In the onChangeImage function, after calling beforeUpload, the files variable is assigned the result, but the original _files array is passed to readFile instead of the processed files. This may lead to issues if beforeUpload modifies the file list.

Apply this diff to use the processed files array correctly:

 let files = []
 if (beforeUpload) {
   files = await beforeUpload(new Array<File>().slice.call(_files))
-}
-files = filterFiles(new Array<File>().slice.call(_files))
-readFile(_files)
+  files = filterFiles(files)
+  if (!files.length) return
+  readFile(files)
 } else {
+  files = filterFiles(_files)
   readFile(files)
 }
📝 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 onChangeImage = async (res: Taro.chooseImage.SuccessCallbackResult) => {
    // 返回选定照片的本地文件路径列表,tempFilePath可以作为img标签的src属性显示图片
    const { tempFiles } = res
    const _files: Taro.chooseImage.ImageFile[] = filterFiles(tempFiles)
    let files = []
    if (beforeUpload) {
      files = await beforeUpload(new Array<File>().slice.call(_files))
      files = filterFiles(files)
      if (!files.length) return
      readFile(files)
    } else {
      files = filterFiles(_files)
      readFile(files)
    }

245-284: ⚠️ Potential issue

Improve error handling in the uploadAction function

In the uploadAction function, when an upload fails, the error is caught, but then it's re-thrown with throw e. This causes the entire Promise.all to reject, and the error is logged in the .catch block. Re-throwing the error after handling it may not be necessary and could lead to unhandled promise rejections.

Apply this diff to prevent re-throwing the error and handle it gracefully:

         } catch (e) {
           const list = fileListRef.current.map((item) => {
             if (item.uid === currentTask.uid) {
               item.status = 'error'
               item.message = locale.uploader.error
             }
             return item
           })
           setFileList(list)
-          throw e
         }
📝 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 uploadAction = async (tasks: FileItem[]) => {
    const taskIds = tasks.map((task) => task.uid)
    setFileList(
      tasks.map((file: FileItem) => {
        if (taskIds.includes(file.uid)) {
          return {
            ...file,
            status: 'uploading',
            message: locale.uploader.uploading,
          }
        }
        return file
      })
    )
    await Promise.all(
      tasks.map(async (currentTask, index) => {
        try {
          const result = await upload(currentTask.file as File)
          const list = fileListRef.current.map((item) => {
            if (item.uid === currentTask.uid) {
              item.status = 'success'
              item.message = locale.uploader.success
              item.url = result.url
            }
            return item
          })
          setFileList(list)
        } catch (e) {
          const list = fileListRef.current.map((item) => {
            if (item.uid === currentTask.uid) {
              item.status = 'error'
              item.message = locale.uploader.error
            }
            return item
          })
          setFileList(list)
        }
      })
    ).catch((errs) => console.error(errs))

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 14

🧹 Outside diff range and nitpick comments (9)
src/packages/uploader/demos/taro/demo4.tsx (1)

24-33: 建议添加无障碍支持

组件实现基本合理,但建议添加以下改进:

  1. 为上传组件添加 aria-label 属性
  2. 考虑添加上传状态的屏幕阅读器提示

建议修改如下:

  <Cell style={{ flexWrap: 'wrap' }}>
    <Uploader
      value={list}
      onChange={setList}
      upload={(file: File) => upload(file)}
      multiple
      maxFileSize={1024 * 50}
      onOversize={onOversize}
+     aria-label="文件上传"
+     role="button"
    />
  </Cell>
src/packages/uploader/demos/taro/demo3.tsx (1)

14-20: 建议优化sleep函数的实现

sleep函数可以添加参数类型约束和默认值,以提高代码的健壮性。

-  function sleep(time: number) {
+  function sleep(time: number = 1000): Promise<void> {
     return new Promise<void>((resolve) => {
       setTimeout(() => {
         resolve()
       }, time)
     })
   }
src/packages/uploader/demos/taro/demo5.tsx (1)

6-14: 建议移除调试代码

代码中的 console.log 语句应该在生产环境中移除。

建议应用以下修改:

  const beforeUpload = async (files: File[]) => {
    const allowedTypes = ['image/png']
    const filteredFiles = Array.from(files).filter((file) =>
      allowedTypes.includes(file.type)
    )
-   console.log(filteredFiles)
    return filteredFiles
  }
src/packages/uploader/demo.taro.tsx (1)

19-21: 建议优化翻译文案的具体性

翻译文案有以下几点可以改进:

  1. "限制上传数量" 建议添加默认限制数量,使用户更清晰了解限制
  2. "自定义上传前的处理" 可以更具体说明可以处理什么,比如文件大小、类型等

建议修改如下:

-      limitedQuantity: '限制上传数量',
+      limitedQuantity: '限制上传数量(默认最多5个)',
-      beforeUpload: '自定义上传前的处理',
+      beforeUpload: '上传前自定义处理(大小、类型等)',

Also applies to: 30-32, 41-42

src/packages/uploader/demos/taro/demo9.tsx (1)

7-40: 建议改进示例数据的真实性

当前的示例数据有以下可以改进的地方:

  1. 文件名可以更贴近真实场景(如:"产品图片.jpg","用户头像.png")
  2. URL 地址都使用了相同的图片
  3. 建议展示更多样的文件类型(如:PDF、DOC等)
src/packages/uploader/uploader.taro.tsx (4)

82-87: 重构:上传处理机制的重大改进

通过将HTTP上传逻辑从组件中分离出来,新的upload方法提供了更灵活的上传实现方式。这种设计允许开发者:

  • 自定义上传逻辑
  • 更好地处理不同平台的上传需求
  • 实现更复杂的上传场景(如分片上传)

建议在文档中提供一些常见上传实现的示例代码,特别是针对不同平台(H5、小程序等)的实现方案。


166-166: 优化:引入上传队列管理机制

新增的上传队列管理提供了更好的上传控制:

  • 支持手动触发上传
  • 提供队列清理机制
  • 实现了自动上传和手动上传的分离

建议添加以下功能增强:

  • 上传队列状态变化的回调
  • 队列暂停/恢复的控制方法
  • 上传进度的批量监控

Also applies to: 171-172, 313-317


Line range hint 279-311: 注意:文件处理逻辑需要优化

当前实现存在以下问题:

  1. idCountRef 计数器可能在组件重新挂载时重置
  2. 文件类型检测依赖文件扩展名,可能不够准确
  3. 状态更新可能存在竞态条件

建议进行如下修改:

- const idCountRef = useRef(0)
+ const idCountRef = useRef(Date.now())
  const readFile = async <T extends TFileType>(files: T[]) => {
+   const currentFileList = [...fileListRef.current]
    const tasks = files.map((file) => {
      // ... existing code ...
    })
-   fileListRef.current = [...fileListRef.current, info]
-   setFileList(fileListRef.current)
+   const newFileList = [...currentFileList, info]
+   fileListRef.current = newFileList
+   setFileList(newFileList)

Line range hint 392-431: 建议:改进上传组件的可访问性和用户体验

上传按钮和预览区域的实现需要考虑以下几点:

  1. 缺少键盘访问支持
  2. 缺少适当的 ARIA 属性
  3. 上传状态的视觉反馈不够明显

建议进行如下改进:

  <View
    className={`nut-uploader-upload ${previewType} ${
      disabled ? 'nut-uploader-upload-disabled' : ''
    }`}
+   role="button"
+   tabIndex={disabled ? -1 : 0}
+   aria-label={locale.uploader.uploadText}
  >
    <View className="nut-uploader-icon">
      {uploadIcon}
-     <span className="nut-uploader-icon-tip">{uploadLabel}</span>
+     <span className="nut-uploader-icon-tip" aria-hidden="true">
+       {uploadLabel}
+     </span>
    </View>
    <Button 
      className="nut-uploader-input" 
      onClick={_chooseImage}
+     aria-hidden="true"
    />
  </View>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2334e78 and 2983242.

📒 Files selected for processing (9)
  • src/packages/uploader/demo.taro.tsx (2 hunks)
  • src/packages/uploader/demos/taro/demo1.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo10.tsx (0 hunks)
  • src/packages/uploader/demos/taro/demo3.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo4.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo5.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo8.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo9.tsx (1 hunks)
  • src/packages/uploader/uploader.taro.tsx (11 hunks)
💤 Files with no reviewable changes (1)
  • src/packages/uploader/demos/taro/demo10.tsx
🧰 Additional context used
📓 Learnings (1)
src/packages/uploader/demos/taro/demo1.tsx (1)
Learnt from: Alex-huxiyang
PR: jdf2e/nutui-react#2416
File: src/packages/uploader/uploader.taro.tsx:230-230
Timestamp: 2024-07-05T02:58:57.992Z
Learning: When changes are made to the `fileList` state in the `src/packages/uploader/uploader.taro.tsx` file, ensure to generate unit tests to cover these changes.
🔇 Additional comments (8)
src/packages/uploader/demos/taro/demo4.tsx (1)

1-6: 代码结构清晰,状态管理符合最佳实践!

使用 useState 管理文件列表状态是一个很好的选择,这样可以确保组件的响应式更新。

src/packages/uploader/demos/taro/demo3.tsx (1)

1-3: 导入声明看起来简洁明了

导入语句组织得当,只引入必要的依赖项。

src/packages/uploader/demos/taro/demo5.tsx (2)

1-3: 导入声明看起来很清晰!

导入声明简洁明了,依赖项引用恰当。


29-36: 组件渲染实现清晰简洁!

组件结构合理,属性传递正确,代码易于理解和维护。

src/packages/uploader/demos/taro/demo1.tsx (1)

10-14: 需要添加文件列表状态管理的单元测试

根据之前的经验,对fileList状态的更改需要相应的单元测试覆盖。建议添加以下测试场景:

  • 初始状态验证
  • 文件列表更新
  • 边界条件处理
src/packages/uploader/demos/taro/demo8.tsx (1)

1-7: 代码结构清晰,类型定义准确!

接口定义简洁明了,很好地定义了上传组件的引用类型。

src/packages/uploader/demo.taro.tsx (1)

53-72: 布局结构优化合理,代码更易维护

代码改动要点:

  1. 从 Taro 特定组件改为标准 HTML 元素,提高了代码的通用性
  2. Demo 组件排序逻辑清晰,从基础用法到进阶特性
  3. 布局结构简化,提高了可维护性
src/packages/uploader/demos/taro/demo9.tsx (1)

1-4: 导入声明看起来很合理!

组件导入和类型声明都很清晰,使用了必要的图标和组件。

src/packages/uploader/demos/taro/demo1.tsx Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (2)
src/packages/uploader/uploader.taro.tsx (2)

Line range hint 280-317: 建议:优化文件处理逻辑

当前实现在文件类型判断和状态管理上有以下可优化点:

  1. 文件类型判断可以抽取为独立函数
  2. 状态更新可以优化以减少重渲染

建议如下优化:

+ const getFileType = (file: TFileType, filepath: string): string => {
+   if (file.fileType) return file.fileType
+   if (file.type) return file.type
+   const imgReg = /\.(png|jpeg|jpg|webp|gif)$/i
+   return (imgReg.test(filepath) || filepath.includes('data:image')) 
+     ? 'image' 
+     : 'unknown'
+ }

  const readFile = async <T extends TFileType>(files: T[]) => {
    const tasks = files.map((file) => {
      const filepath = (file.tempFilePath || file.path) as string
-     let fileType = file.type
-     if (file.fileType) {
-       fileType = file.fileType
-     } else {
-       const imgReg = /\.(png|jpeg|jpg|webp|gif)$/i
-       if (
-         !fileType &&
-         (imgReg.test(filepath) || filepath.includes('data:image'))
-       ) {
-         fileType = 'image'
-       }
-     }
+     const fileType = getFileType(file, filepath)
      // ... rest of the code
    })
  }

Line range hint 386-422: 建议:优化渲染性能

渲染逻辑拆分得很清晰,但建议进行以下优化:

  1. 使用useMemo缓存渲染结果
  2. 避免不必要的重复计算
+ const memoizedListUploader = useMemo(() => {
    return (
      (children || previewType === 'list') && (
        <View className="nut-uploader-slot">
          {/* ... existing code ... */}
        </View>
      )
    )
+ }, [children, previewType, maxCount, fileList.length, locale])

+ const memoizedImageUploader = useMemo(() => {
    return (
      Number(maxCount) > fileList.length &&
      previewType === 'picture' &&
      !children && (
        <View className={`nut-uploader-upload ${previewType} ${
          disabled ? 'nut-uploader-upload-disabled' : ''
        }`}>
          {/* ... existing code ... */}
        </View>
      )
    )
+ }, [maxCount, fileList.length, previewType, children, disabled, uploadIcon, uploadLabel])

  const renderListUploader = () => memoizedListUploader
  const renderImageUploader = () => memoizedImageUploader
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 2983242 and 563e918.

📒 Files selected for processing (1)
  • src/packages/uploader/uploader.taro.tsx (11 hunks)
🔇 Additional comments (1)
src/packages/uploader/uploader.taro.tsx (1)

82-87: 接口重构提升了组件的灵活性和可扩展性

通过移除硬编码的上传配置(如url、timeout等),改为提供upload方法,使组件更容易适应不同的上传场景和需求。

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Outside diff range and nitpick comments (6)
src/packages/uploader/demos/h5/demo5.tsx (1)

28-35: 建议增加无障碍支持

上传组件缺少适当的 ARIA 属性和标签说明。建议添加无障碍支持以提升用户体验。

 <Cell style={{ flexWrap: 'wrap' }}>
   <Uploader
+    aria-label="上传PNG图片"
+    title="选择文件"
     value={list}
     onChange={setList}
     beforeUpload={beforeUpload}
     upload={(file: File) => upload(file)}
   />
 </Cell>
src/packages/uploader/demos/taro/demo5.tsx (1)

14-20: 建议将工具函数移至共享文件

sleep 函数是一个通用的工具函数,建议将其移动到专门的工具文件中,以便在其他组件中复用。

建议创建一个 utils/async.ts 文件来存放这类异步工具函数。

src/packages/uploader/doc.taro.md (2)

92-92: 建议补充 FileItem 类型说明

upload 方法返回值和 defaultValue/value 属性都使用了 FileItem 类型,建议在属性说明中添加对 FileItem 类型的引用链接,方便用户查阅完整的类型定义。

Also applies to: 94-95


Line range hint 138-143: 建议添加方法使用示例

Methods 部分的说明简洁明了,建议添加一个简单的代码示例来展示如何使用 ref 调用这些方法。

建议添加如下示例:

const uploaderRef = useRef<any>()

// 手动上传
const handleSubmit = () => {
  uploaderRef.current?.submit()
}

// 清空文件队列
const handleClear = () => {
  uploaderRef.current?.clear()
}
🧰 Tools
🪛 LanguageTool

[locale-violation] ~104-~104: 'input' é un xenismo. É preferíbel dicir "entrada"
Context: ...-down|cover | | clearInput | 是否需要清空input内容,设为true支持重复选择上传同一个文件 | boolean` | ...

(GL_BARBARISM_REPLACE)


[grammar] ~118-~118: 'album' é un erro. Considere utilizar "álbum"
Context: ...video/wx.chooseMedia.html") | Array | ['album','camera'] | | camera仅支持WEAPP | 仅在 `...

(GL_WIKIPEDIA_COMMON_ERRORS)


[locale-violation] ~121-~121: 'mix' é un xenismo. É preferíbel dicir "mestura"
Context: ...video/wx.chooseMedia.html") | Array | ['image', 'video', 'mix'] | | maxDuration仅支持WEAPP | 拍摄视频最长拍摄...

(GL_BARBARISM_REPLACE)


[locale-violation] ~128-~128: 'status' é un xenismo. É preferíbel dicir "estatuto"
Context: ...| 名称 | 说明 | 默认值 | | --- | --- | --- | | status | 文件状态值,可选'ready,uploading,success,erro...

(GL_BARBARISM_REPLACE)


[typographical] ~128-~128: Símbolo desemparellado: Parece que falta «'»
Context: ...| 文件状态值,可选'ready,uploading,success,error' | ready | | uid | 文件的唯一标识 | - | | n...

(UNPAIRED_BRACKETS)

src/packages/uploader/uploader.taro.tsx (2)

Line range hint 286-323: 建议优化文件处理逻辑

当前实现可以考虑以下优化:

  1. 文件类型检测可以更完善,建议添加更多文件类型的支持
  2. 文件状态可以更细化,比如添加 validating 状态
  3. 考虑添加文件大小的格式化显示

建议参考如下改进:

  const readFile = async <T extends TFileType>(files: T[]) => {
    const tasks = files.map((file) => {
      let fileType = file.type
      const filepath = (file.tempFilePath || file.path) as string
+     const formatSize = (size: number) => {
+       if (size < 1024) return size + 'B'
+       if (size < 1024 * 1024) return (size / 1024).toFixed(2) + 'KB'
+       return (size / 1024 / 1024).toFixed(2) + 'MB'
+     }
      if (file.fileType) {
        fileType = file.fileType
      } else {
-       const imgReg = /\.(png|jpeg|jpg|webp|gif)$/i
+       const imgReg = /\.(png|jpeg|jpg|webp|gif|bmp|svg)$/i
+       const videoReg = /\.(mp4|webm|ogg|mov)$/i
        if (
          !fileType &&
          (imgReg.test(filepath) || filepath.includes('data:image'))
        ) {
          fileType = 'image'
        }
+       if (!fileType && videoReg.test(filepath)) {
+         fileType = 'video'
+       }
      }
      const info: any = {
        uid: idCountRef.current++,
-       status: autoUpload ? 'uploading' : 'ready',
+       status: autoUpload ? 'validating' : 'ready',
        file,
        message: autoUpload
-         ? locale.uploader.uploading
+         ? locale.uploader.validating
          : locale.uploader.waitingUpload,
        name: getEnv() === 'WEB' ? file.originalFileObj?.name : filepath,
        path: filepath,
        type: getEnv() === 'WEB' ? file.originalFileObj?.type : fileType,
+       size: formatSize(file.size),
      }

Line range hint 392-444: 建议优化UI渲染和可访问性

当前UI实现可以考虑以下改进:

  1. 添加适当的 ARIA 属性以提升可访问性
  2. 使用更语义化的 HTML 结构
  3. 添加更多的类型安全保护

建议参考如下改进:

  const renderListUploader = () => {
    return (
      (children || previewType === 'list') && (
-       <View className="nut-uploader-slot">
+       <View 
+         className="nut-uploader-slot"
+         role="region"
+         aria-label={locale.uploader.uploadFiles}
+       >
          <>
            {children || (
              <Button 
                size="small" 
                type="primary"
+               aria-label={locale.uploader.uploadFiles}
              >
                {locale.uploader.list}
              </Button>
            )}
            {Number(maxCount) > fileList.length && (
              <Button 
                className="nut-uploader-input" 
                onClick={_chooseImage}
+               aria-label={locale.uploader.selectFiles} 
              />
            )}
          </>
        </View>
      )
    )
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 563e918 and c2cb152.

📒 Files selected for processing (4)
  • src/packages/uploader/demos/h5/demo5.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo5.tsx (1 hunks)
  • src/packages/uploader/doc.taro.md (3 hunks)
  • src/packages/uploader/uploader.taro.tsx (13 hunks)
🧰 Additional context used
🪛 LanguageTool
src/packages/uploader/doc.taro.md

[locale-violation] ~93-~93: 'input' é un xenismo. É preferíbel dicir "entrada"
Context: ... => Promise|-| | name |input标签name的名称,发到后台的文件参数名 |string` | ...

(GL_BARBARISM_REPLACE)


[locale-violation] ~104-~104: 'input' é un xenismo. É preferíbel dicir "entrada"
Context: ...-down|cover | | clearInput | 是否需要清空input内容,设为true支持重复选择上传同一个文件 | boolean` | ...

(GL_BARBARISM_REPLACE)


[grammar] ~118-~118: 'album' é un erro. Considere utilizar "álbum"
Context: ...video/wx.chooseMedia.html") | Array | ['album','camera'] | | camera仅支持WEAPP | 仅在 `...

(GL_WIKIPEDIA_COMMON_ERRORS)


[locale-violation] ~121-~121: 'mix' é un xenismo. É preferíbel dicir "mestura"
Context: ...video/wx.chooseMedia.html") | Array | ['image', 'video', 'mix'] | | maxDuration仅支持WEAPP | 拍摄视频最长拍摄...

(GL_BARBARISM_REPLACE)


[locale-violation] ~128-~128: 'status' é un xenismo. É preferíbel dicir "estatuto"
Context: ...| 名称 | 说明 | 默认值 | | --- | --- | --- | | status | 文件状态值,可选'ready,uploading,success,erro...

(GL_BARBARISM_REPLACE)


[typographical] ~128-~128: Símbolo desemparellado: Parece que falta «'»
Context: ...| 文件状态值,可选'ready,uploading,success,error' | ready | | uid | 文件的唯一标识 | - | | n...

(UNPAIRED_BRACKETS)

🔇 Additional comments (8)
src/packages/uploader/demos/h5/demo5.tsx (2)

1-3: 导入声明看起来很清晰!

所有必要的依赖项都已正确导入,没有多余的引用。


6-6: 状态管理实现清晰简洁

使用 useState 管理文件列表,类型定义准确。

src/packages/uploader/demos/taro/demo5.tsx (2)

1-3: 导入声明看起来很清晰!

导入语句简洁明了,只引入了必要的依赖项。


21-25: 此问题在之前的代码审查中已经提出

关于硬编码的延迟时间和图片URL的改进建议仍然有效。

src/packages/uploader/doc.taro.md (2)

21-77: 示例部分结构优化

示例部分的重组更加清晰地展示了组件的各种功能,新增的限制上传数量和大小的示例也很有帮助。


Line range hint 160-169: 主题定制部分完善

CSS 变量定义清晰完整,覆盖了组件的主要样式定制需求。

🧰 Tools
🪛 LanguageTool

[locale-violation] ~157-~157: 'background' é un xenismo. É preferíbel dicir "formación de base"
Context: ...的border圆角 | 4px | | --nutui-uploader-background | 上传图片的背景颜色 | $color-background | | ...

(GL_BARBARISM_REPLACE)


[locale-violation] ~157-~157: 'background' é un xenismo. É preferíbel dicir "formación de base"
Context: ...nutui-uploader-background | 上传图片的背景颜色 | $color-background | | --nutui-uploader-background-disab...

(GL_BARBARISM_REPLACE)


[locale-violation] ~158-~158: 'background' é un xenismo. É preferíbel dicir "formación de base"
Context: ...color-background| | \--nutui-uploader-background-disabled | 上传图片禁用状态的背景颜色 |$color-back...

(GL_BARBARISM_REPLACE)


[locale-violation] ~158-~158: 'background' é un xenismo. É preferíbel dicir "formación de base"
Context: ...r-background-disabled | 上传图片禁用状态的背景颜色 | $color-background | | --nutui-uploader-image-icon-tip-f...

(GL_BARBARISM_REPLACE)


[locale-violation] ~161-~161: 'background' é un xenismo. É preferíbel dicir "formación de base"
Context: ... | | --nutui-uploader-preview-progress-background | 上传区域预览进度的背景颜色 | rgba(0, 0, 0, 0.65)...

(GL_BARBARISM_REPLACE)

src/packages/uploader/uploader.taro.tsx (2)

81-91: 接口设计更加清晰和可控

新的接口设计有以下改进:

  1. 移除了具体的上传配置(如 url、headers 等),改为提供 upload 方法,使组件更加灵活
  2. 添加了 onOverCount 回调,优化了文件数量限制的处理
  3. 新增 onUploadQueueChange 回调,方便跟踪上传队列状态

244-282: 🛠️ Refactor suggestion

建议增强上传过程的错误处理和状态管理

当前实现可以进一步优化:

  1. 添加重试机制,提高上传可靠性
  2. 增加上传进度回调
  3. 完善错误信息展示,包含具体错误信息而不是通用错误提示

建议参考如下改进:

  const uploadAction = async (tasks: FileItem[]) => {
    const taskIds = tasks.map((task) => task.uid)
    const list: FileItem[] = fileListRef.current.map((file: FileItem) => {
      if (taskIds.includes(file.uid)) {
        return {
          ...file,
          status: 'uploading',
          message: locale.uploader.uploading,
        }
      }
      return file
    })
    setFileList(list)
    await Promise.all(
      tasks.map(async (currentTask, index) => {
+       let retryCount = 0
+       const maxRetries = 3
+       while (retryCount < maxRetries) {
          try {
-           const result = await upload(currentTask.file as File)
+           const result = await upload(currentTask.file as File).catch((e) => {
+             throw new Error(`上传失败: ${e.message}`)
+           })
            const list = fileListRef.current.map((item) => {
              if (item.uid === currentTask.uid) {
                item.status = 'success'
                item.message = locale.uploader.success
                item.url = result.url
              }
              return item
            })
            setFileList(list)
+           break
          } catch (e) {
+           retryCount++
+           if (retryCount === maxRetries) {
              const list = fileListRef.current.map((item) => {
                if (item.uid === currentTask.uid) {
                  item.status = 'error'
-                 item.message = locale.uploader.error
+                 item.message = e.message || locale.uploader.error
                }
                return item
              })
              setFileList(list)
+             throw e
            }
+           await new Promise(resolve => setTimeout(resolve, 1000 * retryCount))
          }
+       }
      })
-   ).catch((errs) => console.error(errs))
+   ).catch((errs) => {
+     console.error('上传失败:', errs)
+     props.onError?.(errs)
+   })
  }

Likely invalid or redundant comment.

src/packages/uploader/demos/taro/demo5.tsx Show resolved Hide resolved
@Alex-huxiyang Alex-huxiyang changed the title refactor(uploader): Upoader 3.0 refactor(uploader): Upoader 3.0 重构 & 多端适配 Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@jdf2e jdf2e deleted a comment from coderabbitai bot Oct 25, 2024
@oasis-cloud

This comment was marked as resolved.

src/packages/uploader/file-item.ts Outdated Show resolved Hide resolved
src/packages/uploader/demos/h5/demo1.tsx Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Outside diff range and nitpick comments (12)
src/packages/uploader/demos/h5/demo1.tsx (4)

7-12: 建议优化初始状态的类型安全性

当前的状态初始化使用了硬编码的示例数据。建议添加更完整的类型定义来增强类型安全性。

建议修改为:

-const [list, setList] = useState<FileItem[]>([
+const defaultList: FileItem[] = [
  {
    url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
    uid: 133,
+   status: 'success',
+   type: 'image'
  },
-])
+]
+const [list, setList] = useState<FileItem[]>(defaultList)

14-17: 建议使用主题变量进行样式管理

当前的样式直接使用了硬编码的像素值。建议使用主题变量来保持一致性和可维护性。

建议考虑使用 CSS-in-JS 或主题变量:

const uploaderStyle = {
-  marginInlineEnd: '10px',
-  marginBottom: '10px',
+  marginInlineEnd: 'var(--nutui-margin-2)',
+  marginBottom: 'var(--nutui-margin-2)',
}

31-33: 建议完善错误处理示例

当前的失败处理过于简单,建议展示更实际的错误场景。

建议修改为:

async function uploadFail(file: File): Promise<FileItem> {
  await sleep(2000)
-  throw new Error('Fail to upload')
+  throw new Error('上传失败:网络连接超时,请重试')
}

37-50: 建议增加更多的用户反馈

上传组件的交互体验可以进一步优化,建议添加更多的用户反馈机制。

建议添加以下属性:

<Uploader
  uploadLabel="商品主图"
  value={list}
  onChange={setList}
  upload={(file: File) => upload(file)}
  style={uploaderStyle}
+ maxCount={5}
+ onDelete={(file) => console.log('删除文件', file)}
+ onProgress={(event) => console.log('上传进度', event)}
/>
src/packages/uploader/demos/taro/demo1.tsx (3)

1-4: 建议优化导入语句的组织结构

建议按照以下顺序组织导入语句:

  1. React 相关
  2. 第三方库组件
  3. 图标组件
  4. 本地类型定义
 import React, { useState } from 'react'
-import { Uploader, Cell } from '@nutui/nutui-react-taro'
-import { Dongdong } from '@nutui/icons-react-taro'
-import { FileItem } from '../../file-item'
+import { Cell, Uploader } from '@nutui/nutui-react-taro'
+import { Dongdong } from '@nutui/icons-react-taro'
+
+import { FileItem } from '../../file-item'

10-15: 建议改进初始状态的类型定义

当前的状态初始化可以更严谨:

  1. 建议为初始文件项添加更多必要的属性(如 status, type 等)
  2. 考虑使用常量定义初始状态
-const [list, setList] = useState<FileItem[]>([
-  {
-    url: demoUrl,
-    uid: 133,
-  },
-])
+const INITIAL_FILE: FileItem = {
+  url: demoUrl,
+  uid: 133,
+  status: 'success',
+  type: 'image',
+  name: 'demo-image.gif'
+}
+const [list, setList] = useState<FileItem[]>([INITIAL_FILE])

37-52: 建议优化组件结构和样式处理

当前实现存在以下可以改进的地方:

  1. 样式定义应该抽离到单独的样式文件
  2. 缺少无障碍访问属性
  3. 上传组件的属性可以更好地组织
+// 抽离到 styles.ts
+const styles = {
+  cell: {
+    flexWrap: 'wrap' as const,
+  },
+  uploader: {
+    marginRight: '10px',
+    marginBottom: '10px',
+  },
+}

-<Cell style={{ flexWrap: 'wrap' }}>
+<Cell style={styles.cell}>
   <Uploader
     uploadLabel="商品主图"
+    accept="image/*"
+    aria-label="上传商品主图"
     value={list}
     onChange={setList}
     upload={(file: File) => upload(file)}
-    style={uploaderStyle}
+    style={styles.uploader}
   />
src/packages/uploader/demos/h5/demo2.tsx (1)

7-47: 建议提取模拟数据到单独的常量文件中

当前的模拟数据直接定义在组件中,并且包含重复的URL。建议进行以下优化:

  1. 将模拟数据提取到单独的常量文件中
  2. 将重复的URL定义为常量
  3. 考虑使用工厂函数来创建不同状态的文件项

建议修改如下:

+ // src/packages/uploader/demos/h5/mock.ts
+ const DEMO_IMAGE_URL = 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif'
+ 
+ export const createFileItem = (
+   uid: number,
+   status: string,
+   name?: string,
+   message?: string,
+   icon?: React.ReactNode
+ ): FileItem => ({
+   uid,
+   name,
+   url: DEMO_IMAGE_URL,
+   status,
+   message,
+   ...(status === 'error' && icon && { failIcon: icon }),
+   ...(status === 'uploading' && icon && { loadingIcon: icon }),
+ })
+
+ export const mockFileList: FileItem[] = [
+   createFileItem(111, 'success', '文件文件文件文件1文件文件文件文件1文件文件文件文件1.png', '上传成功'),
+   // ... 其他文件项
+ ]
src/packages/uploader/demos/taro/demo2.tsx (2)

7-47: 建议改进示例数据的真实性

虽然示例数据展示了不同的上传状态,但有以下几点可以改进:

  1. 所有图片URL都相同,建议使用不同的示例图片
  2. 部分文件项缺少name属性,这可能无法很好地反映实际使用场景
  3. 建议添加不同类型的文件(如PDF、DOC等)来展示多样性

建议参考以下示例结构:

 {
   uid: 333,
+  name: '报告.pdf',
-  url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
+  url: 'https://example.com/path/to/different/file.pdf',
   status: 'uploading',
 }

48-49: 建议增强删除事件的演示效果

当前的删除事件处理仅打印日志,建议添加更多交互效果来展示组件的实际使用场景。

建议改进为:

 const onDelete = (file: FileItem, fileList: FileItem[]) => {
-  console.log('delete事件触发', file, fileList)
+  console.log('文件删除事件触发:', {
+    删除的文件: {
+      名称: file.name,
+      状态: file.status,
+      URL: file.url
+    },
+    剩余文件数量: fileList.length
+  })
 }
src/packages/uploader/preview.taro.tsx (2)

23-23: 状态处理逻辑的改进

状态处理的改变使代码更加清晰,但建议进一步改进:

  1. 建议将状态值定义为常量枚举
  2. 可以将状态相关的渲染逻辑抽取为独立函数

建议添加以下类型定义:

export enum UploadStatus {
  READY = 'ready',
  SUCCESS = 'success',
  ERROR = 'error'
}

Also applies to: 55-71


116-124: 列表视图样式优化建议

为文件名添加状态类名是个好的改进,但有以下建议:

  1. 考虑添加 CSS 类型定义
  2. 建议在组件文档中说明各状态对应的样式效果

需要我帮助更新组件文档吗?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9d813b1 and d902a65.

📒 Files selected for processing (6)
  • src/packages/uploader/demos/h5/demo1.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo2.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo1.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo2.tsx (1 hunks)
  • src/packages/uploader/preview.taro.tsx (4 hunks)
  • src/packages/uploader/preview.tsx (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/packages/uploader/preview.tsx
🧰 Additional context used
📓 Learnings (1)
src/packages/uploader/demos/taro/demo1.tsx (1)
Learnt from: Alex-huxiyang
PR: jdf2e/nutui-react#2416
File: src/packages/uploader/uploader.taro.tsx:230-230
Timestamp: 2024-07-05T02:58:57.992Z
Learning: When changes are made to the `fileList` state in the `src/packages/uploader/uploader.taro.tsx` file, ensure to generate unit tests to cover these changes.
🔇 Additional comments (6)
src/packages/uploader/demos/h5/demo1.tsx (1)

1-4: 🛠️ Refactor suggestion

需要从 @nutui/nutui-react 包中导入类型定义

根据之前的评论,FileItem 类型应该从 @nutui/nutui-react 包中导入,而不是从相对路径导入。这样可以确保类型定义的一致性和可维护性。

建议修改为:

import React, { useState } from 'react'
-import { Uploader, Cell } from '@nutui/nutui-react'
+import { Uploader, Cell, FileItem } from '@nutui/nutui-react'
import { Dongdong } from '@nutui/icons-react'
-import { FileItem } from '../../file-item'
src/packages/uploader/demos/h5/demo2.tsx (1)

1-6: 导入声明和组件结构清晰明确!

组件命名与文件名保持一致,导入声明组织合理。

src/packages/uploader/demos/taro/demo2.tsx (1)

2-4: 导入语句结构清晰且合理!

所有必要的组件和图标都已正确导入,符合组件的使用需求。

src/packages/uploader/preview.taro.tsx (3)

34-41: 优化的属性解构模式

使用解构赋值并提供默认值是一个很好的改进,它能:

  1. 提高代码可读性
  2. 确保属性缺失时的安全性
  3. 减少重复的属性访问

77-84: 注意图片展示模式的变更

将图片模式从 aspectFit 更改为 aspectFill 可能会影响现有的图片展示效果:

  1. aspectFill 会裁剪图片以填充容器
  2. 这可能导致图片部分内容被裁切

另外,成功状态下显示文件名的新增功能很好地提升了用户体验。

建议验证这个改动对现有上传组件的影响:

#!/bin/bash
# 检查其他使用 Image 组件的地方
rg "mode=['\"]aspectFit['\"]" 

Also applies to: 107-109


8-10: 导入路径的改进

@tarojs/components 切换到自定义的 @/packages/image/image.taro 组件可能会提供更好的跨平台兼容性和自定义功能。

验证新的图片组件实现:

✅ Verification successful

Let me gather more information about the Image component implementation.


从 @tarojs/components 切换到自定义 Image 组件是合理的改进

从代码实现来看,自定义的 @/packages/image/image.taro 组件是对原生 Taro Image 组件的增强封装,提供了以下优势:

  • 统一的错误处理和加载状态展示
  • 更好的跨平台样式处理(RN、Web、小程序)
  • 简化的尺寸和圆角配置
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 检查新的图片组件实现
ast-grep --pattern 'export const Image = $_'

Length of output: 176


Script:

#!/bin/bash
# Search for Image component implementation in image.taro.tsx
rg -A 10 "export (const|function) Image" --type tsx

# Check if there are any other files using the Image component from @tarojs/components
rg "import.*Image.*from ['|\"]@tarojs/components['|\"]" --type tsx

Length of output: 182


Script:

#!/bin/bash
# Search for Image component implementation
rg -t ts -t tsx -A 10 "export (const|function) Image"

# Check if there are any other files using the Image component from @tarojs/components
rg -t ts -t tsx "import.*Image.*from ['|\"]@tarojs/components['|\"]"

# Look for the image.taro.tsx file
fd "image.taro.tsx$" --type f

Length of output: 252


Script:

#!/bin/bash
# Check the content of image.taro.tsx
cat src/packages/image/image.taro.tsx

# Search for other files that might be using this Image component
rg "@/packages/image/image.taro"

Length of output: 4006

src/packages/uploader/demos/h5/demo1.tsx Show resolved Hide resolved
src/packages/uploader/demos/taro/demo1.tsx Show resolved Hide resolved
src/packages/uploader/demos/h5/demo2.tsx Show resolved Hide resolved
src/packages/uploader/demos/taro/demo2.tsx Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 18

🧹 Outside diff range and nitpick comments (30)
src/packages/uploader/index.ts (3)

6-26: 建议添加类型注释并优化类型定义

建议对 FileItem 类型做以下改进:

  1. 添加 JSDoc 注释说明每个属性的用途
  2. 考虑将部分关键属性设为必填,如 uidstatus
  3. type 属性的字符串联合类型可以更严格,建议列举所有可能的值

示例改进:

+ /**
+  * 文件项的类型定义
+  */
 type FileItem = {
+  /** 文件状态 */
-  status?: FileItemStatus
+  status: FileItemStatus
+  /** 状态信息 */
   message?: string
+  /** 唯一标识符 */
-  uid?: string | number
+  uid: string | number
+  /** 文件名称 */
   name?: string
+  /** 文件URL */
   url?: string
+  /** 文件类型 */
-  type?: 'image' | 'list' | string
+  type?: 'image' | 'video' | 'audio' | 'document' | 'list'
   // ... 其他属性保持不变
 }

28-28: 建议为状态类型添加注释

为了提高代码可维护性,建议添加类型注释说明每个状态的含义。

+ /**
+  * 文件项状态
+  * @description
+  * - ready: 准备上传
+  * - uploading: 上传中
+  * - success: 上传成功
+  * - error: 上传失败
+  */
 type FileItemStatus = 'ready' | 'uploading' | 'success' | 'error'

1-1: 建议添加模块级文档

作为 Uploader 3.0 重构的一部分,建议在文件顶部添加完整的模块文档,包括:

  1. 模块的整体功能说明
  2. 导出内容的概述
  3. 版本变更说明
+ /**
+  * @file Uploader 组件类型定义
+  * @description Uploader 3.0 版本的核心类型定义,包含文件项、状态等类型
+  * @version 3.0.0
+  */
 import { ReactNode } from 'react'

Also applies to: 32-32

src/packages/uploader/demos/h5/demo4.tsx (1)

6-21: 建议优化工具函数和错误处理

代码功能实现正确,但有以下优化建议:

  1. sleep 工具函数建议移至公共工具文件
  2. upload 函数需要添加错误处理
  3. 建议将文件大小限制提取为常量

建议按如下方式重构:

+const MAX_FILE_SIZE = 1024 * 50
+const FILE_SIZE_ERROR_MESSAGE = '文件大小不能超过 50kb'

 const onOversize = (files: File[]) => {
-  console.log('oversize 触发 文件大小不能超过 50kb', files)
+  console.log(`oversize 触发 ${FILE_SIZE_ERROR_MESSAGE}`, files)
 }

 async function upload(file: File) {
+  try {
     await sleep(2000)
     return {
       url: URL.createObjectURL(file),
     }
+  } catch (error) {
+    console.error('文件上传失败:', error)
+    throw error
+  }
 }
src/packages/uploader/demos/taro/demo7.tsx (3)

5-11: 建议优化文件名称

当前文件名 文件文件文件文件1文件文件文件文件1文件文件文件文件1.png 存在重复文本,建议简化为更清晰的示例名称。

建议修改如下:

-    name: '文件文件文件文件1文件文件文件文件1文件文件文件文件1.png',
+    name: '示例文件1.png',

15-23: 建议区分两个上传组件的默认值

两个上传组件共用相同的 defaultFileList,这可能会导致用户困惑。建议为每个上传组件设置不同的默认值,以更好地展示其独特性。

建议修改如下:

+const defaultFileList1 = [{
+  name: '示例文件1.png',
+  url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
+  message: '上传成功',
+}]
+
+const defaultFileList2 = [{
+  name: '示例文件2.png',
+  url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
+  message: '上传成功',
+}]

 <Uploader
-  defaultValue={defaultFileList}
+  defaultValue={defaultFileList1}
   ...
 />
 <Uploader
-  defaultValue={defaultFileList}
+  defaultValue={defaultFileList2}
   ...
 />

Also applies to: 24-29


14-30: 建议优化组件结构

当前的布局结构可以通过添加适当的标题或描述来提高可读性和可用性。

建议修改如下:

 <Cell style={{ flexWrap: 'wrap' }}>
+  <Cell.Group title="自定义上传图标展示">
     <Uploader
       defaultValue={defaultFileList}
       uploadIcon={<Dongdong />}
       deleteIcon={<Star />}
       style={{
         marginInlineEnd: '10px',
       }}
       maxCount={1}
     />
     <Uploader
       defaultValue={defaultFileList}
       uploadIcon={<Star />}
       deleteIcon={<Dongdong />}
       maxCount={1}
     />
+  </Cell.Group>
 </Cell>
src/packages/uploader/demos/h5/demo3.tsx (2)

6-9: 建议改进样式对象的命名和类型定义

建议进行以下优化:

  1. 为样式对象添加类型定义
  2. 考虑将样式对象提升到组件外部以提高性能
+type UploaderStyleType = {
+  marginInlineEnd: string
+  marginBottom: string
+}
+
+const uploaderStyle: UploaderStyleType = {
   marginInlineEnd: '10px',
   marginBottom: '10px',
 }
+
 const Demo3 = () => {

10-16: sleep 函数实现可以更简洁

当前的 Promise 实现可以简化。

-function sleep(time: number) {
-  return new Promise<void>((resolve) => {
-    setTimeout(() => {
-      resolve()
-    }, time)
-  })
-}
+const sleep = (time: number) => new Promise<void>((resolve) => setTimeout(resolve, time))
src/packages/uploader/demos/h5/demo5.tsx (1)

6-12: 建议优化文件类型过滤的实现

当前实现可以考虑以下改进:

  1. 将允许的文件类型作为配置项
  2. 添加文件被过滤时的用户提示

建议参考以下实现:

-  const beforeUpload = async (files: File[]) => {
-    const allowedTypes = ['image/png']
-    const filteredFiles = Array.from(files).filter((file) =>
-      allowedTypes.includes(file.type)
-    )
-    return filteredFiles
-  }
+  const allowedTypes = ['image/png']
+  const beforeUpload = async (files: File[]) => {
+    const filteredFiles = Array.from(files).filter((file) => {
+      const isAllowed = allowedTypes.includes(file.type)
+      if (!isAllowed) {
+        console.warn(`文件类型不支持: ${file.name}`)
+      }
+      return isAllowed
+    })
+    return filteredFiles
+  }
src/packages/uploader/demos/taro/demo3.tsx (1)

9-24: 建议优化上传模拟函数的实现

当前的实现有以下几点可以改进:

  1. upload 函数缺少错误处理
  2. 固定的演示URL可能导致测试场景单一
  3. sleep 函数可以考虑提取到公共工具中复用

建议按照以下方式优化:

 async function upload(file: File) {
+  try {
   await sleep(2000)
   return {
     url: demoUrl,
+    status: 'success',
+    message: '上传成功'
   }
+  } catch (error) {
+    return {
+      status: 'error',
+      message: '上传失败'
+    }
+  }
 }
src/packages/uploader/demos/taro/demo4.tsx (3)

1-2: 建议优化类型导入

建议将 FileItem 类型从其他类型中分离出来单独导入,以提高代码的可维护性:

-import { Uploader, Cell, FileItem } from '@nutui/nutui-react-taro'
+import type { FileItem } from '@nutui/nutui-react-taro/dist/types'
+import { Uploader, Cell } from '@nutui/nutui-react-taro'

8-8: 建议使用类型推导优化状态声明

当前的状态声明可以利用 TypeScript 的类型推导功能来简化:

-const [list, setList] = useState<FileItem[]>([])
+const [list, setList] = useState(() => [] as FileItem[])

这样可以获得更好的性能,因为初始状态使用函数形式可以避免在每次渲染时都创建新数组。


28-36: 建议增加无障碍访问支持

为了提高组件的可访问性,建议添加相关的 ARIA 属性:

 <Cell style={{ flexWrap: 'wrap' }}>
   <Uploader
     value={list}
     onChange={setList}
     upload={(file: File) => upload(file)}
     maxFileSize={1024 * 50}
     onOversize={onOversize}
+    aria-label="文件上传"
+    role="button"
+    data-testid="demo4-uploader"
   />
 </Cell>
src/packages/uploader/demos/taro/demo5.tsx (2)

13-24: 建议改进上传模拟实现的灵活性

当前实现存在以下可以改进的地方:

  1. 延迟时间被硬编码为2秒
  2. 返回URL被硬编码在函数中
  3. 缺少错误处理机制

建议按照以下方式重构:

+ const DEMO_IMAGE_URL = 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif'
+ const UPLOAD_DELAY = 2000
+
  function sleep(time: number) {
    return new Promise<void>((resolve) => {
      setTimeout(() => {
        resolve()
      }, time)
    })
  }
  async function upload(file: File) {
+   try {
-     await sleep(2000)
+     await sleep(UPLOAD_DELAY)
      return {
-       url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
+       url: DEMO_IMAGE_URL,
      }
+   } catch (error) {
+     console.error('上传失败:', error)
+     throw error
+   }
  }

27-34: 建议增加无障碍访问支持

上传组件应该添加适当的 ARIA 属性以提升可访问性。

建议修改如下:

  <Cell style={{ flexWrap: 'wrap' }}>
    <Uploader
      value={list}
      onChange={setList}
      beforeUpload={beforeUpload}
      upload={(file: File) => upload(file)}
+     aria-label="图片上传"
+     role="button"
    />
  </Cell>
src/packages/uploader/demos/h5/demo1.tsx (1)

6-11: 建议优化初始状态的设置

当前的初始状态使用了硬编码的URL和uid,建议:

  1. 使用更有意义的示例图片
  2. 添加更多的文件属性来展示组件的完整功能
const [list, setList] = useState<FileItem[]>([
  {
-   url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
-   uid: 133,
+   url: '示例图片URL',
+   uid: 1,
+   status: 'success',
+   type: 'image',
+   name: '示例图片.jpg'
  },
])
src/packages/uploader/demos/taro/demo1.tsx (2)

9-14: 建议完善初始状态的 FileItem 结构

初始状态的 FileItem 对象缺少完整的属性定义,建议与上传函数返回的对象保持一致。

 const [list, setList] = useState<FileItem[]>([
   {
     url: demoUrl,
     uid: 133,
+    status: 'success',
+    type: 'image/gif',
+    name: 'demo-image.gif',
   },
 ])

45-50: 建议优化组件结构

当前实现中有多个 Uploader 组件实例,建议:

  1. 将样式抽取到独立的样式文件
  2. 考虑将不同场景的 Uploader 封装为独立的演示组件
  3. 添加适当的注释说明每个实例的用途
+// 成功上传示例
 <Uploader style={uploaderStyle} upload={(file: File) => upload(file)} />
+// 上传失败示例
 <Uploader
   uploadIcon={<Dongdong />}
   upload={(file: File) => uploadFail(file)}
   style={uploaderStyle}
 />
src/packages/uploader/demos/h5/demo2.tsx (2)

5-5: 建议添加组件重命名的注释说明

组件从 Demo1 重命名为 Demo2,建议添加注释说明重命名的原因,以便其他开发者理解。

+// 重命名自 Demo1,用于展示带有默认值的上传组件示例
const Demo2 = () => {

51-56: 建议添加属性类型说明

为了提高代码可维护性,建议为 Uploader 组件的属性添加类型说明注释。

    <Uploader
+      // 上传文件的默认列表
      defaultValue={defaultList}
+      // 删除文件时的回调函数
      onDelete={onDelete}
+      // 自定义上传图标
      uploadIcon={<Dongdong />}
+      // 最大上传文件数量
      maxCount={6}
    />
src/packages/uploader/demos/h5/demo10.tsx (1)

6-39: 建议改进初始状态数据的处理方式

  1. 建议将硬编码的图片URL替换为配置常量或环境变量
  2. 文件名过长可能会导致UI显示问题,建议添加文件名长度限制

建议按照以下方式重构:

+ const MAX_FILENAME_LENGTH = 20;
+ const DEFAULT_PREVIEW_IMAGE = process.env.DEFAULT_PREVIEW_IMAGE || 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif';

  const [list, setList] = useState<FileItem[]>([
    {
-     name: '文件文件文件文件1文件文件文件文件1文件文件文件文件1.png',
+     name: '示例文件1.png',
-     url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
+     url: DEFAULT_PREVIEW_IMAGE,
      status: 'success',
      message: '上传成功',
    },
    // ... 其他项目类似更新
  ])
src/packages/uploader/demos/taro/demo9.tsx (1)

6-39: 建议改进初始状态的模拟数据

当前的模拟数据存在以下可以改进的地方:

  1. 所有文件使用了相同的图片URL
  2. 文件名称可以更贴近实际场景
  3. 状态消息建议使用国际化配置

建议按照以下方式优化:

 const [list, setList] = useState<FileItem[]>([
   {
-    name: '文件文件文件文件1文件文件文件文件1文件文件文件文件1.png',
+    name: 'product-image-01.png',
     url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
     status: 'success',
-    message: '上传成功',
+    message: t('uploadSuccess'),
   },
   // ... 其他项目类似更新
 ])
src/packages/uploader/uploader.tsx (2)

Line range hint 281-334: 渲染逻辑建议优化

渲染逻辑结构清晰,但建议进行以下优化:

  1. 使用 useMemo 优化渲染性能
  2. 提取公共的 input 组件减少代码重复

建议重构如下:

+const UploadInput = React.memo(({ 
+  maxCount, 
+  fileList, 
+  ...props 
+}) => {
+  if (Number(maxCount) <= fileList.length) return null
+  return (
+    <input
+      className="nut-uploader-input"
+      type="file"
+      {...props}
+    />
+  )
+})

-const renderImageUploader = () => {
+const renderImageUploader = React.useMemo(() => {
   // ... existing code
-}
+}, [maxCount, fileList, disabled, uploadIcon, uploadLabel])
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 194-194: src/packages/uploader/uploader.tsx#L194
Added line #L194 was not covered by tests


[warning] 196-222: src/packages/uploader/uploader.tsx#L196-L222
Added lines #L196 - L222 were not covered by tests


[warning] 224-229: src/packages/uploader/uploader.tsx#L224-L229
Added lines #L224 - L229 were not covered by tests


[warning] 231-231: src/packages/uploader/uploader.tsx#L231
Added line #L231 was not covered by tests


[warning] 234-234: src/packages/uploader/uploader.tsx#L234
Added line #L234 was not covered by tests


[warning] 237-277: src/packages/uploader/uploader.tsx#L237-L277
Added lines #L237 - L277 were not covered by tests


[warning] 281-288: src/packages/uploader/uploader.tsx#L281-L288
Added lines #L281 - L288 were not covered by tests


[warning] 291-294: src/packages/uploader/uploader.tsx#L291-L294
Added lines #L291 - L294 were not covered by tests


[warning] 296-303: src/packages/uploader/uploader.tsx#L296-L303
Added lines #L296 - L303 were not covered by tests


[warning] 305-312: src/packages/uploader/uploader.tsx#L305-L312
Added lines #L305 - L312 were not covered by tests


Line range hint 175-336: 测试覆盖率严重不足

根据静态分析工具显示,大量新增代码缺少测试覆盖。这可能会导致潜在bug无法及时发现。建议优先补充以下场景的测试用例:

  1. 文件上传流程
  2. 错误处理
  3. 队列管理
  4. 渲染逻辑

需要我帮您生成一个完整的测试用例模板吗?这将包括:

  • 基础功能测试
  • 错误场景测试
  • 边界条件测试
  • 性能测试
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 194-194: src/packages/uploader/uploader.tsx#L194
Added line #L194 was not covered by tests


[warning] 196-222: src/packages/uploader/uploader.tsx#L196-L222
Added lines #L196 - L222 were not covered by tests


[warning] 224-229: src/packages/uploader/uploader.tsx#L224-L229
Added lines #L224 - L229 were not covered by tests


[warning] 231-231: src/packages/uploader/uploader.tsx#L231
Added line #L231 was not covered by tests


[warning] 234-234: src/packages/uploader/uploader.tsx#L234
Added line #L234 was not covered by tests


[warning] 237-277: src/packages/uploader/uploader.tsx#L237-L277
Added lines #L237 - L277 were not covered by tests


[warning] 281-288: src/packages/uploader/uploader.tsx#L281-L288
Added lines #L281 - L288 were not covered by tests


[warning] 291-294: src/packages/uploader/uploader.tsx#L291-L294
Added lines #L291 - L294 were not covered by tests


[warning] 296-303: src/packages/uploader/uploader.tsx#L296-L303
Added lines #L296 - L303 were not covered by tests


[warning] 305-312: src/packages/uploader/uploader.tsx#L305-L312
Added lines #L305 - L312 were not covered by tests

src/packages/uploader/uploader.taro.tsx (2)

83-85: 建议优化 upload 属性的类型定义

当前的类型定义中,upload 方法接受的参数类型过于宽泛(使用了 any)。建议根据不同环境定义更精确的联合类型,以提供更好的类型安全性。

- upload: (
-   files: Taro.chooseImage.ImageFile | Taro.chooseMedia.ChooseMedia | any
- ) => Promise<FileItem>
+ upload: (
+   files: Taro.chooseImage.ImageFile | Taro.chooseMedia.ChooseMedia | File
+ ) => Promise<FileItem>

Line range hint 390-426: 建议优化渲染逻辑的组织结构

当前渲染逻辑存在以下可优化点:

  1. 渲染逻辑分散在多个函数中
  2. 条件判断逻辑复杂
  3. 样式类名拼接方式不够优雅

建议:

  1. 使用组件拆分替代渲染函数
  2. 使用策略模式处理不同的预览类型
  3. 使用 classnames 统一处理类名
- const renderListUploader = () => {
-   return (...)
- }
- const renderImageUploader = () => {
-   return (...)
- }
+ const ListUploader: React.FC = () => {
+   return (...)
+ }
+ const ImageUploader: React.FC = () => {
+   return (...)
+ }
src/packages/uploader/demos/taro/demo2.tsx (3)

9-9: 文件名过长可能影响界面展示

在第9行,uid为111的文件名过长,可能会导致在界面上显示不完整,影响用户体验。建议适当缩短文件名或在界面上处理超长文件名的显示,确保界面美观。


22-25: 为上传中的文件(uid: 333)添加提示信息和加载动画

当前uid为333的FileItem缺少messageloadingIcon属性,用户可能无法了解上传进度。建议添加提示信息和加载动画以提升用户体验。

应用以下修改:

{
  uid: 333,
  url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
  status: 'uploading',
+ message: '上传中...',
+ loadingIcon: <Loading className="nut-icon-Loading" color="#fff" />,
},

34-38: 为上传失败的文件(uid: 555)添加失败图标

uid为555的FileItem在上传失败时没有提供failIcon,这可能导致用户无法直观地识别失败状态。建议与uid为444的文件保持一致,添加failIcon以统一用户体验。

应用以下修改:

{
  uid: 555,
  url: 'https://m.360buyimg.com/babel/jfs/t1/164410/22/25162/93384/616eac6cE6c711350/0cac53c1b82e1b05.gif',
  status: 'error',
  message: '上传失败',
+ failIcon: <Star style={{ color: 'white' }} />,
},
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between d902a65 and 22c6899.

📒 Files selected for processing (21)
  • src/packages/uploader/demos/h5/demo1.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo10.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo2.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo3.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo4.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo5.tsx (1 hunks)
  • src/packages/uploader/demos/h5/demo7.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo1.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo2.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo3.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo4.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo5.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo7.tsx (1 hunks)
  • src/packages/uploader/demos/taro/demo9.tsx (1 hunks)
  • src/packages/uploader/file-item.ts (0 hunks)
  • src/packages/uploader/index.taro.ts (1 hunks)
  • src/packages/uploader/index.ts (1 hunks)
  • src/packages/uploader/preview.taro.tsx (4 hunks)
  • src/packages/uploader/preview.tsx (5 hunks)
  • src/packages/uploader/uploader.taro.tsx (11 hunks)
  • src/packages/uploader/uploader.tsx (10 hunks)
💤 Files with no reviewable changes (1)
  • src/packages/uploader/file-item.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/packages/uploader/demos/h5/demo7.tsx
  • src/packages/uploader/index.taro.ts
  • src/packages/uploader/preview.taro.tsx
  • src/packages/uploader/preview.tsx
🧰 Additional context used
📓 Learnings (1)
src/packages/uploader/demos/taro/demo1.tsx (1)
Learnt from: Alex-huxiyang
PR: jdf2e/nutui-react#2416
File: src/packages/uploader/uploader.taro.tsx:230-230
Timestamp: 2024-07-05T02:58:57.992Z
Learning: When changes are made to the `fileList` state in the `src/packages/uploader/uploader.taro.tsx` file, ensure to generate unit tests to cover these changes.
🪛 GitHub Check: codecov/patch
src/packages/uploader/uploader.tsx

[warning] 175-175: src/packages/uploader/uploader.tsx#L175
Added line #L175 was not covered by tests


[warning] 194-194: src/packages/uploader/uploader.tsx#L194
Added line #L194 was not covered by tests


[warning] 196-222: src/packages/uploader/uploader.tsx#L196-L222
Added lines #L196 - L222 were not covered by tests


[warning] 224-229: src/packages/uploader/uploader.tsx#L224-L229
Added lines #L224 - L229 were not covered by tests


[warning] 231-231: src/packages/uploader/uploader.tsx#L231
Added line #L231 was not covered by tests


[warning] 234-234: src/packages/uploader/uploader.tsx#L234
Added line #L234 was not covered by tests


[warning] 237-277: src/packages/uploader/uploader.tsx#L237-L277
Added lines #L237 - L277 were not covered by tests


[warning] 281-288: src/packages/uploader/uploader.tsx#L281-L288
Added lines #L281 - L288 were not covered by tests


[warning] 291-294: src/packages/uploader/uploader.tsx#L291-L294
Added lines #L291 - L294 were not covered by tests


[warning] 296-303: src/packages/uploader/uploader.tsx#L296-L303
Added lines #L296 - L303 were not covered by tests


[warning] 305-312: src/packages/uploader/uploader.tsx#L305-L312
Added lines #L305 - L312 were not covered by tests


[warning] 332-336: src/packages/uploader/uploader.tsx#L332-L336
Added lines #L332 - L336 were not covered by tests

🔇 Additional comments (19)
src/packages/uploader/index.ts (1)

30-30: 建议明确 FileType 类型的使用场景

FileType 是一个通用的键值映射类型,但其用途和使用场景不够明确。建议:

  1. 添加详细的类型注释说明使用场景
  2. 考虑使用更具描述性的类型名称
  3. 如果可能,提供使用示例

让我们检查一下这个类型在代码库中的使用情况:

src/packages/uploader/demos/h5/demo4.tsx (1)

1-5: 代码结构清晰,状态管理合理!

使用 useState 管理文件列表状态,符合 React Hooks 的最佳实践。导入声明简洁明确。

src/packages/uploader/demos/taro/demo7.tsx (2)

1-3: 导入声明看起来不错!

导入语句组织得当,包含了所有必要的组件和图标。


14-14: 建议将样式迁移到样式文件中

当前组件使用内联样式定义布局,这降低了代码的可维护性。建议将样式迁移到独立的样式文件中。

Also applies to: 19-21

src/packages/uploader/demos/h5/demo3.tsx (1)

1-5: 导入声明和状态管理的实现看起来不错!

使用 useState 管理文件列表状态是一个很好的做法,这样可以更好地控制上传组件的行为。

src/packages/uploader/demos/h5/demo5.tsx (1)

1-5: 代码结构清晰,类型定义完整!

使用 TypeScript 类型定义确保了类型安全,FileItem[] 的状态管理实现规范。

src/packages/uploader/demos/taro/demo3.tsx (2)

1-8: 导入声明和状态管理的实现看起来不错!

代码结构清晰,使用了 React 的 useState 进行状态管理,符合最佳实践。


1-38: 验证组件在不同场景下的表现

需要验证以下场景:

  1. 文件类型限制
  2. 并发上传限制
  3. 大文件上传
  4. 网络异常处理
✅ Verification successful

已验证上传组件的各项功能实现

通过分析代码库,发现上传组件已经完整实现了所有关注的场景:

  1. 文件类型限制:

    • Demo5 中通过 beforeUpload 实现了文件类型过滤
    • 组件支持 accept 属性限制文件类型
  2. 并发上传限制:

    • 组件支持 multiple 属性控制是否允许多文件上传
    • maxCount 属性限制最大上传数量
    • autoUpload 属性控制是否自动上传,可以手动控制上传时机
  3. 大文件上传:

    • maxFileSize 属性限制文件大小
    • onOversize 回调处理超限文件
  4. 网络异常处理:

    • 上传失败时自动设置 error 状态
    • 支持自定义失败图标和提示信息
    • Demo1/Demo10 展示了上传失败的处理方式

所有这些功能都有完整的示例代码和实现,无需额外验证。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 搜索其他demo文件中的测试用例实现
echo "查找其他demo文件中的测试场景实现..."
rg -t tsx "onError|maxFileSize|beforeUpload" "src/packages/uploader/demos"

Length of output: 310


Script:

#!/bin/bash
# 搜索上传组件的所有demo文件
echo "查找所有上传组件demo文件..."
fd -e tsx -e ts -e jsx -e js . src/packages/uploader/demos --exec cat {}

Length of output: 21619


Script:

#!/bin/bash
# 搜索上传组件的源代码实现
echo "查找上传组件源代码实现..."
fd -e tsx -e ts -e jsx -e js uploader.tsx src/packages/uploader --exec cat {}

Length of output: 9751


Script:

#!/bin/bash
# 搜索测试文件
echo "查找测试文件..."
fd -e test.tsx -e test.ts -e test.jsx -e test.js . src/packages/uploader --exec cat {}

Length of output: 186

src/packages/uploader/demos/taro/demo5.tsx (2)

1-5: 代码结构清晰,使用了适当的类型定义!

组件的基础架构符合 React 最佳实践,使用了 TypeScript 类型定义确保类型安全。


6-12: 文件类型过滤实现仍需改进

之前的审查意见仍然有效。

src/packages/uploader/demos/h5/demo1.tsx (1)

1-2: 类型导入需要优化

建议将类型从专门的类型定义文件中导入,以保持一致性:

import React, { useState } from 'react'
-import { Uploader, Cell, FileItem } from '@nutui/nutui-react'
+import { Uploader, Cell } from '@nutui/nutui-react'
+import type { FileItem } from '@nutui/nutui-react/dist/types'
src/packages/uploader/demos/taro/demo1.tsx (1)

38-44: 建议添加状态管理相关的测试用例

需要为组件添加以下测试场景:

  1. 文件列表的初始状态验证
  2. setList 回调函数的调用验证
  3. 上传成功和失败的状态变化验证
src/packages/uploader/demos/h5/demo2.tsx (1)

47-49: 请参考之前的代码审查建议

之前的代码审查中已经指出了 onDelete 处理器的改进建议,这些建议仍然有效。

src/packages/uploader/demos/h5/demo10.tsx (1)

1-3: 导入声明看起来不错!

代码使用了命名导入,结构清晰,包含了所有必要的依赖项。

src/packages/uploader/demos/taro/demo9.tsx (1)

1-3: 导入声明看起来不错!

导入的依赖项都是必要的,并且符合组件的功能需求。

src/packages/uploader/uploader.tsx (1)

27-27: 请确认新的上传函数接口设计

upload 函数的设计简化了组件接口,提供了更大的灵活性。但是默认的 maxCount 设置为 Number.MAX_VALUE 可能会带来潜在的内存问题。建议设置一个更合理的默认值,比如 10 或 20。

Also applies to: 55-55

src/packages/uploader/uploader.taro.tsx (1)

242-280: ⚠️ Potential issue

建议增强错误处理和重试机制

当前上传实现存在以下问题:

  1. 错误处理过于简单,仅打印错误
  2. 缺少重试机制
  3. 没有上传进度回调

建议添加以下功能:

  1. 完善的错误处理回调
  2. 自动重试机制
  3. 上传进度通知
- ).catch((errs) => console.error(errs))
+ ).catch((errs) => {
+   const retryCount = 3;
+   const retryDelay = 1000;
+   
+   const retry = async (task: FileItem, attempt: number) => {
+     try {
+       const result = await upload(task.file as File);
+       updateFileStatus(task.uid, 'success', result.url);
+     } catch (error) {
+       if (attempt < retryCount) {
+         setTimeout(() => retry(task, attempt + 1), retryDelay);
+       } else {
+         updateFileStatus(task.uid, 'error', locale.uploader.error);
+         onError?.(error, task);
+       }
+     }
+   };
+   
+   tasks.forEach(task => retry(task, 0));
+ })

Likely invalid or redundant comment.

src/packages/uploader/demos/taro/demo2.tsx (2)

47-48: 删除事件处理逻辑完善

onDelete函数正确地处理了文件删除事件,并输出了相关信息,逻辑清晰,符合预期。


51-56: Uploader组件配置合理

Uploader组件的配置满足示例需求,使用了自定义的uploadIcon,并设置了合适的maxCount,组件显示效果良好。

src/packages/uploader/demos/h5/demo4.tsx Show resolved Hide resolved
src/packages/uploader/demos/h5/demo3.tsx Show resolved Hide resolved
src/packages/uploader/demos/h5/demo3.tsx Show resolved Hide resolved
src/packages/uploader/demos/h5/demo5.tsx Show resolved Hide resolved
src/packages/uploader/demos/h5/demo5.tsx Show resolved Hide resolved
src/packages/uploader/demos/h5/demo10.tsx Show resolved Hide resolved
src/packages/uploader/demos/taro/demo9.tsx Show resolved Hide resolved
src/packages/uploader/demos/taro/demo9.tsx Show resolved Hide resolved
src/packages/uploader/uploader.tsx Outdated Show resolved Hide resolved
src/packages/uploader/uploader.taro.tsx Show resolved Hide resolved
@oasis-cloud
Copy link
Collaborator

AI 针对 uloader.tsx 和 uploader.taro.tsx 文件提出的建议中,重点关注抛错给用户的细节处理。可以把这部分补充下。

@oasis-cloud
Copy link
Collaborator

需要增加 V2 升级 V3 的说明文档。

@oasis-cloud
Copy link
Collaborator

image 鸿蒙下可点击区域太小。

@Alex-huxiyang
Copy link
Collaborator Author

image 鸿蒙下可点击区域太小。

这个是受到了button组件可点击区域的影响了,右侧部分点击失效。应该会一起修复

image 鸿蒙下可点击区域太小。

Copy link

@coderabbitai coderabbitai bot left a 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 (4)
src/packages/uploader/uploader.tsx (1)

Line range hint 282-349: UI渲染逻辑可以进一步优化

当前实现已经将图片上传器和列表上传器的渲染逻辑分离,这是个很好的改进。不过还可以进一步优化:

  1. 建议将通用的input部分抽取为独立组件:
interface UploaderInputProps {
  capture: boolean | 'user' | 'environment'
  name: string
  accept: string
  disabled: boolean
  multiple: boolean
  onChange: (event: React.ChangeEvent<HTMLInputElement>) => void
}

const UploaderInput: React.FC<UploaderInputProps> = (props) => (
  <input
    className="nut-uploader-input"
    type="file"
    {...props}
  />
)
  1. 使用组合模式重构渲染逻辑:
-  const renderImageUploader = () => {
+  const ImageUploader: React.FC<{show: boolean}> = ({show}) => {
     return (
-      shouldShow && (
+      show && (
         <div className={classNames('nut-uploader-upload', previewType)}>
           {/* ... */}
-          <input {...inputProps} />
+          <UploaderInput {...inputProps} />
         </div>
       )
     )
   }
src/packages/uploader/uploader.taro.tsx (3)

81-91: 接口设计改进建议

新增的属性改进了上传控制,但建议进一步完善类型定义:

  1. upload 方法返回的 FileItem 应该有更明确的类型约束
  2. beforeUpload 的返回值类型 File[] 可能与 Taro 环境不完全匹配

建议添加以下类型定义:

interface UploadResult extends FileItem {
  url: string;
  status: 'success' | 'error';
  message?: string;
}

upload: (file: File | Taro.chooseImage.ImageFile | Taro.chooseMedia.ChooseMedia) => Promise<UploadResult>

Line range hint 284-321: 建议增强文件处理逻辑

文件处理逻辑可以进一步优化:

  1. 文件类型检测可以更严格
  2. 缺少文件大小的预检查
  3. 文件名处理需要考虑更多场景

建议进行以下优化:

 const readFile = async <T extends TFileType>(files: T[]) => {
+  // 预检查文件
+  const validateFile = (file: T) => {
+    if (!file) return false
+    // 检查文件大小
+    if (file.size > maxFileSize) return false
+    // 检查文件类型
+    const allowedTypes = accept.split(',').map(type => type.trim())
+    if (allowedTypes[0] !== '*' && !allowedTypes.includes(file.type)) return false
+    return true
+  }
+
   const tasks = files.map((file) => {
+    if (!validateFile(file)) {
+      throw new Error(`文件 ${file.name} 验证失败`)
+    }

391-425: 建议优化无障碍访问支持

上传组件的UI渲染需要增强无障碍支持:

  1. 缺少适当的 ARIA 属性
  2. 键盘导航支持不足
  3. 上传状态提示可以更友好

建议添加以下优化:

 const renderListUploader = () => {
   return (
     previewType === 'list' && (
-      <View className="nut-uploader-slot">
+      <View 
+        className="nut-uploader-slot"
+        role="region"
+        aria-label={locale.uploader.uploadArea}
+      >
         <>
-          <Button size="small" type="primary">
+          <Button 
+            size="small" 
+            type="primary"
+            aria-label={locale.uploader.selectFile}
+          >
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 22c6899 and 4cf906b.

📒 Files selected for processing (2)
  • src/packages/uploader/uploader.taro.tsx (10 hunks)
  • src/packages/uploader/uploader.tsx (10 hunks)
🔇 Additional comments (1)
src/packages/uploader/uploader.tsx (1)

Line range hint 17-113: 接口重构改进了组件的可维护性!

接口重构带来以下改进:

  1. 移除了与HTTP请求相关的属性(url、timeout等),改为更灵活的upload函数
  2. 新增了onOverCount和onUploadQueueChange回调,增强了组件的可控性
  3. 使用mergeProps工具统一处理属性,提高了代码的可维护性

src/packages/uploader/uploader.tsx Show resolved Hide resolved
src/packages/uploader/uploader.tsx Show resolved Hide resolved
src/packages/uploader/uploader.taro.tsx Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants