-
-
Notifications
You must be signed in to change notification settings - Fork 327
feat: Support non-click folder upload #659
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
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @linhf123, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在增强文件上传组件的功能,通过引入一种新的模式,允许用户仅通过拖拽或粘贴操作来上传整个文件夹,同时阻止通过点击触发的文件选择器选择文件夹。这为开发者提供了更灵活的控制,以适应不同的用户交互需求,特别是在需要区分文件和文件夹上传行为的场景中。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
你好,感谢你的贡献。这个 PR 增加了对非点击文件夹上传的支持,通过为 directory
prop 添加 'nonClick'
选项实现。
整体实现思路是正确的。但我在代码中发现了一些相关的问题,并提出了一些改进建议:
- 在
AjaxUploader.tsx
中,有几个地方只处理了directory
prop 而忽略了folder
prop,这会导致使用folder
时功能异常。这是一个需要修复的 bug。 - 在
src/interface.tsx
中,新功能被添加到了一个已弃用的directory
prop 上。为了 API 的清晰和一致性,建议将此功能也应用到folder
prop 上。
我已在具体的代码文件中留下了详细的审查意见和修改建议。请查看。
// because input don't have directory/webkitdirectory type declaration | ||
const dirProps: any = | ||
directory || folder ? { directory: 'directory', webkitdirectory: 'webkitdirectory' } : {}; | ||
directory === true || folder ? { directory: 'directory', webkitdirectory: 'webkitdirectory' } : {}; |
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.
你好,此处的修改正确处理了 directory
为 'nonClick'
时的情况,避免了点击时触发目录选择。
然而,我注意到组件中的其他地方未能正确地同时处理 directory
和 folder
这两个 prop,这会导致使用 folder
prop 时出现 bug。既然你正在修改相关的逻辑,建议在此 PR 中一并修复:
-
onDataTransferFiles
方法 (L70, L79):
此方法用于处理拖拽上传。目前它只解构并检查了directory
prop,忽略了folder
。这导致当使用folder={true}
时,拖拽上传文件夹的功能会失效。- 应修改为:
const { multiple, accept, directory, folder } = this.props;
- 以及
if (directory || folder)
- 应修改为:
-
onChange
方法 (L34, L37):
此方法在通过文件对话框选择文件/文件夹后触发。它同样只检查了directory
prop 来决定是否要对文件进行accept
过滤。当使用folder={true}
时,文件过滤将不会按预期工作。- 应修改为:
const { accept, directory, folder } = this.props;
- 以及
(file: RcFile) => !(directory || folder) || attrAccept(file, accept)
- 应修改为:
修复这些问题可以确保在使用推荐的 folder
prop 时,组件功能正常。
method?: UploadRequestMethod; | ||
/** @deprecated Please use `folder` instead */ | ||
directory?: boolean; | ||
directory?: boolean | 'nonClick'; |
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.
ref #522 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #659 +/- ##
=======================================
Coverage 89.03% 89.03%
=======================================
Files 6 6
Lines 301 301
Branches 82 82
=======================================
Hits 268 268
Misses 33 33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@zombieJ 帮忙看看 |
// because input don't have directory/webkitdirectory type declaration | ||
const dirProps: any = | ||
directory || folder ? { directory: 'directory', webkitdirectory: 'webkitdirectory' } : {}; | ||
directory === true || folder ? { directory: 'directory', webkitdirectory: 'webkitdirectory' } : {}; |
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.
directory 已经标注废弃,改用 folder,所以你两个都得判断
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.
folder 不会走以下逻辑(在拖拽、复制时对文件夹内容进行读取并拍平),原 PR 看着只是开启原生文件夹选择
#643
Lines 69 to 93 in 4958954
onDataTransferFiles = async (dataTransfer: DataTransfer, existFileCallback?: () => void) => { | |
const { multiple, accept, directory } = this.props; | |
const items: DataTransferItem[] = [...(dataTransfer.items || [])]; | |
let files: File[] = [...(dataTransfer.files || [])]; | |
if (files.length > 0 || items.some(item => item.kind === 'file')) { | |
existFileCallback?.(); | |
} | |
if (directory) { | |
files = await traverseFileTree(Array.prototype.slice.call(items), (_file: RcFile) => | |
attrAccept(_file, this.props.accept), | |
); | |
this.uploadFiles(files); | |
} else { | |
let acceptFiles = [...files].filter((file: RcFile) => attrAccept(file, accept)); | |
if (multiple === false) { | |
acceptFiles = files.slice(0, 1); | |
} | |
this.uploadFiles(acceptFiles); | |
} | |
}; |
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.
拖拽场景下只能获取到文件夹对象(需要用户手动处理),不能获取内部文件
手动处理指的是怎么处理?
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.
需要用户自己实现 onDataTransferFiles
中的实现,才可以获取文件夹下的文件
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.
所以 folder
预期拖拽也需要直接返回文件夹中的子文件吗?
支持拖拽、粘贴文件夹,点击时为非

directory
模式