Skip to content
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

bug:change isDragReject true when maxFiles rejectDrag #1020

Merged
merged 11 commits into from Oct 28, 2020

Conversation

morijenab
Copy link
Contributor

What kind of change does this PR introduce?

  • bugfix
  • feature
  • refactoring / style
  • build / chore
  • documentation

Did you add tests for your changes?

  • Yes, my code is well tested
  • Not relevant

If relevant, did you update the documentation?

  • Yes, I've updated the documentation
  • Not relevant

Summary
There is a bug in maxFiles feature that isDragReject doesn't change when DragEvent contains more than the max number of files
This bug reported #1017

Does this PR introduce a breaking change?

shouldn't
Other information

@coveralls
Copy link

coveralls commented Oct 16, 2020

Pull Request Test Coverage Report for Build f75389a2e2c95503c910f25fbd75e86b442f8290-PR-1020

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.3%) to 99.18%

Totals Coverage Status
Change from base Build 1b1177daaa51d7cc542d59f32dfd1e2956b92a55: -0.3%
Covered Lines: 201
Relevant Lines: 201

💛 - Coveralls

@coveralls
Copy link

coveralls commented Oct 16, 2020

Pull Request Test Coverage Report for Build 26c7ed2059c3b678c151d05ed9ed4b5a664ad252-PR-1020

  • 2 of 2 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 99.454%

Totals Coverage Status
Change from base Build 96e7c150b008132347c0132363bd94d5e5993601: 0.005%
Covered Lines: 201
Relevant Lines: 201

💛 - Coveralls

Copy link
Collaborator

@rxmarbles rxmarbles left a comment

Choose a reason for hiding this comment

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

LGTM just need to fix a few spacing issues

src/index.js Outdated Show resolved Hide resolved
src/utils/index.js Outdated Show resolved Hide resolved
Co-authored-by: Rick Markins <rmarkins@gmail.com>
@rolandjitsu
Copy link
Collaborator

I think we're missing some tests, @Morteza-Jenabzadeh could you add some test cases for these changes?

@morijenab
Copy link
Contributor Author

I think we're missing some tests, @Morteza-Jenabzadeh could you add some test cases for these changes?

I have added a test to check isDragReject.

src/utils/index.js Show resolved Hide resolved
Copy link
Collaborator

@rolandjitsu rolandjitsu left a comment

Choose a reason for hiding this comment

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

@Morteza-Jenabzadeh just a few more style changes and we're good to merge.

src/utils/index.spec.js Outdated Show resolved Hide resolved
src/utils/index.spec.js Show resolved Hide resolved
src/index.spec.js Outdated Show resolved Hide resolved
@rolandjitsu rolandjitsu merged commit b4a1ac6 into react-dropzone:master Oct 28, 2020
@github-actions
Copy link

🎉 This PR is included in version 11.2.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants