-
-
Notifications
You must be signed in to change notification settings - Fork 320
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: support paste upload file #543
base: master
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #543 +/- ##
==========================================
+ Coverage 88.84% 89.93% +1.08%
==========================================
Files 6 6
Lines 278 298 +20
Branches 73 83 +10
==========================================
+ Hits 247 268 +21
+ Misses 31 30 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
是不是要跟拖拽上传一样风格 <Dragger {...props}> |
This branch has conflicts that must be resolved. |
Walkthrough此次变更新增了两个 Markdown 文件( Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Uploader as AjaxUploader
participant RCUpload as rc-upload 组件
User->>Uploader: 鼠标进入上传区域 (handleMouseEnter)
User->>Uploader: 粘贴文件 (paste event)
Uploader->>Uploader: onPrePaste 检测 isMouseEnter
alt 鼠标在范围内
Uploader->>Uploader: 执行 onFileDropOrPaste 处理粘贴文件
Uploader->>RCUpload: 触发文件上传流程
else
Uploader-->>User: 忽略粘贴事件
end
User->>Uploader: 鼠标离开上传区域 (handleMouseLeave)
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
docs/examples/paste.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. docs/examples/pasteDirectory.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. tests/uploader.spec.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
@afc163 done |
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
🧹 Nitpick comments (10)
docs/demo/paste.md (1)
8-8
: 代码标签格式需要修正代码引用标签的格式有误。当前的
<code src="../examples/paste.tsx"/></code>
同时包含了自闭合标签和关闭标签,这是不正确的 Markdown 语法。建议修改为以下格式:
-<code src="../examples/paste.tsx"/></code> +<code src="../examples/paste.tsx" />docs/demo/pasteDirectory.md (1)
8-8
: 代码标签格式需要修正代码引用标签的格式有误。当前的
<code src="../examples/pasteDirectory.tsx"/></code>
同时包含了自闭合标签和关闭标签,这是不正确的 Markdown 语法。建议修改为以下格式:
-<code src="../examples/pasteDirectory.tsx"/></code> +<code src="../examples/pasteDirectory.tsx" />docs/examples/paste.tsx (2)
6-8
: 建议增加更多文件类型支持当前仅接受
.png
文件类型,考虑到粘贴上传的场景,用户可能会粘贴其他格式的图片(如 JPEG、GIF 等)。- accept: '.png', + accept: 'image/*',
36-36
: 考虑国际化支持组件中硬编码了中文文本 "开始上传",这可能不利于国际化。
考虑使用配置项或国际化工具来处理文本:
- <a>开始上传</a> + <a>{props.buttonText || '开始上传'}</a>或者在实际的项目中,使用国际化工具如 i18n:
- <a>开始上传</a> + <a>{intl.get('upload.start') || '开始上传'}</a>docs/examples/pasteDirectory.tsx (2)
8-9
: 建议增加更多文件类型支持当前仅接受
.png
文件类型,考虑到目录上传的场景,用户可能会上传各种格式的文件。- accept: '.png', + accept: '*/*',
37-37
: 考虑国际化支持组件中硬编码了中文文本 "开始上传",这可能不利于国际化。
考虑使用配置项或国际化工具来处理文本:
- <a>开始上传</a> + <a>{props.buttonText || '开始上传'}</a>或者在实际的项目中,使用国际化工具如 i18n:
- <a>开始上传</a> + <a>{intl.get('upload.start') || '开始上传'}</a>tests/uploader.spec.tsx (3)
59-59
: 改进错误处理方式这里的修改改进了错误处理,但可以进一步优化使用可选链操作符。
- return error && error(new Error('read file error')); + return error?.(new Error('read file error'));🧰 Tools
🪛 Biome (1.9.4)
[error] 59-59: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
376-379
: 考虑直接在 rcUpload 上触发粘贴事件目前测试在 input 上触发粘贴事件,但在真实场景中,粘贴事件是在文档级别被捕获的,并且在实现中只有当鼠标位于上传组件上时才处理。为保持一致性,建议直接在 rcUpload 元素上触发粘贴事件。
fireEvent.mouseEnter(rcUpload); -fireEvent.paste(input, { +fireEvent.paste(rcUpload, { clipboardData: { files }, });
444-446
: 与之前建议一致,考虑在 rcUpload 上触发粘贴事件同样的问题出现在这个测试中,建议在 rcUpload 上触发粘贴事件而不是 input。
fireEvent.mouseEnter(rcUpload); -fireEvent.paste(input, { clipboardData: { files } }); +fireEvent.paste(rcUpload, { clipboardData: { files } });src/AjaxUploader.tsx (1)
108-112
: 粘贴事件预处理方法可以优化此方法实现粘贴功能,但有两点可以改进:
- 缺少对禁用状态的检查
- 没有考虑到如果用户正在编辑文本时,阻止默认行为可能会干扰正常文本粘贴
onPrePaste(e: ClipboardEvent) { - if (this.isMouseEnter) { + // 检查组件是否启用并且鼠标在组件上 + if (this.isMouseEnter && !this.props.disabled) { + // 只有在剪贴板包含文件时才阻止默认行为 + if (e.clipboardData?.files?.length > 0) { this.onFileDropOrPaste(e); + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/demo/paste.md
(1 hunks)docs/demo/pasteDirectory.md
(1 hunks)docs/examples/paste.tsx
(1 hunks)docs/examples/pasteDirectory.tsx
(1 hunks)src/AjaxUploader.tsx
(4 hunks)tests/uploader.spec.tsx
(10 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
src/AjaxUploader.tsx (2)
src/interface.tsx (1) (1)
RcFile
(78-80)src/attr-accept.ts (1) (1)
file
(4-53)
tests/uploader.spec.tsx (2)
src/AjaxUploader.tsx (1) (1)
render
(295-361)src/Upload.tsx (1) (1)
render
(37-39)
🪛 Biome (1.9.4)
tests/uploader.spec.tsx
[error] 59-59: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (15)
docs/demo/paste.md (1)
1-8
: 文件结构与元数据设置正确Markdown 文件的 frontmatter 部分设置合理,包含了标题和导航路径的定义,这有助于在文档系统中正确显示和定位此示例。
docs/demo/pasteDirectory.md (1)
1-8
: 文件结构与元数据设置正确Markdown 文件的 frontmatter 部分设置合理,包含了标题和导航路径的定义,这有助于在文档系统中正确显示和定位此示例。
docs/examples/paste.tsx (2)
5-25
: 配置对象结构清晰
props
对象的配置结构清晰,包含了上传所需的各种回调函数和样式属性。日志输出有助于调试和理解上传流程。
27-41
: 组件实现简洁,但缺少实际的粘贴功能说明虽然文件名为
paste.tsx
,但组件中并没有明确处理粘贴事件的代码。根据 PR 的目标(支持粘贴上传文件),建议添加一些注释或文本说明,告知用户如何使用粘贴功能。return ( <div style={{ margin: 100, }} > <div> + <p style={{ marginBottom: 10 }}>支持粘贴上传文件 (Ctrl+V)</p> <Upload {...props}> <a>开始上传</a> </Upload> </div> </div> );
docs/examples/pasteDirectory.tsx (2)
5-26
: 配置对象结构清晰
props
对象的配置结构清晰,包含了上传所需的各种回调函数和样式属性。日志输出有助于调试和理解上传流程。
28-42
: 组件实现简洁,但缺少实际的粘贴功能说明虽然文件名为
pasteDirectory.tsx
,但组件中并没有明确处理粘贴事件的代码,也没有关于如何通过粘贴方式上传目录的说明。根据 PR 的目标(支持粘贴上传文件),建议添加一些注释或文本说明。return ( <div style={{ margin: 100, }} > <div> + <p style={{ marginBottom: 10 }}>支持粘贴上传目录 (Ctrl+V)</p> <Upload {...props}> <a>开始上传</a> </Upload> </div> </div> );
tests/uploader.spec.tsx (5)
352-384
: 新增粘贴上传功能测试看起来很全面这个测试用例验证了粘贴文件上传功能的正常工作,覆盖了鼠标进入、粘贴事件触发和请求响应等关键流程。设计合理,模拟了真实用户交互。
386-407
: 不符合类型的粘贴文件测试逻辑完善这个测试正确验证了当粘贴不符合 accept 类型的文件时,不会触发 onStart 回调。测试场景考虑全面。
479-492
: 使用箭头函数和期望对象更新了测试断言将函数声明更新为箭头函数并更新了期望对象中的断言,代码风格更加现代化和一致。
506-519
: 添加鼠标进入和离开事件测试这个测试很好地验证了 onMouseEnter 和 onMouseLeave 回调函数是否正确调用,支持新增的粘贴功能所需的鼠标状态跟踪。
785-805
: 粘贴目录功能测试覆盖全面测试正确验证了粘贴包含目录结构的功能,清晰地模拟了用户交互并验证了预期行为。
src/AjaxUploader.tsx (4)
31-31
: 新增鼠标状态跟踪属性添加 isMouseEnter 属性用于跟踪鼠标是否悬停在上传组件上,为支持粘贴上传功能奠定基础。
71-106
: 重构方法支持文件拖放和粘贴将 onFileDrop 重命名并扩展为 onFileDropOrPaste,支持处理拖放和粘贴事件。实现逻辑清晰,根据事件类型正确获取文件和项目。
283-293
: 鼠标事件处理方法实现良好handleMouseEnter 和 handleMouseLeave 方法实现简洁明了,既更新内部状态又调用可选的外部回调函数。
329-332
: 事件监听更新支持新功能事件处理器的更新正确使用了新添加的方法,支持粘贴上传和鼠标状态跟踪功能。
componentDidMount() { | ||
this._isMounted = true; | ||
document.addEventListener('paste', this.onPrePaste.bind(this)); |
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.
🛠️ Refactor suggestion
事件监听器绑定方式可能导致内存泄漏
每次调用 .bind(this)
都会创建一个新的函数实例,这可能导致添加和移除的不是同一个函数引用,从而可能引起内存泄漏。
+constructor(props) {
+ super(props);
+ this.boundOnPrePaste = this.onPrePaste.bind(this);
+}
componentDidMount() {
this._isMounted = true;
- document.addEventListener('paste', this.onPrePaste.bind(this));
+ document.addEventListener('paste', this.boundOnPrePaste);
}
componentWillUnmount() {
this._isMounted = false;
this.abort();
- document.removeEventListener('paste', this.onPrePaste.bind(this));
+ document.removeEventListener('paste', this.boundOnPrePaste);
}
📝 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.
document.addEventListener('paste', this.onPrePaste.bind(this)); | |
class AjaxUploader extends React.Component<Props, State> { | |
constructor(props) { | |
super(props); | |
this.boundOnPrePaste = this.onPrePaste.bind(this); | |
} | |
componentDidMount() { | |
this._isMounted = true; | |
document.addEventListener('paste', this.boundOnPrePaste); | |
} | |
componentWillUnmount() { | |
this._isMounted = false; | |
this.abort(); | |
document.removeEventListener('paste', this.boundOnPrePaste); | |
} | |
onPrePaste(event) { | |
// existing logic for handling paste events | |
} | |
// additional component methods... | |
} |
ref: ant-design/ant-design#46429
Summary by CodeRabbit
文档
新功能
测试