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

Set initial files with File or URL string #194

Merged
merged 5 commits into from
Jun 14, 2020

Conversation

isaacbuckman
Copy link
Contributor

Description

  • Fixes set initialFiles with File object array #192
  • Change initialFiles to take a File[]
  • Refactor the old initialFiles (taking string[]) to initialFileURLs
  • Add example to docs of setting initialFiles with simple txt
  • All changes made to both DropzoneArea and DropzoneDialog

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
    • initialFiles now takes File[] instead of string[]

Test Configuration:

  • Browser: Chrome

@isaacbuckman isaacbuckman changed the title Set initial file objs set initial files with File or URL string Jun 9, 2020
@isaacbuckman isaacbuckman changed the title set initial files with File or URL string Set initial files with File or URL string Jun 9, 2020
@panz3r
Copy link
Contributor

panz3r commented Jun 9, 2020

Hi @isaacbuckman ,

Thanks for your contribution, I'd try to avoid such a breaking change and instead prefer to add a condition on the initialFiles prop.

I'd make the prop accept both string and fileObject and based on the type make the this.loadInitialFiles method call createFileFromUrl(url) or skip it.

@panz3r panz3r self-requested a review June 9, 2020 20:35
@panz3r panz3r added this to the 3.3 milestone Jun 9, 2020
@isaacbuckman
Copy link
Contributor Author

Hey @panz3r,

I agree that the breaking change is probably unnecessary for now. Maybe it is something we can do in the future. I updated the files so the change is no longer breaking.

Copy link
Contributor

@panz3r panz3r left a comment

Choose a reason for hiding this comment

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

A bit of changes to the typings and it's good for me.

P.S. Can you please do a git rebase after the changes to keep only the required commits (you can remove all the commits up to and including the Revert one)?

src/components/DropzoneArea.js Outdated Show resolved Hide resolved
src/components/DropzoneArea.js Outdated Show resolved Hide resolved
src/components/DropzoneDialog.js Show resolved Hide resolved
src/components/DropzoneDialog.js Outdated Show resolved Hide resolved
src/index.d.ts Outdated Show resolved Hide resolved
src/index.d.ts Outdated Show resolved Hide resolved
@isaacbuckman isaacbuckman force-pushed the set-initial-file-objs branch from 919eae6 to c9b489a Compare June 10, 2020 17:28
@isaacbuckman
Copy link
Contributor Author

@panz3r Removed the old commits so we should be good now.

@panz3r
Copy link
Contributor

panz3r commented Jun 10, 2020

LGTM 👍

Thanks again for your contribution!

⚠️ Waiting to merge because I'd like to finish some bugfixes on 3.2 before starting 3.3.

@panz3r panz3r merged commit a385dda into Yuvaleros:master Jun 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set initialFiles with File object array
2 participants