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 input to WebView #12807

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
@farazs
Contributor

farazs commented Mar 8, 2017

Fixes #5219 which was moved to ProductPains. Allows file input in a webview. Previously attempting to use file input would do nothing in the webview. Now the default dialog is shown allowing you to pick Camera to take a picture or Photos/Gallery to select an existing photo.

The problem in fixing this before was that this was accomplished by using undocumented APIs for earlier versions of Android. However, for 5.0+ there is a documented API for this. Since there is no documented way to fix this for earlier versions of Android, it seems better to have a solution for 5.0+ than no solution at all.

Based on this example: https://github.com/GoogleChrome/chromium-webview-samples/tree/master/input-file-example/

The ActivityEventListener does not work if it is attached to the ThemedReactContext itself but needs to be added to the original ReactContext similar to how LifecycleEventListener is handled.

@yihanseattle

This comment has been minimized.

yihanseattle commented Mar 15, 2017

works perfectly for me! just what we need.

@farazs

This comment has been minimized.

Contributor

farazs commented Mar 20, 2017

Tested manually on: Samsung Galaxy S7 Edge, Samsung Galaxy S5, Nexus 6P emulator, LG v20

@farazs farazs changed the title from Add file input to webview to Add file input to Webview Mar 30, 2017

@farazs farazs changed the title from Add file input to Webview to Add file input to WebView Mar 30, 2017

@farazs

This comment has been minimized.

Contributor

farazs commented Apr 3, 2017

Fixes #11230

@farazs farazs force-pushed the farazs:webview_file_input branch from 3a8de30 to 1a44b86 Apr 11, 2017

@prithsharma

This comment has been minimized.

prithsharma commented Apr 25, 2017

Any update or ETA on this?

@farazs

This comment has been minimized.

Contributor

farazs commented Apr 25, 2017

Sadly no one has reviewed it yet.

@headlessme

This comment has been minimized.

headlessme commented May 3, 2017

Would be great to see this merged. @jacobp100

@jacobp100

This comment has been minimized.

Contributor

jacobp100 commented May 3, 2017

Sorry, I don't have power to merge things! The code looks good, but I just have a few questions,

Is it possible to handle this use-case using #10946? I know that PR hasn't landed, but it is useful for other reviewers to know.

Does this work for things uploads than images?

What happens if a user is on an older version of Android?

@farazs

This comment has been minimized.

Contributor

farazs commented May 3, 2017

To answer your questions:

  1. No, I believe the changes to ThemedReactContext would still be necessary for this to work even with #10946
  2. I don't believe so. I was unable to select an audio file for a file input with no accept attribute.
  3. I cannot say for sure without testing on a real device but on a JellyBean AVD Nexus 5 (emulator), when clicking/tapping a file input, nothing happens. Same with KitKat Nexus 5 AVD. On Nexus 5 emulator with a later version of Android, it works.
@headlessme

This comment has been minimized.

headlessme commented May 15, 2017

Anyone know who needs pinging to get this reviewed?

@jacobp100

This comment has been minimized.

Contributor

jacobp100 commented May 15, 2017

@headlessme @farazs I think the ThemedReactContext changes should be fine to go in.

If the ThemedReactContext changes did get merged, I believe you would be able to implement this with said PR (and even publish it as a module!). That would mean who ever merges this doesn't have to review things like device compatibility or consider uploading use cases for things other than images.

@farazs

This comment has been minimized.

Contributor

farazs commented May 15, 2017

sounds good. I can update the PR to include only those changes.

I'm not sure how close that PR is to being merged though. Is it possible it becomes stale and does not end up being merged?

@jacobp100

This comment has been minimized.

Contributor

jacobp100 commented May 15, 2017

I'm working hard with the author to get it published soon!

@niztal

This comment has been minimized.

niztal commented May 16, 2017

Why doesn't anyone merge it? We need it ASAP

@shergin shergin added the Android label May 31, 2017

@monsty

This comment has been minimized.

monsty commented Jun 7, 2017

Any update ?

@farazs

This comment has been minimized.

Contributor

farazs commented Jun 7, 2017

#10946 has still not been merged. So if this PR will not be merged as is, it's not possible to implement this using that until it is merged and released in a new RN version.

@0is1 0is1 referenced this pull request Jun 8, 2017

Closed

Feedback view #25

@andreipfeiffer

This comment has been minimized.

andreipfeiffer commented Jun 12, 2017

I'm interested in this merge also.

@cbrevik

This comment has been minimized.

Contributor

cbrevik commented Aug 19, 2017

Hey @hramos. I agree this would work as an external package.

That would be a lot easier if #15016 was merged. Would you mind looking at that PR (I see you're already set as a reviewer)?

@mantou132

This comment has been minimized.

mantou132 commented Aug 23, 2017

@farazs
this PR can support file multiple choice ?

@farazs

This comment has been minimized.

Contributor

farazs commented Aug 23, 2017

@mantou132 I'm not sure what you mean by that, but you're welcome to try it out and see! :)

@mantou132

This comment has been minimized.

mantou132 commented Aug 30, 2017

multiple file & MIME type:

       if (fileChooserParams.getMode() == FileChooserParams.MODE_OPEN_MULTIPLE) {
          contentSelectionIntent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, true);
        }
        ...
        String[] acceptTypes = fileChooserParams.getAcceptTypes();
        contentSelectionIntent.setType(acceptTypes[0].isEmpty() ? "*/*" : acceptTypes[0]);

