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

Add file download support for Android #203

Merged

Conversation

smathson
Copy link
Contributor

Addresses #80.

Caveat: I am not an Android developer. This code comes from a fork of the original RN WebView that we have been using in production for some time, so all credit goes to @Oblongmana: https://github.com/Oblongmana/react-native-webview-file-upload-android.

Setting up a DownloadManager for the WebView is pretty straightforward, as is adding any known cookies to the request. Most of the complication comes from the requirement after SDK 23 to ask the user for the WRITE_EXTERNAL_STORAGE permission. Unfortunately there is no mechanism to suspend the download request until permission is resolved so this code stores off the request and sets up a listener that enqueues the download once permissions are resolved so the user experience is really nice.

I didn't see anything in the way of tests or documentation that needs to be added for this change, so let me know if I missed anything. Thanks!

@jamonholmgren
Copy link
Member

Hey @smathson, this looks really cool! I'm cloning down your fork and trying it out now.

@jamonholmgren
Copy link
Member

jamonholmgren commented Dec 13, 2018

This is very cool. Screenshots:

Shoot, screenshots didn't work. Anyway, this looks good to me.

EDIT: here's two

image

image

@jamonholmgren jamonholmgren added the Maintainer: ready to publish This PR has been reviewed and is ready to be published label Dec 13, 2018
jamonholmgren
jamonholmgren previously approved these changes Dec 13, 2018
Copy link
Member

@jamonholmgren jamonholmgren left a comment

Choose a reason for hiding this comment

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

I tested this and it looks/works great.

@Titozzz
Copy link
Collaborator

Titozzz commented Dec 14, 2018

@andreipfeiffer Would you mind reviewing this ? You spent a lot of time doing the upload so you probably have useful insights on this! 😄

@andreipfeiffer
Copy link
Contributor

Hey guys,

Great work @smathson :D
In this situation, I guess that the user would like the downloaded file to be in a shared folder, that he could open/read with other apps. This means that the WRITE_EXTERNAL_STORAGE permission is mandatory here.

I would add in the docs that this permission is required for downloading files.

I will also test this in the next days.

@smathson
Copy link
Contributor Author

@andreipfeiffer Good idea. Do you think we should add it to https://github.com/react-native-community/react-native-webview/blob/master/docs/Guide.md alongside the File Upload support or somewhere else?

@andreipfeiffer
Copy link
Contributor

@smathson you're right, it should definitely go in there.

@smathson
Copy link
Contributor Author

Added documentation re: permissions and merged master.

docs/Guide.md Outdated Show resolved Hide resolved
@andreipfeiffer
Copy link
Contributor

Hey @smathson,
I added a comment regarding the docs.

Also, is the download attr supported in iOS by default?
Is so, it should be specified in the docs.
If some setting is necessary, it should be specified what is needed to make it work.
It it's not supported, it should also be specified, and hopefully somebody can step-up to add support for it.

Copy link
Contributor

@andreipfeiffer andreipfeiffer left a comment

Choose a reason for hiding this comment

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

Looks great to me 👍

Copy link
Member

@jamonholmgren jamonholmgren left a comment

Choose a reason for hiding this comment

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

Let's ship it.

@jamonholmgren
Copy link
Member

@Titozzz I'm going to let you push the merge button 😀

@Titozzz Titozzz merged commit 2114a9b into react-native-webview:master Jan 7, 2019
Titozzz pushed a commit that referenced this pull request Jan 7, 2019
# [2.15.0](v2.14.3...v2.15.0) (2019-01-07)

### Features

* **Android Webview:** Add file download support for Android ([#203](#203)) ([2114a9b](2114a9b)), closes [#80](#80)
@Titozzz
Copy link
Collaborator

Titozzz commented Jan 7, 2019

🎉 This PR is included in version 2.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Titozzz
Copy link
Collaborator

Titozzz commented Jan 18, 2019

@allcontributors[bot] please add @smathson for code, doc

@allcontributors
Copy link
Contributor

@Titozzz

I've put up a pull request to add @smathson! 🎉

Titozzz pushed a commit that referenced this pull request Jan 18, 2019
Adds @smathson as a contributor for code, doc.

This was requested by Titozzz [in this comment](#203 (comment))
@smathson smathson deleted the android-file-downloads branch January 29, 2019 04:23
phuongwd pushed a commit to phuongwd/react-native-webview that referenced this pull request Apr 29, 2020
…ative-webview#203)

Addresses react-native-webview#80.

Caveat: I am not an Android developer. This code comes from a fork of the original RN WebView that we have been using in production for some time, so all credit goes to @Oblongmana: https://github.com/Oblongmana/react-native-webview-file-upload-android.

Setting up a DownloadManager for the WebView is pretty straightforward, as is adding any known cookies to the request. Most of the complication comes from the requirement after SDK 23 to ask the user for the WRITE_EXTERNAL_STORAGE permission. Unfortunately there is no mechanism to suspend the download request until permission is resolved so this code stores off the request and sets up a listener that enqueues the download once permissions are resolved so the user experience is really nice.

I didn't see anything in the way of tests or documentation that needs to be added for this change, so let me know if I missed anything. Thanks!
phuongwd pushed a commit to phuongwd/react-native-webview that referenced this pull request Apr 29, 2020
phuongwd pushed a commit to phuongwd/react-native-webview that referenced this pull request Apr 29, 2020
Adds @smathson as a contributor for code, doc.

This was requested by Titozzz [in this comment](react-native-webview#203 (comment))
noproblem23 added a commit to noproblem23/react-native-webview that referenced this pull request Apr 18, 2023
noproblem23 added a commit to noproblem23/react-native-webview that referenced this pull request Apr 18, 2023
Adds @smathson as a contributor for code, doc.

This was requested by Titozzz [in this comment](react-native-webview/react-native-webview#203 (comment))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintainer: ready to publish This PR has been reviewed and is ready to be published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants