XExtensionItemSource API is Too Complicated

[AriX]

I'm a little worried that XExtensionItemSource is a little bit confusing/difficult to use for newcomers. Normal users might not be familiar with what a "placeholder item" is or why it's here.

It seems confusing that you can initialize an XExtensionItemSource with an attachments array or use the registerItemProvidingBlock: method and what the difference is between those things. And how are people supposed to understand how attributedContentText ties into those things? I worry that you have to be an expert in how UIActivityItemSource/NSExtensionItem/NSItemProvider work in order to use this API.

Compare that to UIActivityViewController, which (in the simple case) is easy as can be - literally just pass it whatever object you want. I wonder if we should be trying to offer a slightly higher-level API that abstracts away a few of these things. Particularly the placeholder item, the attachments array, and the attributedContentText/attributedTitle properties. I think that attributedTitle and subject are fundamentally the same concept and could be easily merged (since the activities that use subject never use attributedTitle)

Working on ideas for alternatives.

[irace]

For starters, what about an initializer that just looks like:

@interface XExtensionItemSource

- (instancetype)initWithItem:(id)item;

@end

Unfortunately I think this would need to be additive, rather than replacing the existing initializers, but would at least provide a low-barrier for those who just want to throw a single item in and not worry about placeholders or additional attachments. Metadata could still be appended using XExtensionItem’s existing setters.

[mbbischoff]

Love that idea.

[irace]

Additionally, as per @AriX and @prendio2 we can ditch the existing registerSubject:forActivityType: method (and close this ticket) in favor of just using the attributedTitle for the subject.

[prendio2]

How about:

@interface XExtensionItemSource

- (instancetype)initWithItems:(NSArray *)items;

@end

Which would accept an array of objects (urls, images, strings, whatever). The first item in the array is used as placeholder and all are translated into NSItemProviders and added as attachments.

[irace]

@prendio2 Would this initializer be in addition to the existing ones, or in place of?

I can see having it by the low-barrier-to-entry option, but the problem with this being the only initializer is that it’s not easy to translate an array of items into an array of item providers. A couple reasons why:

1. By always choosing the first item as the placeholder, you’re not giving the developer the ability to use the placeholder for it’s intended purpose (acting as a stand-in for a more complex object to be constructed if needed)

2. You need to initialize an NSItemProvider with a type identifier string. I don’t know of an easy way of derive these from objects, apart from hardcoding a mapping between classes e.g. NSURL and identifiers e.g. kUTTypeURL.

3. NSItemProviders can have their items loaded asynchronously (see the registerItemForTypeIdentifier:loadHandler: method). Building a layer of abstraction on top of them that does not allow for this could be pretty limiting.

[prendio2]

Right yeah, maybe a simplification too far

[AriX]

Hey all,

I just finished implementing a proposed revision of the XExtensionItemSource interface, aiming for something that is simple and easy-to-use while retaining the power that we want to provide to developers. Here is the commit to take a look at (not finalized enough yet to be a pull request).

Some notes:

• I spent some time exploring what types of content are generally shared with traditional UIActivityViewController items. It turns out that they all seem to fit into four buckets: URLs, text, images, and files. This proposed API optimizes for those use cases.
• When creating an XExtensionItemSource, you choose one of those four types of content and provide a block to generate the content. The block, when called, is provided with the activity type that the user selected, so you can implement any behavior you'd like without calling additional customization methods. Additionally, because these items can all be represented by traditional activity items, this customization works as-is for activities that do not support NSExtensionItem input.
• Attachments can be added, optionally on a per-activity type basis. Attachments are added in addition to, not in place of, the item that the XExtensionItemSource was initialized with. The user can provide attachments of type NSURL, NSString, or UIImage, in addition to NSItemProvider, and they will automatically be converted.
• The interface is still a little complex, but at least most users don't have to worry about what a placeholder item or NSItemProvider is. You can see the real-world benefits of the simpler API by looking at the ViewController.m diff in the same commit.
• I replaced the attributedTitle method with a title method of type NSString. Making the title an NSAttributedString seems inconvenient and I can't think of any reason why anyone would do so. It seems like a boneheaded decision on Apple's part. What types of attributes would ever be appropriate in an item's title? Bolding? Custom fonts? I just don't get it. What apps would ever honor such a thing? That said, if anyone has any contrary views, I'd happily restore it to NSAttributedString.
• I temporarily stripped out the inline documentation in XExtensionItem.h in order to focus on just the interface for the moment.

I'd be glad to hear any thoughts on the matter. What do you like? What do you not like? Any concerns? Did I miss anything?

[irace]

@AriX Definitely an interesting approach. I need to chew on this some more for sure but here are a few initial thoughts.

• I do worry slightly about item-providing blocks being the primary way to initialize an XExtensionItemSource. Seems like a bit of a power user feature to me.
• I can’t think of a situation offhand when you’d want to add an attachment on a per-activity type basis. Can you?
• I do worry slightly about typeIdentifierForActivityItem not being comprehensive. I wonder if we could be missing anything that otherwise needs to be able to get through. Property lists, perhaps?
• Losing the ability to pass multiple attachments to the initializer is a slight bummer, but I suppose it does make sense considering the fact that only a single one may make it through, and we were previously asking for a separate placeholder item anyway.

