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

[Native Share] Implement Native Share for IOS #48788

Open
grgia opened this issue Sep 9, 2024 · 67 comments
Open

[Native Share] Implement Native Share for IOS #48788

grgia opened this issue Sep 9, 2024 · 67 comments
Assignees
Labels

Comments

@grgia
Copy link
Contributor

grgia commented Sep 9, 2024

Description

This is a tracking issue for the IOS implementation of the Native Share project. This section of the project will be handled separately but merged alongside the Android issue.

Project

https://docs.google.com/document/d/14C1VifxvIXeyLAf0XstbC-wvyAb5SQZ0AOw0-jLRyZs/edit

Tracking

#36613

@BrtqKr
Copy link
Contributor

BrtqKr commented Sep 9, 2024

Hey, I'm from SWM, I'd like to take care of this issue.

Copy link

melvin-bot bot commented Sep 12, 2024

@grgia, @BrtqKr Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Sep 16, 2024

@grgia, @BrtqKr 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Copy link

melvin-bot bot commented Sep 18, 2024

@grgia, @BrtqKr 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

Copy link

melvin-bot bot commented Sep 20, 2024

@grgia, @BrtqKr 10 days overdue. Is anyone even seeing these? Hello?

@grgia
Copy link
Contributor Author

grgia commented Sep 23, 2024

Bumped for update / steps for unblocking permissions

@BrtqKr
Copy link
Contributor

BrtqKr commented Sep 23, 2024

I am focusing on split navigation atm. I'll try to check the setup whenever it's applied and try to continue whenever the navigation is ready.

@melvin-bot melvin-bot bot removed the Overdue label Sep 23, 2024
Copy link

melvin-bot bot commented Sep 26, 2024

@grgia, @BrtqKr Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 26, 2024
@BrtqKr
Copy link
Contributor

BrtqKr commented Sep 30, 2024

Taking care of this atm. I've checked the groups before the weekend and they seemed to work as expected.

@melvin-bot melvin-bot bot removed the Overdue label Sep 30, 2024
@BrtqKr
Copy link
Contributor

BrtqKr commented Sep 30, 2024

Working on the files processing - they are being saved correctly from the share extenbsion, but there's something wrong with retrieving them on the react native side. This might be something related to the permissions, but I'll try to confirm it tomorrow.

@grgia
Copy link
Contributor Author

grgia commented Oct 1, 2024

Thanks @BrtqKr- do you have a draft PR up for your branch?

@BrtqKr
Copy link
Contributor

BrtqKr commented Oct 1, 2024

Not yet, wired it up though and the files are being passed to the react native wihthout any issues, so happily there's no need for the additional set-ups. I'll be verifying the scanning/sending with the passed files tomorrow, so this would probably be the initial version for the ios. I'll share more details next evening.

@BrtqKr
Copy link
Contributor

BrtqKr commented Oct 2, 2024

Moving forward with that - wired up most of the js parts - I'll continue working on that and the files cleanup after sharing tomorrow

@grgia
Copy link
Contributor Author

grgia commented Oct 3, 2024

Sounds great, thanks @BrtqKr

@BrtqKr
Copy link
Contributor

BrtqKr commented Oct 3, 2024

Still working on the JS part - today it was mostly persistence and onyx-related things, which are necessary for removing files - it's not done yet though

@BrtqKr
Copy link
Contributor

BrtqKr commented Oct 4, 2024

Continued the topic from yesterday. I've found some edge cases about the cleanup part - there's an issue when the share attempt gets cancelled and the file isn't removed properly, but we can handle this by performing cleanups for the files older than a week. Besides I had to slightly modify the model for the image, but apply small updates to the doc when I'm sure everything works.

@grgia
Copy link
Contributor Author

grgia commented Oct 7, 2024

Great @BrtqKr, do you have an idea for when we might be able to get this into the review stage?

@melvin-bot melvin-bot bot added the Overdue label Oct 7, 2024
@BrtqKr
Copy link
Contributor

BrtqKr commented Oct 8, 2024

@grgia, I've finished wiring up the core version for the ios today. Also, @filip-solecki mentioned that there wasn't much left regarding the Android native part, so I'd say that the remaining things before the PR would be:

  1. Adjust the data format on Android to match the ios (I had to slightly change the format to handle some cases on the react-native side)
  2. Finish the file removal
  3. Adjust the display for the files
  4. Pass to @staszekscp and @war-in to apply the hybrid app-related changes and test it properly

The first three points shouldn't take much time since those are just the follow-ups to the already existing parts (the second point is a bit troublesome to test though). I'm not sure about the fourth point. As long as everything goes as expected on the hybrid part we should be able to send the final version sometime at the beginning the next week, but this is something @staszekscp or @war-in would need to estimate when they start applying the changes.

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-08.at.02.19.43.mp4

@melvin-bot melvin-bot bot removed the Overdue label Oct 8, 2024
@filip-solecki
Copy link
Contributor

Hi! Quick update - I took the issue from @BrtqKr and I'll work on both right now as JS part will be shared for both Android and iOS

@grgia
Copy link
Contributor Author

grgia commented Nov 29, 2024

Hey @filip-solecki could we go back to daily updates?

