Use a different green spinner during onboarding #5623
Conversation
…or for new users. Also moves key verification out of a bridge presenter in AuthenticationVC and into the AuthenticationCoordinator.
| - (void)authenticationViewController:(AuthenticationViewController *)authenticationViewController | ||
| didLoginWithSession:(MXSession *)session | ||
| andPassword:(NSString *)password; |
There was a problem hiding this comment.
Looks like I should indicate the parameter nullability for Swift.
| override func viewWillAppear(_ animated: Bool) { | ||
| navigationController?.setNavigationBarHidden(true, animated: false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Was going to use ElementViewController but that fails to load as it can't find the view controller in the main storyboard.
|
📱 Scan the QR code below to install the build for this PR. If you can't scan the QR code you can install the build via this link: https://i.diawi.com/4CLv5b |
| } | ||
|
|
||
| private func authenticationDidComplete() { | ||
| completion?(.didComplete(authenticationViewController.authType)) |
There was a problem hiding this comment.
I see this is called from a lot of places, even on failures. Is that expected?
There was a problem hiding this comment.
Yes in as much as this is how AuthenticationViewController previously worked (calling dismiss).
The calls from sessionStateDidChange are all once the session is running and the failures are only crypto/cross-signing related so should all be handled from the home tab once the onboarding has completed. There is probably scope for improvement once the auth flow is re-written though.
| import UIKit | ||
| import Reusable | ||
|
|
||
| class LaunchLoadingViewController: UIViewController, Reusable { |
There was a problem hiding this comment.
I always liked the "SplashScreen" name 😁
There was a problem hiding this comment.
Too many options! I followed LaunchLoadingView as a convention here, but there are designs to update the loading view at some stage, so maybe at that point we can switch it 💦
This will allow other screens to be presented during onboarding without having the spinner thrown on top of them. Additionally moves key verification out of a bridge presenter in AuthenticationVC and into the AuthenticationCoordinator.
I may end up moving both the spinner and the key verification the
OnboardingCoordinatorinstead of theAuthenticationCoordinator, but this seems like a decent sized PR for now and fixes a couple of potential issues with the spinner presentation that would be great to get into the next release.Fixes #5621.