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 multiple attachments #32

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

Conversation

markdaws
Copy link

This change adds support for multiple attachments, allowing callers
to create a share extensions that can select more than one item to
share.

This change adds a dataMulti() function to the share extension, which
instead of returning a single item returns an array of items, the original
data() function is unchanged, so this change is backwards compatible.

The change only adds this support to iOS, not Android.

This change adds support for multiple attachments, allowing callers
to create share extensions that can select more than one item to
share.
@markdaws
Copy link
Author

I should have put this is a separate branch, this change only pertains to the first commit, not the second, I can clean up if you think you want to accept this.

@npomfret
Copy link
Contributor

It looks like some similar work has been done on the java side: https://github.com/alinz/react-native-share-extension/pull/34/files

It would be nice to get the behaviour the same on both.

This is a breaking change. The current version of the code doesn't
handle public.image correctly in some cases. It assumes that the
value returned from getItemForTypeIdentifier is always a NSURL*.
This is true for things like Photos, but the app can define what
it passes back, while testing, I found that it can also be a
UIImage* instance or even NSData*, for example in Notes if you
create a note, add a drawing and insert a photo, the drawing will
be found as a public.image but it will be in NSData format, not
NSURL.

In the cases where we get UIImage* or NSData* in order to share them
later on we have to have a URL to them on disk, so I used app groups
to create a temporary container location to store the files. Then the
caller can reference these URLs in the JS code, and finally when we
close the share extension we clean up all the temp files to make sure
we don't keep growing the size of our disk footprint.
@BigPun86
Copy link

BigPun86 commented Apr 6, 2018

@markdaws i tried your markdaws/react-native-share-extension@6dee20e which worked but there still is one problem i have maybe u can help me on that. I could share an image and open up my Host Application, but unfortunately the image i shared was not available anymore when the ShareExtension was closed. Any ideas what i am missing? I am using AppGroups and my Image path looks like this: file:///var/mobile/Media/PhotoData/OutgoingTemp/0C37E764-DB6A-4A3D-9BCD-174E4BDFB1FA/IMG_0616.JPG

Copy link
Collaborator

@AndrewHenderson AndrewHenderson left a comment

Choose a reason for hiding this comment

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

@markdaws This looks like a decent amount of Objective-C additions. Please resolve any conflicts and review code.

Merging this will depend on matching the behavior on Android. Please coordinate with @cbjs on his PR: #34

Note: @cbjs also created a clear method which we may or may not need. Please discuss with him.

"main": "lib/index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
"_args": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the modifications to the package.json? I assume this is a mistake.

data: () => NativeModules.ReactNativeShareExtension.data(),
close: () => NativeModules.ReactNativeShareExtension.close()
data: (appGroupId) => NativeModules.ReactNativeShareExtension.data(appGroupId),
dataMulti: (appGroupId) => NativeModules.ReactNativeShareExtension.dataMulti(appGroupId),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need a separate method or can it be supported in the existing data method.

@AndrewHenderson
Copy link
Collaborator

Please take a look at #84 as well. Since this PR supports both platforms, it’s more likely we’ll merge that one.

You’ve got a good understanding of the issue, so a review and collaboration on that PR would be greatly appreciated.

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.

4 participants