Enterprise support with db #340
Conversation
Added SSO Header
EnterpriseInteractor embedded in the now multi role Database Presenter Created InfoBar Component for SSO Message Retired Enterprise Route, now handled in DatabaseView Seamless keyboard behaviour between enterprise mode switch
More swift in places View spacing
|
|
||
|
|
||
| func presentEnterprise() { | ||
| if self.infoBar != nil { return } |
There was a problem hiding this comment.
Why is the reason we check this?
| func presentEnterprise() { | ||
| if self.infoBar != nil { return } | ||
|
|
||
| let form = self.form as! CredentialView |
There was a problem hiding this comment.
Let's avoid the force unwrap, also if there is a chance this might not hold use a guard and report the error
|
|
||
| let form = self.form as! CredentialView | ||
| let infoBar = InfoBarView() | ||
| let viewCount = self.container!.subviews.count |
There was a problem hiding this comment.
Let's avoid this force-unwrap too
|
|
||
| func removeEnterprise() { | ||
| guard let infoBar = self.infoBar, spacer = self.spacer else { return } | ||
| let form = self.form as! CredentialView |
There was a problem hiding this comment.
Let's avoid this force-unwrap too
| var initialEmail: String? { return self.authenticator.validEmail ? self.authenticator.email : nil } | ||
| var initialUsername: String? { return self.authenticator.validUsername ? self.authenticator.username : nil } | ||
|
|
||
|
|
||
| weak var databaseView:DatabaseOnlyView? |
| guard let attr = attribute else { return } | ||
| do { | ||
| try self.authenticator.update(attr, value: input.text) | ||
| // Check for Entperise domain match in login view | ||
| if var enterpriseInteractor = self.enterpriseInteractor, let mode = self.databaseView?.switcher?.selected { |
There was a problem hiding this comment.
it's mutating. However, this section can be refined a bit anyway so will do that.
| guard let attr = attribute else { return } | ||
| do { | ||
| try self.authenticator.update(attr, value: input.text) | ||
| // Check for Entperise domain match in login view | ||
| if var enterpriseInteractor = self.enterpriseInteractor, let mode = self.databaseView?.switcher?.selected { | ||
| if enterpriseInteractor.isEnterprise(input.text) && mode == .Login { |
There was a problem hiding this comment.
I think there is no need for the nested if
| mutating func isEnterprise(value: String?) -> Bool { | ||
| do { | ||
| try updateEmail(value) | ||
| if self.connection != nil { return true } |
There was a problem hiding this comment.
just return , there is no need for the if
There was a problem hiding this comment.
A valid email does not always mean a valid connection, hence the check.
| @@ -41,15 +41,22 @@ class EnterpriseDomainPresenter: Presentable, Loggable { | |||
| let authCollectionView = self.authPresenter?.newViewToEmbed(withInsets: UIEdgeInsetsMake(0, 0, 0, 0), isLogin: true) | |||
| let view = EnterpriseDomainView(email: email, authCollectionView: authCollectionView) | |||
| let form = view.form | |||
|
|
|||
| if self.interactor.connection != nil { | |||
| view.infoBar?.hidden = false | |||
There was a problem hiding this comment.
Can't we just
view.infoBar?.hidden = self.interactor.connection != nilThere was a problem hiding this comment.
it will make Ray sad, but yes we can :)
| } | ||
|
|
||
| public override func intrinsicContentSize() -> CGSize { | ||
| return CGSize(width: 200, height: 35) |
There was a problem hiding this comment.
Set the with as non determined otherwise you'll have a hard time making it grow. The only fix is the height like we have now with 35 pts
Feedback changes EnterpriseDomainInteractor refactored Tests updated
More swift in places
View spacing
Tests