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

feat(share_plus): support rich preview on the share sheet #3372

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

slaci
Copy link

@slaci slaci commented Nov 27, 2024

Description

Support rich preview on Android with the help of the new title and thumbnail parameters. The title is also used for the Web Share API's title parameter in favor of the subject, but falls back to the subject for backward compatibility (as the previous code used the subject for this).

The contribution guide says features for only one platform won't be accepted, but I still want to try.

Related Issues

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
  • All existing and new tests are passing.
  • The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
  • No, this is not a breaking change.

@miquelbeltran
Copy link
Member

Hi @slaci thanks for the contribution!

I am not convinced about adding another two parameters to the method, specially title, which added to text and subject just becomes more confusing what does what on which platform.

The share method definitely needs a refactor and a parameter cleanup in some way. Maybe this PR is an opportunity to do that?

There is some discussion about that in #3307

The contribution guide says features for only one platform won't be accepted, but I still want to try.

That's true, maybe there is a way to provide a preview thumbnail on iOS as well?

@vbuberen
Copy link
Collaborator

I like the idea of adding this functionality as it is something that Android offers by default. But agree with Miguel that we might think about the API more here.

@slaci
Copy link
Author

slaci commented Nov 28, 2024

I can extract the thumbnail to a SharePlusAndroidSettings class or something. The sharePositionOrigin could go into an IOS or Apple one, but that would be a breaking change (or the parameter could be deprecated first of course).

The rest of the parameters are problematic, as the new title param is not the odd one, but the subject. Android uses the title for the EXTRA_TITLE parameter and the web uses it as the title parameter of the Web Share API. For iOS I may need some more research indeed, but as I see that could also use the title. There is the thumbnailImageForActivityType method which currently returns the image when an image is shared. It may be worth to check how this works (what happens if I pass a thumbnail to it when sharing text). When an NSURL is shared, then the activityViewControllerLinkMetadata callback runs and creates an LPLinkMetadata which contains a title + thumbnail, but this is sadly only for urls (as far as I understand).

@slaci
Copy link
Author

slaci commented Dec 2, 2024

I have spent some hours checking the iOS thumbnail possibilities, but sadly I couldn't figure out when the mentioned thumbnailImageForActivityType runs.

There are multiple reports about this, but sadly without any answers: SO | Apple Forum

From the docs it seems like it depends on the target app (though not 💯 ), like the subject (eg. the subjectForActivityType runs for the Mail app), but I couldn't find any app that would trigger the thumbnail callback or maybe it is totally bugged on iOS 18.

I haven't played with the title too much yet, but currently the subject is used for the LPLinkMetadata's title when it is available which is incosistent with my changes. However it is only used for shareXFiles (because files are provided as NSURL-es, so they have this metadata) where I don't pass the title as providing a title when sharing files didn't do anything on Android nor web (will need to check what happens on iOS if anything in this case).

So if I could make the thumbnail work on iOS, then it could stay as a common option, otherwise I guess I would need to extract it to a Platform specific option class.

@miquelbeltran
Copy link
Member

miquelbeltran commented Dec 2, 2024

Thanks for looking into it @slaci !

Re: params, I'm thinking that just having a single ShareParams class with all options may be the easiest, unify it all into a single share(ShareParams) method, and that should go all the way down. That will allow us to use any of the provided optional params no matter if sharing a uri, text, or files. We may need to add some asserts to enforce providing certain combination of params, but that would be it.

class ShareParams {
  final String? text;

  /// Used as share sheet title where supported (e.g. EXTRA_TITLE on Android)
  final String? title;

  /// Only used as email subject where supported (e.g. EXTRA_SUBJECT on Android)
  final String? subject;

  /// Only used in Android as preview thumbnail
  final XFile? previewThumbnail;

  /// Only used in iPad and Mac as share sheet origin
  final Rect? sharePositionOrigin;

  /// Share a URI, alternative to sharing [text]
  final Uri? uri;

  /// Share multiple files, can be used in combination with [text]
  final List<XFile>? files;
  final List<String>? fileNameOverrides;

  ShareParams({
    this.text,
    this.subject,
    this.extraTitle,
    this.previewThumbnail,
    this.sharePositionOrigin,
    this.uri,
    this.files,
    this.fileNameOverrides,
  });
}

Since we would be at it, I'd like to unify all methods into one, and to make Share class instantiable, instead of providing a static method (some people have asked this to be able to mock it)

@miquelbeltran
Copy link
Member

Added a comment on #3307 (comment) your feedback would be 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.

[Request]: (share_plus) EXTRA_TITLE in share Intent on Android [share_plus] [Request]: Thumbnail support
3 participants