fix: Upload drag & drop not working #625
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改将原有的文件拖拽和粘贴事件处理逻辑进行重构，将原本合并的 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DOM
participant AjaxUploader
participant UploadLogic
User->>DOM: 拖拽文件/粘贴文件
DOM->>AjaxUploader: 触发 onFileDrop/onFilePaste/onFileDragOver
AjaxUploader->>AjaxUploader: onFileDragOver (仅阻止默认)
AjaxUploader->>AjaxUploader: onFileDrop/onFilePaste
AjaxUploader->>AjaxUploader: onDataTransferFiles(dataTransfer)
AjaxUploader->>UploadLogic: 处理文件/目录、过滤、上传
Possibly related PRs
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
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. src/AjaxUploader.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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 @@
## master #625 +/- ##
==========================================
+ Coverage 89.07% 89.10% +0.03%
==========================================
Files 6 6
Lines 302 303 +1
Branches 87 80 -7
==========================================
+ Hits 269 270 +1
Misses 33 33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/AjaxUploader.tsx (3)
95-108: 粘贴事件处理器逻辑正确，但建议简化条件检查。方法正确处理了粘贴事件并调用了统一的文件处理逻辑。
考虑简化条件检查，因为方法名已经表明了它的用途：
- if (e.type === 'paste') { - const clipboardData = (e as ClipboardEvent).clipboardData; - return this.onDataTransferFiles(clipboardData, () => { - e.preventDefault(); - }); - } + const clipboardData = e.clipboardData; + return this.onDataTransferFiles(clipboardData, () => { + e.preventDefault(); + });
114-122: 拖拽释放处理器设计合理，但建议简化条件检查。方法正确处理了拖拽释放事件并防止了默认行为。
类似于粘贴处理器，可以简化条件检查：
- if (e.type === 'drop') { - const dataTransfer = (e as React.DragEvent<HTMLDivElement>).dataTransfer; - - return this.onDataTransferFiles(dataTransfer); - } + const dataTransfer = e.dataTransfer; + return this.onDataTransferFiles(dataTransfer);
130-130: 事件监听器绑定更新正确，但可以优化内存管理。事件监听器的添加和移除逻辑正确，符合 React 生命周期最佳实践。
根据检索到的学习记录，考虑使用预绑定的方法而不是
.bind(this)来避免潜在的内存泄漏。当前实现已经很好地使用了预绑定的方法（this.onFilePaste），这是正确的做法。另外，建议在
componentWillUnmount中添加条件检查：componentWillUnmount() { this._isMounted = false; this.abort(); - document.removeEventListener('paste', this.onFilePaste); + if (this.props.pastable) { + document.removeEventListener('paste', this.onFilePaste); + } }Also applies to: 137-137, 144-144, 146-146
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/AjaxUploader.tsx(4 hunks)tests/uploader.spec.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/AjaxUploader.tsx (1)
Learnt from: zombieJ
PR: react-component/upload#543
File: src/AjaxUploader.tsx:116-116
Timestamp: 2025-04-09T06:22:00.335Z
Learning: 在 react-component/upload 项目中，黏贴上传功能应该是可配置的，只有当启用此功能时才应添加 paste 事件监听器，而不是无条件添加。同时，事件监听器绑定应使用预先绑定的方法而非直接使用 .bind(this) 以避免内存泄漏。
🔇 Additional comments (4)
tests/uploader.spec.tsx (1)
738-754: 测试增强很好地验证了 preventDefault 的调用。这个测试补充完美地配合了代码重构，确保
dragOver事件期间正确调用preventDefault。测试结构清晰，使用了适当的 spy 模式来验证方法调用，并在断言后正确恢复了 spy。src/AjaxUploader.tsx (3)
69-93: 新的 onDataTransferFiles 方法设计良好，提供了统一的文件处理逻辑。这个方法很好地统一了粘贴、拖拽等事件的文件处理逻辑，支持目录和单文件上传，并正确处理文件过滤和多文件限制。使用可选的
existFileCallback参数允许调用者在检测到文件时执行特定操作（如调用 preventDefault）。
110-112: 拖拽悬停处理器完美解决了原始问题。这个简单但关键的方法通过在
dragOver事件期间调用preventDefault解决了拖拽上传不工作的核心问题。实现简洁明了。
346-347: 事件属性绑定正确更新以匹配新的方法结构。将
onDrop和onDragOver绑定到新的专用方法是正确的，确保了拖拽功能的正常工作。
故障原因
在 #624 中，对存在文件的情况进行
preventDefault处理。这本对于 Paste 和 Drog 都会有效，但是由于原来的onFileDropOrPaste方法其实除了处理paste和drop事件外，还处理了onDragOver事件。而onDragOver事件是没有 files 要处理的，导致其不会触发preventDefault逻辑。当文件拖拽不处理
dragOver的preventDefault时，drop也会被浏览器忽略。此外，测试用例中也仅仅做了
drop事件的测试，而没有做dragOver测试导致preventDefault逃逸。修复方式
重构了这部分代码，将三个事件拆分成独立的函数进行分别处理。对于相同逻辑处，提取
onDataTransferFiles抽象逻辑。Summary by CodeRabbit