Also do not need to set the title:
chooserIntent.putExtra(Intent.EXTRA_TITLE, "Image Chooser");

multiple file receive:

        if(resultCode == Activity.RESULT_OK) {
          if(data == null) {
            // If there is not data, then we may have taken a photo
            if(mCameraPhotoPath != null) {
              results = new Uri[]{Uri.parse(mCameraPhotoPath)};
            }
          } else {
            String dataString = data.getDataString();
            if (dataString != null) {
              results = new Uri[]{Uri.parse(dataString)};
            } else {
              if (data.getClipData() != null) {
                results = new Uri[data.getClipData().getItemCount()];
                for (int i = 0; i < data.getClipData().getItemCount(); i++) {
                  results[i] = data.getClipData().getItemAt(i).getUri();
                }
              } else {
                results = null;
              }
            }
          }
        }

https://stackoverflow.com/questions/42919913/multiple-file-upload-from-android-webview-kitkat-and-lower-android-versions

@JustPassingThrough

This comment has been minimized.

JustPassingThrough commented Sep 6, 2017

It seems like #15016 was approved, any chance this might be merged anytime soon?

@Titozzz

This comment has been minimized.

Collaborator

Titozzz commented Sep 7, 2017

I need this too 😢 , any chance it's coming soon ?

@SaKKo

This comment has been minimized.

SaKKo commented Sep 20, 2017

I need this too T____T

@0x5e

This comment has been minimized.

0x5e commented Oct 12, 2017

Need this too :-)

@hramos

This comment has been minimized.

Contributor

hramos commented Oct 12, 2017

Please go ahead and and split this out into a separate package, now that the other PR has landed.

@hramos hramos closed this Oct 12, 2017

@dahjelle

This comment has been minimized.

dahjelle commented Nov 13, 2017

@farazs Now that #15016 is merged, is it possible to implement WebView file upload either ourselves or as an external package? I'm looking at it, but I haven't yet figured out yet if that'd work. Or is a RN fork still the best option?

@dahjelle

This comment has been minimized.

dahjelle commented Nov 17, 2017

Here's a sample of what I did to get Android WebView file upload working in my project, in case it is helpful to anyone else.

I suspect one could make an external package based off of that, but I've not dove in to that.

Hope it helps someone!

@spearmootz

This comment has been minimized.

spearmootz commented Dec 6, 2017

this is terrible! why would this be split up in a different package?

for the sake of argument, this package can become stale and not get any new features that get added to react-native.

@farazs could this work/be made as a higher order component?

@JustPassingThrough

This comment has been minimized.

JustPassingThrough commented Dec 11, 2017

Still waiting for this, with an app which should've gone into production a long time ago.
@farazs or anyone else, what can be done to get this feature added?

@hramos

This comment has been minimized.

Contributor

hramos commented Dec 22, 2017

WebView accounts for a good chunk of issues opened on the repo, but these issues don't seem to affect Facebook's own internal use cases. This is why I highly recommend that the community step in and provide a WebView module.

@farazs

This comment has been minimized.

Contributor

farazs commented Apr 4, 2018

Sorry I have not had time to take a look at this. It doesn't seem like it will get merged into the WebView component itself though I'm not sure why. Are no further improvements going to be made to the react-native webview? Until it's actually pulled out into it's own community WebView module how are any updates going to be made?

Someone would have to put the time creating and owning the WebView module and I don't think I can be that person. If someone creates such a module I would be happy to port this PR over there.

@guguji5

This comment has been minimized.

guguji5 commented Apr 24, 2018

Hi @farazs, thanks for your share. Unfortunately it doesn't work to me.
I have modify the react native source code according to the changes of this PR . But the input type=file is missing in my webview.

@guguji5

This comment has been minimized.

guguji5 commented Apr 25, 2018

@yihanseattle the change of this PR works? could you share some experience?

@farazs

This comment has been minimized.

Contributor

farazs commented Apr 25, 2018

@guguji5 I'm not sure what you mean. Even without support for , the issue is that nothing happens in the webview. Are you saying the element itself is not in your webview? That may be an issue with the page you are trying to load.

You would need to fork react native, apply these changes to that fork, and then use that fork when building your application. See https://facebook.github.io/react-native/docs/building-from-source.html

@farazs

This comment has been minimized.

Contributor

farazs commented Apr 25, 2018

We are using these changes in production so they definitely do work for me. master may have diverged from this branch significantly so the changes might need to be altered slightly.

@guguji5

This comment has been minimized.

guguji5 commented Apr 25, 2018

@farazs yes. I can't see the element input type=file of my page in the android webview. it's a little weird.

@guguji5

This comment has been minimized.

guguji5 commented Apr 25, 2018

@farazs I modify the react native file in the node_module folder of my project according to this PR changes, and run react-native run-android . But it seems nothing changed.

@farazs

This comment has been minimized.

Contributor

farazs commented Apr 25, 2018

@guguji5 Please see the link I posted. Editing the node_modules folder is not sufficient to get your project to build the source from there.

For further issues, your questions might be better suited to stackoverflow as it's not specific to this pull request but regarding building react-native from source for Android in general.

@guguji5

This comment has been minimized.

guguji5 commented Apr 25, 2018

@farazs ok .thank you very much.

kseniamukhortova pushed a commit to kseniamukhortova/react-native that referenced this pull request Jul 16, 2018

YUSHEN2015 added a commit to YUSHEN2015/react-native that referenced this pull request Jul 25, 2018

YUSHEN2015 added a commit to YUSHEN2015/react-native that referenced this pull request Jul 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment