Add Login Options to Dev Menu #3950
Conversation
…e login flow type or even boot config. NB: the view code was moved out of AuthFlowTester into SalesforceSDKCore.
Clang Static Analysis Issues
Generated by 🚫 Danger |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #3950 +/- ##
==========================================
+ Coverage 62.57% 63.55% +0.97%
==========================================
Files 249 252 +3
Lines 21715 22098 +383
==========================================
+ Hits 13588 14044 +456
+ Misses 8127 8054 -73
🚀 New features to boost your workflow:
|
|
|
||
| // Check if we're showing the login screen | ||
| BOOL isShowingLogin = [presentedViewController isKindOfClass:[SFLoginViewController class]] || | ||
| [presentedViewController.presentingViewController isKindOfClass:[SFSDKAuthRootController class]]; |
There was a problem hiding this comment.
Is there a cleaner way to do that?
There was a problem hiding this comment.
Could potentially call isEnabled on the authWindow here, but from a different issue I've been wanting to add an autoCreate param to that method so that it doesn't always create the window if one doesn't already exist (could be separate work)
There was a problem hiding this comment.
For now we could only support showing login options with regular login. So I would remove line 481.
For advanced auth, I'm missing the code to restart the auth flow once login options have been changed (see https://github.com/forcedotcom/SalesforceMobileSDK-iOS/pull/3950/files#diff-1334e2942ea2928b6db7bca76fdebd960b717bc0463c404244b998a86ee08121R497).
|
Also, I am not restarting the login flow so the new settings chosen in the action sheet won't be applied. I'll look at it on Monday. |
Generated by 🚫 Danger |
Also info is grouped in sections for better readability
Done with 0330aa9 |
Addressing warning (declared but not defined function)
| // TODO uncomment to support advanced auth case (once we add code to restart auth in that case below) | ||
| // || [presentedViewController.presentingViewController isKindOfClass:[SFSDKAuthRootController class]]; |
There was a problem hiding this comment.
Restarting Auth is included now, right?
There was a problem hiding this comment.
Only if it's regular auth - for now I commented out the case it's advanced auth.
|
@wmathurin Still reviewing, but isn't moving |
I don't think so. |
| }]]; | ||
|
|
||
| // Login Options - only show on login screen | ||
| if (isShowingLogin) { |
There was a problem hiding this comment.
NIT: Is there a reason to only show it on the login screen?
There was a problem hiding this comment.
It's only useful if you do a login right after - and if you do a login (let's say after a logout or when adding a new user), then you will get a chance to bring up the screen when the login screen is shown. So it feels like login is the right time to show the login options screen. The oauth settings should be inspectable through dev infos at any time though.
| } | ||
|
|
||
| // Logout - only show if there's a current user and not on login screen | ||
| if (currentUser && !isShowingLogin) { |
There was a problem hiding this comment.
I was thinking about exactly this the other day. Android doesn't take deferred auth into account, I need to also check for a user.
… of doing the actual call to the server which was flapping)
|
Last run, I was down to a single failure (one of DomainDiscoveryCoordinatorTests) - now it's back to 4. |
3 participants
Matching the Android work: forcedotcom/SalesforceMobileSDK-Android#2801
Overview
Notes