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

fix: `isDragReject` now true if a file is not within size range #878

Merged
merged 3 commits into from Oct 6, 2019

Conversation

@Dehorser
Copy link
Contributor

Dehorser commented Sep 14, 2019

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

Previously, isDragReject would not trigger if a file was not within the specified size range. For example, it would not return true if a file was too large. See: #861. This is because it was not checking for size.

Does this PR introduce a breaking change?

No

Other information

Fixes #861

@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 14, 2019

Codecov Report

Merging #878 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #878   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines         204    205    +1     
  Branches       67     68    +1     
=====================================
+ Hits          204    205    +1
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️
src/utils/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4644e8e...6e1bf3b. Read the comment docs.

@Dehorser

This comment has been minimized.

Copy link
Contributor Author

Dehorser commented Sep 14, 2019

Suggestion: onDropAccepted and isDragAccepted should probably behave similarly. A file should trigger both or neither, but not only one.

They both rely on the expression fileAccepted(file, accept) && fileMatchSize(file, maxSize, minSize). Thus, I suggest extracting this to its own function, as this will couple onDropAccepted and isDragAccepted. I could include this change in this PR.

@rolandjitsu

This comment has been minimized.

Copy link
Collaborator

rolandjitsu commented Sep 18, 2019

Please rebase your PR. I will check it soon to verify that this change doesn't break some intended behavior.

Previously, `isDragReject` would not trigger if a file exceeded the maximum size. See: #861. This is because it was not checking for size.

fix: create new test, fix previous tests
@Dehorser Dehorser force-pushed the Dehorser:isDragReject-size branch from 27ae49f to ccc2d7f Sep 18, 2019
@Dehorser

This comment has been minimized.

Copy link
Contributor Author

Dehorser commented Sep 18, 2019

Please rebase your PR. I will check it soon to verify that this change doesn't break some intended behavior.

Just rebased it!

rolandjitsu added 2 commits Oct 6, 2019
@rolandjitsu rolandjitsu merged commit c8a7322 into react-dropzone:master Oct 6, 2019
4 checks passed
4 checks passed
codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 4644e8e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@okonet

This comment has been minimized.

Copy link
Collaborator

okonet commented Oct 6, 2019

🎉 This PR is included in version 10.1.10 🎉

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
Projects
None yet
3 participants
You can’t perform that action at this time.