-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(uploader): add onOverLimit props & fix incorrect updating files when type = multiple #2566
Conversation
Walkthrough此次更改引入了一个新的可选回调函数 Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #2566 +/- ##
==========================================
- Coverage 83.88% 83.87% -0.02%
==========================================
Files 218 218
Lines 17888 17893 +5
Branches 2609 2609
==========================================
+ Hits 15006 15007 +1
- Misses 2877 2881 +4
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
src/packages/uploader/uploader.tsx (1)
Line range hint
290-317
: 使用临时数组_files
来累积文件项,提高了代码的清晰度。这是一个很好的改进,确保了无论文件是否为图像,状态都能够一致地更新。
提醒:为未覆盖的代码行添加测试。
静态分析工具指出,此代码段中的一些新增行没有被测试覆盖:
- 第290行
- 第308-309行
- 第313-314行
为了确保代码的可靠性,请为这些行添加测试。
如果需要帮助编写测试,请告诉我,我很乐意提供帮助。
Tools
GitHub Check: codecov/patch
[warning] 308-309: src/packages/uploader/uploader.tsx#L308-L309
Added lines #L308 - L309 were not covered by tests
[warning] 313-314: src/packages/uploader/uploader.tsx#L313-L314
Added lines #L313 - L314 were not covered by tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (7)
src/packages/uploader/doc.zh-TW.md (1)
177-177
: 建議增加onOverLimit
屬性的詳細說明新增的
onOverLimit
屬性描述簡潔明瞭,但可以考慮進行以下改進:
- 提供更詳細的解釋,說明該回調函數的具體用途和使用場景。
- 添加一個簡單的代碼示例,展示如何使用該屬性。
- 解釋該屬性與
maxCount
屬性的關係,說明它們如何協同工作。- 考慮在文檔的"示例代碼"部分添加一個使用
onOverLimit
的實際例子。這些改進將有助於用戶更好地理解和使用這個新屬性。
src/packages/uploader/doc.md (1)
178-178
: 新增的onOverLimit
属性增强了组件功能新增的
onOverLimit
回调函数是一个很好的补充,它允许开发者处理文件数量超过限制的情况。这个新属性与现有的文档风格和格式保持一致,增强了 Uploader 组件的灵活性。为了进一步提高文档的完整性,建议考虑添加一个简单的示例,展示如何使用这个新的回调函数。这将帮助开发者更好地理解和实现这个功能。
您是否需要我为这个新属性提供一个简单的使用示例?
src/packages/uploader/doc.taro.md (1)
178-178
: 新增的onOverLimit
属性文档看起来不错,建议稍作改进。新增的
onOverLimit
回调属性文档已正确添加到 Props 表格中,包含了名称、描述、类型和默认值。这个添加很好地补充了组件的功能文档。为了使描述更加清晰,建议稍微扩展一下描述,以包含回调函数接收的参数信息。建议将描述修改为:
-| onOverLimit | 文件数量超过限制时触发 | `(param: {responseText: XMLHttpRequest['responseText'];option: UploadOptions;files: FileItem[]}) => void` | `-` | +| onOverLimit | 文件数量超过限制时触发。回调函数接收一个包含 responseText、option 和 files 的对象作为参数。 | `(param: {responseText: XMLHttpRequest['responseText'];option: UploadOptions;files: FileItem[]}) => void` | `-` |这样可以让用户更清楚地了解回调函数的参数结构。
src/packages/uploader/doc.en-US.md (2)
177-177
: 新属性onOverLimit
的添加是正确的,但可以稍作改进。
onOverLimit
属性的添加解决了之前的问题,描述现在正确地表明它在文件数量超过maxCount
时触发。然而,为了更清晰,我们可以稍微调整描述。建议修改为:
-| onOverLimit | Triggered when the number of files exceeds the maxCount | `(file: File[]) => void` | `-` | +| onOverLimit | Triggered when the number of selected files exceeds the maxCount limit | `(files: File[]) => void` | `-` |这个小改动可以使描述更加精确,并且参数名称从
file
改为files
更准确地反映了它是一个数组。
Line range hint
9-17
: 建议改进 "Basic usage" 部分的描述为了提高文档的清晰度和一致性,建议对 "Basic usage" 部分进行小幅改进。
建议修改如下:
## Basic usage :::demo <CodeBlock src='h5/demo1.tsx'></CodeBlock> ::: -> When using the Uploader component to upload files, you may encounter the problem of garbled Chinese characters in the response file information. This usually happens when the client and server are inconsistent in how they handle the encoding of the file. To avoid this problem, it is recommended to ensure that the encoding format of the file read by the server is consistent with that of the client. +> 注意:使用 Uploader 组件上传文件时,可能会遇到响应文件信息中出现乱码的问题。这通常发生在客户端和服务器对文件编码的处理方式不一致时。为避免此问题,建议确保服务器读取文件的编码格式与客户端一致。以下是一个处理示例:这个改动将注意事项更清晰地标记出来,并提供了更具体的上下文。
src/packages/uploader/uploader.tsx (1)
Line range hint
4-4
: 提醒:请注意解决 TODO 注释中提到的测试缺失问题。代码中存在一个关于添加测试的 TODO 注释。虽然这不是新的变更,但解决这个问题对于提高代码的整体质量和可靠性非常重要。建议尽快添加相应的测试用例。
如果需要帮助编写测试用例,我可以提供一些建议或示例代码。您希望我为哪些特定功能生成测试用例吗?
src/packages/uploader/uploader.taro.tsx (1)
Line range hint
394-439
:readFile
函数可能覆盖已有的文件列表在
readFile
函数中,初始化了临时数组_files
,并在处理完文件后直接使用setFileList(_files)
更新文件列表。这可能会覆盖之前已存在的文件列表,导致之前的文件被丢弃。建议在更新文件列表时,合并之前的文件列表,以保留已上传的文件。可以修改为:
- setFileList(_files) + setFileList([...fileListRef.current, ..._files])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/packages/uploader/doc.en-US.md (1 hunks)
- src/packages/uploader/doc.md (1 hunks)
- src/packages/uploader/doc.taro.md (1 hunks)
- src/packages/uploader/doc.zh-TW.md (1 hunks)
- src/packages/uploader/uploader.taro.tsx (5 hunks)
- src/packages/uploader/uploader.tsx (5 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/uploader/uploader.tsx
[warning] 293-293: src/packages/uploader/uploader.tsx#L293
Added line #L293 was not covered by tests
[warning] 311-312: src/packages/uploader/uploader.tsx#L311-L312
Added lines #L311 - L312 were not covered by tests
[warning] 316-317: src/packages/uploader/uploader.tsx#L316-L317
Added lines #L316 - L317 were not covered by tests
[warning] 335-335: src/packages/uploader/uploader.tsx#L335
Added line #L335 was not covered by tests
🔇 Additional comments (7)
src/packages/uploader/doc.zh-TW.md (1)
Line range hint
1-277
: 文檔更新總體評價新增的
onOverLimit
屬性是對 Uploader 組件功能的有益補充。文檔的整體結構良好,新屬性的添加與現有文檔風格一致。然而,為了進一步提高文檔質量,建議考慮以下幾點:
- 在"示例代碼"部分添加一個使用
onOverLimit
的實際例子,以展示其實際應用。- 更新"基礎用法"或"限制上傳數量"的示例,加入
onOverLimit
的使用說明。- 在 Props 表格中,考慮將相關的屬性(如
maxCount
和onOverLimit
)放在一起,以便用戶更容易理解它們之間的關係。- 檢查是否需要更新其他語言版本的文檔(如簡體中文和英文),以保持所有語言版本的一致性。
這些小改進將使文檔更加全面,有助於用戶更好地理解和使用 Uploader 組件的所有功能。
src/packages/uploader/uploader.tsx (4)
68-68
: 新增onOverLimit
回调函数,增强了文件上传限制的处理能力。这个新增的可选回调函数很好地实现了PR的目标,为处理超出预定义文件数量限制的场景提供了灵活的解决方案。它的类型定义与接口中的其他回调函数保持一致,增强了组件的可扩展性。
293-293
: 优化了文件读取过程中的状态更新逻辑。这个改动通过引入
_files
数组来收集所有FileItem
实例,并将setFileList
调用移到循环外部,有效地解决了多文件上传时的状态更新问题。这种方法不仅提高了性能,还确保了状态更新的正确性,符合 React 的最佳实践。然而,静态分析工具指出这些新增的代码行没有被测试覆盖。建议为这些更改添加相应的单元测试,以确保功能的可靠性。
是否需要我协助编写这部分的单元测试?
Also applies to: 311-312, 316-317
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 293-293: src/packages/uploader/uploader.tsx#L293
Added line #L293 was not covered by tests
335-335
: 实现了文件数量超限时的回调函数。这个改动很好地实现了PR的目标,在文件数量超过最大限制时调用
onOverLimit
回调函数。这增强了组件处理文件上传限制的能力,为使用者提供了更多的控制权。然而,静态分析工具指出这一新增的代码行没有被测试覆盖。建议为这个新功能添加单元测试,以确保其在各种情况下都能正确工作。
是否需要我协助编写这部分的单元测试?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 335-335: src/packages/uploader/uploader.tsx#L335
Added line #L335 was not covered by tests
Line range hint
1-458
: 总体评价:成功实现了PR的目标,提升了上传组件的功能。这次更改很好地实现了PR的两个主要目标:
- 添加了
onOverLimit
回调函数,用于处理文件数量超过预定义限制的情况。- 修复了多文件上传时文件更新不正确的问题。
代码质量整体良好,逻辑清晰。然而,还有一些地方可以进一步改进:
- 新增的代码缺少测试覆盖,建议添加相应的单元测试。
- 存在一个未解决的TODO注释,提醒添加测试。
建议在合并此PR之前,着重关注提高测试覆盖率,以确保新增功能的可靠性和稳定性。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 311-312: src/packages/uploader/uploader.tsx#L311-L312
Added lines #L311 - L312 were not covered by tests
[warning] 316-317: src/packages/uploader/uploader.tsx#L316-L317
Added lines #L316 - L317 were not covered by testssrc/packages/uploader/uploader.taro.tsx (2)
115-117
: 新增onOverLimit
回调函数,功能完善添加了可选的
onOverLimit
回调函数,用于在选择的文件数量超过最大限制时触发,设计合理,满足需求。
205-205
: 解构提取onOverLimit
从 props 中解构出
onOverLimit
,方便在组件内部使用,代码清晰。
@@ -448,6 +456,7 @@ const InternalUploader: ForwardRefRenderFunction< | |||
const currentFileLength = filterFile.length + fileList.length | |||
if (currentFileLength > maximum) { | |||
filterFile.splice(filterFile.length - (currentFileLength - maximum)) | |||
onOverLimit?.(filterFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onOverLimit
回调传递的文件列表可能不正确
在 filterFiles
函数中,当文件总数超过最大限制时,使用 splice
方法移除了多余的文件。但随后调用 onOverLimit?.(filterFile)
时,传递的是已经被修改后的 filterFile
,这可能导致回调函数无法获取到超出限制的文件列表。
建议在移除超出限制的文件之前,先保存这些文件,然后在回调中传递给 onOverLimit
。修改如下:
+ const overLimitFiles = filterFile.slice(-(currentFileLength - maximum))
filterFile.splice(filterFile.length - (currentFileLength - maximum))
- onOverLimit?.(filterFile)
+ onOverLimit?.(overLimitFiles)
📝 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.
onOverLimit?.(filterFile) | |
const overLimitFiles = filterFile.slice(-(currentFileLength - maximum)) | |
filterFile.splice(filterFile.length - (currentFileLength - maximum)) | |
onOverLimit?.(overLimitFiles) |
新增onOverLimit回调
修复多个文件上传的所选文件的错误
新特性提交
日常 bug 修复
站点、文档改进
TypeScript 定义更新
Summary by CodeRabbit
新功能
onOverLimit
回调函数,用于处理超过文件上传限制的情况,提供用户反馈。文档更新
onOverLimit
的使用和功能说明,确保用户了解新属性及其用途。