Prevent onDrop from being overridden

[alukach]

Currently, defining onDrop in props overrides the Dropzone component's onDrop parameter due to the spread props.

I believe that this should fix #46.

[traveled1]

Thanks so much!! Will pull the latest!! :)

[ilyakatz]

Just tried it out and works great, would be good to merge it in

[ilyakatz]

Actually, if the onDrop is used for validation, what do you think of something like this

    if (!this.props.onDrop || this.props.onDrop(files, rejectedFiles)) {
      new S3Upload(options);
    }

And require onDrop to return true/falsy value?

[alukach]

@ilyakatz can you describe what the advantage would be for this second method?

[ilyakatz]

According to this discussion, it looks like creators of the module want to use onDrop for validation. So if validation fails, we should tell react-dropzone-s3-uploder not to upload file into s3

[alukach]

I don't think any change in logic is necessary, this is a tiny bugfix. I believe that, with this fix, the module works as intended.

[ilyakatz]

Well, current fix is definitely better than nothing, but it doesn't allow for validation of file prior to upload to S3, which is what it seems onDrop is supposed to do

[alukach]

This module simply glues react-dropzone and react-s3-loader together. react-dropzone has tooling to use the provided onDrop handler. My understanding is that this module is not responsible for any validation, rather it should pass the appropriate onDrop logic to react-dropzone where that logic will be handled. Our goal here is to get react-dropzone the information it needs so that it can do its job (such as validation).

…

On Fri, Mar 16, 2018, 4:16 PM Ilya Katz, ***@***.***> wrote: Well, current fix is definitely better than nothing, but it doesn't allow for validation of file prior to upload to S3, which is what it seems onDrop is supposed to do — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#52 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2xCvaAzZiGMY2rZy2QQTej6MDcwYEKks5tfDmngaJpZM4R4ygq> .

[alukach]

@founderlab any input on this?