@melvin-bot melvin-bot bot removed the Overdue label Nov 29, 2024
@filip-solecki
Copy link
Contributor

Hi @grgia ! Of course, So today @Guccio163 was working on displaying AttachmentModal when clicking on thumbnail in Share flow, while I was fixing PDF uploading and investigating possible solutions for sharing aspectRatio for videos from native module, we are really close to prepare Draft PR and allow you test everything

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2024
@grgia
Copy link
Contributor Author

grgia commented Dec 2, 2024

@filip-solecki sounds great, how's the AttachmentModal going today?

@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2024
@Guccio163
Copy link
Contributor

Hi, implementing AttachmentModal logic is going smoothly; I've stumbled upon a problem with handling PDF thumbnail on android and @filip-solecki still have to figure out a couple of native issues, but it looks like it's on a good way to PR soon 👌

@melvin-bot melvin-bot bot added the Overdue label Dec 5, 2024
@grgia
Copy link
Contributor Author

grgia commented Dec 5, 2024

@Guccio163 how's that going? Can we get a PR up this week or next?

@melvin-bot melvin-bot bot removed the Overdue label Dec 5, 2024
@filip-solecki
Copy link
Contributor

Hi! @Guccio163 is working on fixes suggested by design team and I am working on Submit flow for not existing user and these are our latest bigger issues, it is hard to say the exact day, we hope we will be ready tomorrow to prepare draft PR, but it may be also early next week

@filip-solecki
Copy link
Contributor

In the meantime I got access to Hybrid app today and I'll try to connect the new Share flow there

@filip-solecki
Copy link
Contributor

filip-solecki commented Dec 6, 2024

Hi!
Today's update is that we didn't make the PR draft, we're still working on Submit flow to an unknown user, and I've been looking at how we can merge our new sharing flow with the old one in the hybrid app.

@melvin-bot melvin-bot bot added the Overdue label Dec 9, 2024
@grgia
Copy link
Contributor Author

grgia commented Dec 9, 2024

@filip-solecki how's the hybrid app flow going?

@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@filip-solecki
Copy link
Contributor

Hi! I am working on this, I have to merge modules for processing files cause they are a little bit different as we support other types than only images right now in ND and then I'll have to find the best place to redirect users to ND if they are using it or to stay in OD and process files here. Additionally I have one concern about supported media types. In OD we supported only images and it was easy job. Right now we have a few more media types that we can share to chats in ND. Android allows us to specify exact type of data we want allow user to share, but iOS groups it in specified groups like: images, videos, text, files and attachments. So my questions are:

  1. What should we do if someone is not using ND and tries to share video. In OD we don't support it, should we redirect user to ND despite all or display toast message in OD that this type of file is not supported?
  2. We are also not able to block e.g. CSV files sharing (AFAIK this type was not in scope) on iOS as it is part of NSExtensionActivationSupportsFileWithMaxCount group. Should we then process it where it is possible? Or block and display some generic message that this type is not supported? (Allowing users to share shouldn't require any additional work)

@grgia
Copy link
Contributor Author

grgia commented Dec 11, 2024

What should we do if someone is not using ND and tries to share video. In OD we don't support it, should we redirect user to ND despite all or display toast message in OD that this type of file is not supported?

@filip-solecki by this do you mean that they don't have the ND app?

We are also not able to block e.g. CSV files sharing (AFAIK this type was not in scope) on iOS as it is part of NSExtensionActivationSupportsFileWithMaxCount group. Should we then process it where it is possible? Or block and display some generic message that this type is not supported? (Allowing users to share shouldn't require any additional work)
AFAIK this type was not in scope

given it's not in scope, we can likely just block CSV file sharing and show users a message that such files are not supported

@JmillsExpensify could you confirm this

@filip-solecki
Copy link
Contributor

@filip-solecki by this do you mean that they don't have the ND app?

I mean they have Hybrid App installed, but never used ND, only working on OD

@filip-solecki
Copy link
Contributor

Hi! Quick update:
I am still working on merging methods for files processing as it is much more complicated than I thought before seeing OD method. And @Guccio163 finished navigation issues and will be working on adding message for users while sharing unsupported media type

@melvin-bot melvin-bot bot added the Overdue label Dec 16, 2024
Copy link

melvin-bot bot commented Dec 17, 2024

@grgia, @BrtqKr, @cdOut, @filip-solecki Whoops! This issue is 2 days overdue. Let's get this updated quick!

@grgia
Copy link
Contributor Author

grgia commented Dec 18, 2024

@filip-solecki do you have a rough estimate on ETA?

@filip-solecki
Copy link
Contributor

Here is the draft PR for both iOS and Android! We are still working on some really minor polishes, but it would be good if you can test it in your free time and send us some feedback. Today we also want to send it to our internal review 🚀

@melvin-bot melvin-bot bot added the Overdue label Dec 23, 2024
Copy link

melvin-bot bot commented Dec 23, 2024

@grgia, @BrtqKr, @cdOut, @filip-solecki Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Dec 25, 2024

@grgia, @BrtqKr, @cdOut, @filip-solecki Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Dec 27, 2024

@grgia, @BrtqKr, @cdOut, @filip-solecki Still overdue 6 days?! Let's take care of this!

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

No branches or pull requests

5 participants