Skip to content

fix: when the upload setting pastable is set to true in the same Form, the copying of FormItemInput becomes invalid #624

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

Merged
merged 3 commits into from
May 27, 2025

Conversation

jin19980928
Copy link
Contributor

@jin19980928 jin19980928 commented May 26, 2025

fix ant-design/ant-design#53907

Summary by CodeRabbit

  • Bug Fixes
    • 优化了拖拽或粘贴文件时的事件处理逻辑,仅在检测到实际文件时才阻止默认行为,避免对非文件拖拽或粘贴操作产生影响。
    • 新增粘贴事件处理的测试用例,确保只有包含文件的粘贴事件会阻止默认行为。

Copy link

vercel bot commented May 26, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
upload ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 27, 2025 1:57am

Copy link

coderabbitai bot commented May 26, 2025

Walkthrough

此次更改优化了AjaxUploader组件中onFileDropOrPaste方法的事件处理逻辑。现在,只有在检测到事件中实际包含文件时,才会调用e.preventDefault(),以避免对非文件拖拽或粘贴操作的不必要干预。

Changes

文件/路径 变更摘要
src/AjaxUploader.tsx 优化onFileDropOrPaste方法,增加对事件中是否包含文件的判断,仅在存在文件时调用e.preventDefault(),避免无意义阻止默认行为。
tests/uploader.spec.tsx 新增粘贴事件测试用例,验证带文件和不带文件的粘贴事件中preventDefault调用的正确性。

Assessment against linked issues

Objective Addressed Explanation
确保Upload组件设置pastable属性时,不影响同一表单中其他组件的粘贴功能 (#53907)

Possibly related PRs

  • feat: support paste upload file #543: 该PR首次引入了AjaxUploader.tsxonFileDropOrPaste方法的粘贴上传支持,当前改动基于此进行了事件处理逻辑的细化。

Suggested reviewers

  • zombieJ

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/AjaxUploader.tsx

Oops! 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.tsx

Oops! 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.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0c133a and 38e2be0.

📒 Files selected for processing (2)
  • src/AjaxUploader.tsx (1 hunks)
  • tests/uploader.spec.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/AjaxUploader.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/uploader.spec.tsx (1)
src/AjaxUploader.tsx (1)
  • render (300-368)
🔇 Additional comments (2)
tests/uploader.spec.tsx (2)

492-509: 测试实现正确,验证了带文件的粘贴事件处理逻辑。

这个测试用例正确地验证了当粘贴事件包含文件时,preventDefault 方法应被调用的行为。测试设置合理,使用 jest.spyOn 监视 Event.prototype.preventDefault 的调用,并在测试结束后正确恢复 spy。测试数据模拟了包含文件的 clipboardData,符合实际使用场景。


511-526: 测试实现正确,验证了不包含文件的粘贴事件处理逻辑。

这个测试用例正确地验证了当粘贴事件不包含文件时,preventDefault 方法不应被调用的行为。这是此次修复的核心部分 - 避免对非文件粘贴操作的不必要干预,从而解决了在表单中启用 pastable 设置时 FormItemInput 复制功能失效的问题。

测试设置与第一个测试保持一致,使用了正确的模拟数据(kind: 'string' 和空的 files 数组)来模拟不包含文件的粘贴事件。

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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.

Support

Need 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)

  • @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 generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this 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.

} else if (e.type === 'paste') {
const clipboardData = (e as ClipboardEvent).clipboardData;
items = [...(clipboardData.items || [])];
files = [...(clipboardData.files || [])];
hasFile = items.some(item => item.kind === 'file') || files.length > 0;
Copy link
Member

Choose a reason for hiding this comment

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

test case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok


if (e.type === 'drop') {
const dataTransfer = (e as React.DragEvent<HTMLDivElement>).dataTransfer;
items = [...(dataTransfer.items || [])];
files = [...(dataTransfer.files || [])];
hasFile = items.some(item => item.kind === 'file');
Copy link
Member

Choose a reason for hiding this comment

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

这个逻辑和下面的不一样。看起来 hasFile 变量不用定义。直接在下面写 if 就行了,逻辑合并掉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -87,6 +85,10 @@ class AjaxUploader extends Component<UploadProps> {
files = [...(clipboardData.files || [])];
}

if (items.some(item => item.kind === 'file') || files.length > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

我感觉交换一下位置,相对耗能的在后面

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link

codecov bot commented May 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.07%. Comparing base (a0e8f5a) to head (38e2be0).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #624      +/-   ##
==========================================
+ Coverage   89.03%   89.07%   +0.03%     
==========================================
  Files           6        6              
  Lines         301      302       +1     
  Branches       86       87       +1     
==========================================
+ Hits          268      269       +1     
  Misses         33       33              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zombieJ zombieJ merged commit 4b79f45 into react-component:master May 27, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

在Upload组件设置pastable属性的话,同一表单中的<Input/>组件的粘贴功能失效
3 participants