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

Multiple data type #84

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

Conversation

arribbar
Copy link
Contributor

@arribbar arribbar commented Mar 2, 2018

  • Example and Readme updated

iOS and Android

  • Allow sharing multi content
  • Share images, texts or urls
  • Return a list like : [{ type, value }, { type, value }]

Android

  • We get as well video and audio

@arribbar
Copy link
Contributor Author

@alinz Do you have some time to take a look at this PR, please ? :)

@pierretanguay
Copy link

Works perfectly for me. Tried it in a live project.

@abarisic86
Copy link

abarisic86 commented Apr 27, 2018

How can one extend this to accept application/octet? That should be similar to audio/video? @arribbar
I'm trying to extend your PR with support for it but I have trouble starting the Sample Project. Another question, do we actually need android/src/main/java/com/github/... and child folders and files? It looks like duplicate old code.

@kalek
Copy link

kalek commented Aug 24, 2018

@pierretanguay export chats from Whatsapp also works for you ?

@arribbar
Copy link
Contributor Author

Oh, sorry @abarisic86, I haven't seen your message before. I'll try to take a look soon and make some corrections if needed

AudreyBramy and others added 3 commits September 12, 2018 15:29
# |-------- 50 characters for the title --------| 
# If applied, this commit will...

# |----------------- 72 characters for the body ----------------------|
# Why is this change needed? How do you solve it ?
# |-------- 50 characters for the title --------| 
# If applied, this commit will...

# |----------------- 72 characters for the body ----------------------|
# Why is this change needed? How do you solve it ?
@arribbar
Copy link
Contributor Author

@abarisic86 Do you still need help on this or not? I might have some time this weekend.

@abarisic86
Copy link

abarisic86 commented Sep 20, 2018 via email

@arribbar
Copy link
Contributor Author

Ok, sorry for my late response

@AndrewHenderson
Copy link
Collaborator

@arribbar If you want this merged, please resolve conflicts. Thanks!

@arribbar
Copy link
Contributor Author

arribbar commented Nov 9, 2018

@AndrewHenderson done. Thank you :)

@isaachinman
Copy link
Collaborator

@AndrewHenderson I've been using this in a live project for several months now, but I suppose it still needs to get reviewed and put on the roadmap.

@@ -9,6 +9,171 @@
import android.content.ContentUris;
import android.os.Environment;

<<<<<<< HEAD
Copy link
Collaborator

@AndrewHenderson AndrewHenderson Nov 9, 2018

Choose a reason for hiding this comment

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

@arribbar Still seeing Git conflict commenting in this file. Please remove. Also, please check other files.

It's been awhile since you first submitted this, so please give review it from top to bottom. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh ****, I'll take a look at it this week and get back to you asap. Thank you and sorry for this error

@arribbar
Copy link
Contributor Author

Ok, I tested it a bit on Android. I made a fix to get images from whatsapp but I wonder if we shouldn't change getDataColumn function by adding a catch and use copyFile function if we are in the exception.

Link to getDataColumn : 90a6207#diff-5068d814d5a6c28612588fdf7b19debdR91

What do you think @AndrewHenderson ?

@AndrewHenderson
Copy link
Collaborator

@arribbar I'm testing this on a Samsung Galaxy S8 running Android v8.0. I'm finding that the sharing of images from Photos results in a value of null rather than a path to the image.

@arribbar
Copy link
Contributor Author

Hmm, that's super strange.
I am on a One Plus 5T on Android v8.1 and it works fine on my app. I checked it for one or several pictures as well as for videos on Photos, Gallery and Whatsapp

@arribbar
Copy link
Contributor Author

@AndrewHenderson Can you provide me an example to test it ? I could not run the provided example by this project on Android :(

@AndrewHenderson
Copy link
Collaborator

@arribbar Thanks for providing device info.

I don't have a reduced test case I can send you. I'll work on getting the current example working or write a new one.

@isaachinman and I recently began assisting with the maintenance of this repo. Please bear with us as we get things moving smoothly again.

@isaachinman
Copy link
Collaborator

@AndrewHenderson Looks like the example hasn't been updated in a year. Any idea what it'll take to get that back up and running?

@arribbar
Copy link
Contributor Author

@AndrewHenderson Can you provide me your running example ? I'll check the changes then that we need to do in order to make it work.
Thanks !

@arribbar
Copy link
Contributor Author

Ok, @AndrewHenderson. Can you try again now ? It should work better now I think

@AndrewHenderson
Copy link
Collaborator

Can you provide me an example to test it ? I could not run the provided example by this project on Android :(

@arribbar I restored the working example on the following branch: https://github.com/alinz/react-native-share-extension/tree/example/examples/simple.

It is boilerplate and runs on Android. In order to run on iOS one needs to follow the iOS Setup in order to manage code signing, etc.

If you still need the lightweight sample can use to validate the issue I mentioned, you can use that.

@isaachinman I wonder if there is a way we can have the iOS example run without all the added setup?

@isaachinman
Copy link
Collaborator

I wonder if there is a way we can have the iOS example run without all the added setup?

As far as I know, each of the steps are absolutely required. For iOS we are basically creating a "second" app entirely, so it's never going to be a quick process.

I am not an iOS dev by trade though - perhaps someone knows better than I.

@arribbar
Copy link
Contributor Author

I'll take a look at this soon
@AndrewHenderson Can you test with this last version ? I have made some improvements since you reviewed it and tell me if it is still not working for you or not :)

@chaitanyadeorukhkar
Copy link

Is this PR still active? How can I help to get this merged?

@djorkaeffalexandre
Copy link

@arribbar Do you plan fix that to merge? I want to use this library in my project on Google Summer of Code but I need support to multiple data.

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.

9 participants