-
Notifications
You must be signed in to change notification settings - Fork 168
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
Authentication: Addressing Debt #1202
Authentication: Addressing Debt #1202
Conversation
@@ -52,6 +52,5 @@ | |||
- (void)presentPasswordResetAlert; | |||
- (void)presentPasswordCompromisedAlert; | |||
- (void)presentUnverifiedEmailAlert; | |||
- (void)showAuthenticationErrorForCode:(NSInteger)responseCode responseString:(NSString *)responseString; |
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 was triggering a warning. I've missed dropping the forward declaration, when ported to Swift
} | ||
|
||
authWindowController.switchToMagicLinkRequestedUI(email: email) | ||
} |
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.
Not in use any longer!
case passwordField: | ||
state.password = passwordField.stringValue | ||
state.password = passwordField.stringValue.trimmingCharacters(in: .whitespacesAndNewlines) |
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.
Legacy usernameText
and passwordText
had this trimming (L97 + L102 in this PR)
@IBAction | ||
func switchToPasswordAuth(_ sender: Any) { | ||
mode = AuthenticationMode.loginWithPassword | ||
} |
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 one wasn't in use
} | ||
|
||
var authWindowController: AuthWindowController? { | ||
view.window?.windowController as? AuthWindowController | ||
} |
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.
👋 too!!
TY!! 🔥 |
Fix
In this PR we're addressing pending tech debt, in the AuthenticationViewController class.
Test
Please smoke test Auth with Password + Code, and verify everything looks great!!
Release