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

feat: adds removeFiles, onRemoveFiles #942

Closed

Conversation

estrattonbailey
Copy link

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
I came across a situation where I'd like to be able to remove files from the internal state of react-dropzone. I saw that others have asked for this, and that help was needed.

This PR introduces two new API methods onRemoveFiles and removeFiles:

<Dropzone onRemoveFiles={files => console.log('removed', files)}>
  {({ removeFiles, acceptedFiles }) => (
    <button onClick={() => removeFiles(acceptedFiles[0])}>Remove one file</button>
  )}
</Dropzone>

onRemoveFiles will always receive an array of Files, and removeFiles always accepts an array of Files. This makes it easy to remove multiple files at a time, say to "clear" the dropzone if needed.

Does this PR introduce a breaking change?
Shouldn't.

Other information
I could use some feedback on the tests, types, and JSDoc. Generally tried to stick to the patterns in place, but lmk if those need work.

Also, thanks for the library :)

const removeFiles = React.useCallback(
files => {
const acceptedFiles = state.acceptedFiles.filter(acceptedFile => {
return !files.find(f => f === acceptedFile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Array.prototype.find is not supported in IE: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find

This project doesn't have an official list of supported browsers. See #630. That being said, the project isn't using find anywhere else, so this would be a breaking change for any project that isn't polyfilling find and that has IE users.

I don't maintain this project but I think it would be better to use Array.prototype.some instead. :)

@estrattonbailey
Copy link
Author

@nylon22 thanks for the feedback.

In interest of time, I've actually just wrapped this library to create an API that's more suitable to my needs. I made a gist that outlines the approach I took.

Because of that, I'm not going to follow up on this PR, and I think I'll close to help reduce clutter.

@Angelk90
Copy link

@estrattonbailey : I have a similar problem can I ask you for a hand on something?

@rajesh-cp
Copy link

Virat - Copy

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.

None yet

4 participants