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.

Sign up for GitHub

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

Jump to bottom

F/media uploader #343

Merged
merged 10 commits into from
Oct 26, 2018
Merged

F/media uploader #343

merged 10 commits into from
Oct 26, 2018

Conversation

radditude
Copy link
Contributor

connects to #342

Adds a media uploader component that wraps the FileInput component and:

  • handles previewing images and videos
  • validates by size and file type
  • can optionally handle uploading to an S3 bucket via presigned URLs

oct-25-2018 17-19-47

bryanjos
bryanjos reviewed Oct 25, 2018
View reviewed changes
Copy link
Contributor

@bryanjos bryanjos left a comment

Choose a reason for hiding this comment

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

Just one comment about expected response from signing server for S3 uploads. It's fine for us, but could be made more generic in the future.

Or actually...maybe instead of having the prop to a function that returns a url, have it be a function that returns the signed request url for uploading and the url to access the picture.

src/MediaUploader/utils.js Outdated
}
}

xhr.open('PUT', data.signed_request)
Copy link
Contributor

Choose a reason for hiding this comment

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

data and it's properties may be too specific and need to be made more general for larger use, but for our purposes it works fine

@radditude
Copy link
Contributor Author

Oooh, good idea. I was having a hard time abstracting that logic in a way that was general enough.

@radditude
Copy link
Contributor Author

@bryanjos updated! Now the prop function just needs to return an object with fileUrl and signedRequestUrl keys.

prehnRA
prehnRA approved these changes Oct 26, 2018
View reviewed changes
Copy link
Contributor

@prehnRA prehnRA left a comment

Choose a reason for hiding this comment

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

Well, I've got nothing to add. This is great work!

@prehnRA prehnRA merged commit f891b6f into master Oct 26, 2018
@prehnRA prehnRA deleted the f/media-uploader branch October 26, 2018 14:48
revelry-integrations pushed a commit that referenced this pull request Oct 26, 2018
@semantic-release-bot
chore(release): Bump version to 4.3.0 [skip ci]
84201b1 
# [4.3.0](v4.2.3...v4.3.0) (2018-10-26)

### Features

* Add MediaUploader component ([#343](#343), [#342](#342)) ([f891b6f](f891b6f))
@revelry-integrations
Copy link

🎉 This PR is included in version 4.3.0 🎉

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
Reviewers

@bryanjos bryanjos bryanjos left review comments

@prehnRA prehnRA prehnRA approved these changes

Assignees

@bryanjos bryanjos

@prehnRA prehnRA

Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@radditude @revelry-integrations @bryanjos @prehnRA @CheetoMao