-
Notifications
You must be signed in to change notification settings - Fork 232
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
Enable Credentials Manager for tvOS and Mac Platforms #206
Conversation
Enable BioMetrics for iOS Platform Only Remove restriction in CocoaPod Spec Added tvOS, Mac Apps for Hosted Testing (Required for Keychain)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if instead of adding #
checks everywhere in the existing code, you just do that once at the instantiation of the CredentialsManager
and instead, create a TvCredentialsManager
or similar which would override the original's methods. Those methods not being override can still call super, so no need to re-write everything and code would remain much more clearer IMO.
BioMetrics only available for iOS Platform Only
e.g. A no-op TvBiometrics
that could still be called.
How have you tested this? Is there a GIF or video you can post?
@@ -16,11 +16,30 @@ web_auth_files = [ | |||
'Auth0/SilentSafariViewController.swift', | |||
'Auth0/NativeAuth.swift', | |||
'Auth0/AuthProvider.swift', | |||
'Auth0/CredentialsManager.swift', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this are referenced in s.osx.exclude_files
. Why did you remove them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enabling it for tvOS also allows it for Mac
'Auth0/BioAuthentication.swift' | ||
] | ||
|
||
watchos_exclude_files = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks like web_auth_files
plus 2 entries. Is there a nicer way to define this array? i.e. joining the already defined base array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly not, i've tried joining arrays etc it will just throw out an error in cocoapods when linting the lib.
} | ||
|
||
func applicationWillResignActive(_ application: UIApplication) { | ||
// Sent when the application is about to move from active to inactive state. This can occur for certain types of temporary interruptions (such as an incoming phone call or SMS message) or when the user quits the application and it begins the transition to the background state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(such as an incoming phone call or SMS message)
how can this happen on a TV?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shrug this is default boiler plate apple app creation.
|
||
import UIKit | ||
|
||
class ViewController: UIViewController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the point on keeping this class if it only overrides 2 methods that call the super? as opposed to OAuth2Mac/ViewController.swift
which does change one of the methods logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again this is Apple's boilerplate application creation.
@lbalmaceda these are pre-processor macros, not run-time. There is no need to create a pointless no-op method, in the above the method will not be visible in any way when building a tvOS app. From the perspective of the developer they only see the methods available to that platform. I've tested locally for tvOS, fastlane doesn't support tvOS so I can't easily add to the current CI setup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then GLMT ⚔️
BioMetrics only available for iOS Platform Only
Remove restriction in CocoaPod Spec
Added tvOS, Mac Apps for Hosted Testing (Required for Keychain)