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

[Feature] Android Sharesheet #2968

Merged
merged 4 commits into from Oct 6, 2020
Merged

[Feature] Android Sharesheet #2968

merged 4 commits into from Oct 6, 2020

Conversation

abelgardep
Copy link
Contributor

@abelgardep abelgardep commented Sep 16, 2020

Implements #2902 partially.

We cannot exclude oC from sharing targets till API 24, so we'll keep the previous behaviour for API <24, and use sharesheet for newer ones.

Old code will be removed when we upgrade min_sdk_version to api 24.

QA

Test plan: https://github.com/owncloud/QA/blob/master/Mobile/Android/Release_2.16/2922-new_share_sheet.md

Bugs & improvements:

val excludeLists = ArrayList<ComponentName>()
if (resInfo.isNotEmpty()) {
for (info in resInfo) {
val activityInfo = info.activityInfo
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this block can be put into a function:
getExcludeListForInfo(info: ResolveInfo, packagesToExcloude: Array<String>) : ArrayList<ComponentName

Copy link
Contributor

@theScrabi theScrabi left a comment

Choose a reason for hiding this comment

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

LFTM

@jesmrec
Copy link
Collaborator

jesmrec commented Oct 6, 2020

(1) [FIXED]

I've tested how the UI looks like with different versions. With Android 7 (API 24, it takes the new approach), no title is shown in the menu, so it seems not too clear. However, other newer versions (Android 9, Android 11), show a title :

Android 11 Android 7
Screenshot 2020-10-06 at 12 32 20 Screenshot 2020-10-06 at 12 26 54

is this something that can be fixed? or is it a system feature?

@abelgardep
Copy link
Contributor Author

Yes, you are right, i have added a title according to https://developer.android.com/training/sharing/send#adding-rich-content-previews

@jesmrec
Copy link
Collaborator

jesmrec commented Oct 6, 2020

(2) [FIXED]

Just to keep compatibility and same wording as previous version, when the link is shared the proper string is Send link to (activity_chooser_title) instead of share with. What do you think?

@jesmrec
Copy link
Collaborator

jesmrec commented Oct 6, 2020

(3) [DONE]

  1. Select a file
  2. Select the option Send

Current: In the menu, we can see Share (Android11), or Share with (Android 7).
Expected: Send

is this feasible?

@abelgardep
Copy link
Contributor Author

(2)

Just to keep compatibility and same wording as previous version, when the link is shared the proper string is Send link to (activity_chooser_title) instead of share with. What do you think?

Ok, updated.

(3)

  1. Select a file
  2. Select the option Send

Current: In the menu, we can see Share (Android11), or Share with (Android 7).
Expected: Send

is this feasible?

Im not sure why it changes 🤔

@jesmrec
Copy link
Collaborator

jesmrec commented Oct 6, 2020

(3) affected only Android 7, since newer versions use their own wording "Share"

PR Approved on my side, everything fixed

Thanks also @ddopik for your contribution!!

@abelgardep abelgardep merged commit dd561fc into master Oct 6, 2020
@abelgardep abelgardep deleted the feature/share_sheet branch October 6, 2020 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace custom share dialog with Android Sharesheet
3 participants