[AriX]

• Re: item providing blocks, totally understand that. The blocks provide a lot of power in the ability to return different items based on the activity type. And I think it's much more elegant than dealing in placeholder items or the - (void)registerItemProvidingBlock:(XExtensionItemProvidingBlock)itemBlock forActivityType:(NSString *)activityType approach. But you're right, it's a little hairy. One option is that we could add corresponding simple initializers, like -(void)initWithURL:(NSURL *)URL and move the block-based ones to the bottom or relegate them to a category, like this. I actually like that idea better.
• Re: per-activity attachments, I think there are a lot of uses. For example when the user is sharing a workflow, on Twitter we share the item @"Check out the workflow I built with @workflowhq!" with a URL to the workflow as the attachment, whereas for an email we'd share the item @"Check out the workflow I built! <a href="%@">%@" with an image attachment of the workflow's icon. Does that make sense?
• Re: typeIdentifierForActivityItem we can definitely beef that up as necessary but my investigations into how people use these APIs didn't really reveal any other classes that are used. What activity items would you pass in to map to property list, exactly? NSDictionary? Under what circumstances would an app share an NSDictionary object and how would other apps handle that (outside of the Safari special-case)? (Happy to add it in just in case, of course, just wondering if you had anything in mind.)

Keep me updated on any other thoughts and ideas you have. Happy to change this around or ditch my ideas altogether.

[irace]

One option is that we could add corresponding simple initializers, like -(void)initWithURL:(NSURL *)URL and move the block-based ones to the bottom or relegate them to a category, like this. I actually like that idea better.

👍

For example when the user is sharing a workflow, on Twitter we share the item @"Check out the workflow I built with @workflowhq!" with a URL to the workflow as the attachment, whereas for an email we'd share the item @"Check out the workflow I built! %@" with an image attachment of the workflow's icon. Does that make sense?

It does, I guess I just kind of figured that the block-based initializers would cover this use case sufficiently. But maybe not.

What activity items would you pass in to map to property list, exactly? NSDictionary? Under what circumstances would an app share an NSDictionary object and how would other apps handle that (outside of the Safari special-case)?

Honestly I was just thinking of the Safari special-case, so it's probably not worth worrying about.

[AriX]

Just pushed the corresponding changes to my repo on GitHub. Everyone, please let me know if you have thoughts or if you think it makes sense to pull any of this into the mainline repo.

Also realized that an added benefit of the way this version is implemented is that it can easily be backwards-compatible with iOS 7 (minus the extensions and attachments and metadata, of course). Not sure if that's something anyone cares about, but it could help with developer adoption, since iOS 7 is still rather widely used.

[irace]

This is looking really good; I’d like to move forward with this approach. @prendio2 @mattbischoff: if you two could have a look and weigh in as well, it’d be much appreciated.

@AriX How do you think we should go about integrating? Either of these options is fine with me, I have zero preference:

• You open a pull request: I think there’s quite a bit of work that still needs to be done on your branch – adding documentation, matching the rest of the repo stylistically, fixing/adding unit tests, updating the README – but I’m happy to do all of this in a separate PR after yours is merged, in the interest of minimizing the number of back-and-forths. The annoying part: you need to fill out and send in this Contributor License Agreement before I can merge.
• I open a pull request that incorporates your work: less work for you to do but you wouldn’t get credit for your changes.

My goal is to have a 1.0 tag applied by next Thursday, May 21 – ideally sooner, to give myself some breathing room. I’m going to be off the grid for the three weeks that follow, traveling for my honeymoon.

We likely won’t drum up too much PR around this until Tumblr.app support actually hits the App Store, so I plan to have a blog post queued up that I’ll leave in @mattbischoff’s hands.

[irace]

@AriX A few bits of specific feedback:

• I wonder if the attachments property should be called additionalAttachments, to make clear that you won’t be overwriting the item that you passed into the initializer when you set this property. I suppose documenting this could be sufficient as well.
• Not sure how important it is, but your proposal does not address this particular issue: Allow attributedContentText to be configured on a per-activity type basis #40

I’m going to close this issue (default subject) in favor of the changes proposed on your branch.

[mbbischoff]

This is looking really good; I’d like to move forward with this approach. @prendio2 @mattbischoff: if you two could have a look and weigh in as well, it’d be much appreciated.

I took a look and agree that this is a better API / approach. If there are any specific open questions left, let me know and I can help work those out, but otherwise full steam ahead.

[AriX]

Fantastic. Let me take your feedback and update my branch and create a pull request with my changes. If you'd be willing to handle the unit testing and stuff, that'd be great. Happy to sign that waiver.

[AriX]

See #46!

[prendio2]

This looks good to me. We've already submitted Unread 1.5.1 with the current API but would happily move to this new approach. One thought on the per-activity attributedContentText is it might be nice to return it in a block instead of registering them all in advance… It's unlikely to effect performance much but I like the idea of not having to generate the various strings in advance.