AcknowList icon indicating copy to clipboard operation
AcknowList copied to clipboard

UIApplication avoidance

Open KieranHarper opened this issue 5 years ago • 9 comments

Added an optional closure parameter that can be invoked when the user wants to open the cocoapods URL, rather than using UIApplication.

This is needed in situations where the view controller lives in a framework target that doesn't allow UIApplication to be used.

KieranHarper avatar Oct 20 '18 08:10 KieranHarper

Hello Kieran, and thanks for the pull request.

I appreciate that not relying on UIApplication in those situations would be valuable, but that shouldn’t come at the expense of the most-used configuration. Unfortunately, I don’t think there’s a good solution for that, because you can’t really test at build time if the target supports UIApplication. Maybe add a preprocessor check (#if etc)?

If somebody has a better solution, please let us know.

vtourraine avatar Oct 25 '18 07:10 vtourraine

~~In Objective-C, you can use NS_EXTENSION_UNAVAILABLE macro, but in Swift, there isn't really an replacement for that.~~

So what I did is this:

https://github.com/LiulietLee/LLDialog/blob/master/Source/LLDialog.swift#L69-L73

Since https://github.com/apple/swift/pull/12410 got merged, you can just use @available to check if it's app extension.

~~I don't think there's enough sample to prove this implementation is App Store compatible, but I discussed it with one of the Apple engineers during WWDC17. He said it "should" be fine.~~

ApolloZhu avatar Oct 25 '18 07:10 ApolloZhu

That’s super interesting. Thank you very much for pointing that out, @ApolloZhu!

I guess we should update this pull request to replace the closure with the appropriate @available checks, then I‘ll be happy to merge it in.

vtourraine avatar Oct 25 '18 21:10 vtourraine

So I’ve tried adding the @available checks on openCocoaPodsWebsite(), as pointed out by @ApolloZhu. It works, but unfortunately that only moves the problem. Now the compilation error is on the UITapGestureRecognizer instantiation, because the selector is not available for Extensions:

'openCocoaPodsWebsite' is unavailable: This method is NS_EXTENSION_UNAVAILABLE.

It seems like this should be easily solvable by testing the availability when creating the gesture recognizer, but as fas as I understand, this type of condition does not exist. Something like if #available(iOSApplicationExtension 1.0, *) {...} wouldn’t help, because the * is mandatory, so we can’t limit just for Extensions.

To reiterate: I want to keep the current behavior in a non-Extension context, and I don’t want to require more code from the app using this library.

Am I missing something, or do we need to wait for more flexible compiler instructions?

vtourraine avatar Nov 30 '18 09:11 vtourraine

FYI in XCode 13b3 this started causing issues, as the SwiftPM packages are being built with -application-extension flag, so now any app that imports AcknowList package fails to build.

Looking at the discussion in Swift Forums, right now the only solution seems to be annotating either the class or some part of it with @available(iOSApplicationExtension, unavailable).

I see two options here:

  • We can create two different VC from the current one, one for main apps (with the unavailable annotation) and the other for extensions
  • Use this PR as a base and add some additional constructors, mark those with the unavailable annotation, and only those would have an urlOpener.

@vtourraine Are you open to receiving any PR for this or you'd rather wait and see if there's any fixes in this area on further betas?

ifrins avatar Jul 31 '21 09:07 ifrins

@ifrins Hello Francesc, and thanks a lot for raising this issue!

It looks like we should do something about it, but maybe there’s an easier solution. I think we can add the @available() statement to the whole AcknowListViewController class, which would silence the compilation error. I’ve never used this library from an app Extension, but maybe I’m missing other use-cases?

@available(iOS 9.0.0, tvOS 9.0.0, *)
@available(iOSApplicationExtension, unavailable)
open class AcknowListViewController: UITableViewController {

vtourraine avatar Aug 02 '21 08:08 vtourraine

What you suggested works (I created a fork doing exactly that to unblock myself 😊); but it would break if anyone is using this VC in an extension (not really sure if that's a real use case though).

ifrins avatar Aug 02 '21 09:08 ifrins

Right, so we’re on the same page. I think you should create a pull request for this. We’ll merge it to main, then see if that turns out problematic for some apps. I’d really prefer to avoid adding alternative implementations/methods just to dance around this issue.

vtourraine avatar Aug 02 '21 09:08 vtourraine

@ifrins Well... another pull request was already submitted with the @available annotation (#85), so I’ve merged it. I’ve also mentioned you in the changelog as a co-author, I hope that’s OK.

Everyone can now test the Xcode 13 support with the xcode-13 branch. And I’ve opened a dedicated pull request (#86) to discuss any further changes.

vtourraine avatar Aug 04 '21 08:08 vtourraine