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

support Android #13

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

support Android #13

wants to merge 12 commits into from

Conversation

choru-k
Copy link

@choru-k choru-k commented Jul 2, 2016

No description provided.

@aroth
Copy link
Owner

aroth commented Jul 3, 2016

This is great! Thanks for the PR -- a lot of people
will benefit from your work. I will review and merge shortly.

@choru-k
Copy link
Author

choru-k commented Jul 7, 2016

:D Could you please merge the request?

@aroth
Copy link
Owner

aroth commented Jul 9, 2016

I've had a chance to quickly review the PR. I'd like to see a few changes:

  • Naming
    • I use RNUploader, you're using RNFileTransfer. Please use RNUploader for consistency.
  • Feature Parity
    • I would like both iOS and Android versions to behave the same, including:
      • Support for POST and PUT
      • Support for custom headers
      • Support for custom params
      • Support for base64 data
      • Progress support
      • Cancellation support
  • A working example for Android under examples/.
  • Documentation
    • I'd like cleaner documentation detailing the Android implementation, but I don't mind taking care of this once I'm content with the PR.

Do you think you could take implement any of these requests such that both iOS and Android implementations support the same features?

Thank you again for your work.

@GeoffreyPlitt
Copy link

I'm on RN 0.32 and your module gives me "Cannot call upload of undefined" because NativeModules.FileTransfer does not exist.

Your module seems to depend on NativeModules.FileTransfer, but this does not exist for me (I console.log it out and it's NULL)

@aroth
Copy link
Owner

aroth commented Sep 14, 2016

Hi @GeoffreyPlitt -- I'll take a look with a current version of RN next week. Can you post a separate issue please?

@aroth aroth mentioned this pull request Sep 14, 2016
@GeoffreyPlitt
Copy link

Will do

@faceyspacey
Copy link

+1

1 similar comment
@RichardLindhout
Copy link

+1

@StevePotter
Copy link

+1 and I'd be happy to do the necessary updates. Would you merge a suitable version?

@BilalBudhani
Copy link

Can we follow same name spacing between android and ios implementation?

@StevePotter
Copy link

I would also add a JS interface to hide the awkward DeviceEmitter stuff that needs to go into code. So the API would be exactly the same in iOS and Android.

@superandrew213
Copy link

Anyone still working on this?

@daywong1119
Copy link

I got error on sync gradle Error:Configuration with name 'default' not found.

@WillyRamirez
Copy link

@aroth will this lib ever support android? it looks like this PR has been abandoned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.