-
Notifications
You must be signed in to change notification settings - Fork 11
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
UL&S: SiteCredentialsViewController username and password fields #329
Conversation
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.
Hey @mindgraffiti ! Just had one small thing in the code, thanks!!
override func viewWillAppear(_ animated: Bool) { | ||
super.viewWillAppear(animated) | ||
|
||
configureSubmitButton(animating: false) |
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 call already exists in viewDidLoad()
, so maybe it should be removed from here?
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.
Removed them both for now. This PR was meant to focus on the username and password textfields. I shouldn't have put in the configuration method for the Continue button.
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.
Just a couple code nitpicks. Otherwise, !
if showSecureTextEntryToggle == false { | ||
return | ||
} |
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.
Just a suggestion - maybe this could be a guard
?
guard showSecureTextEntryToggle else {
return
}
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.
Thanks! I converted the Obj-C class WPWalkthroughTextField
into Swift and forgot to make the code actually Swift-y 😀
func setSecureTextEntry(_ secureTextEntry: Bool) { | ||
// This is a fix for a bug where the text field reverts to a system | ||
// serif font if you disable secure text entry while it contains text. | ||
textField.font = nil |
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.
Is setting it to nil
necessary since it's immediately followed by setting the font? I commented it out and didn't see a difference.
This PR is meant for the username and password fields
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.
!
Thank you for the reviews Giorgio and Stephenie! |
Ref. #283
This PR adds the password type to the
TextfieldTableViewCell
and adds the username and password textfields toSiteCredentialsViewController
. The "Continue" button is not yet implemented because this PR was getting kinda big.Test PR: wordpress-mobile/WordPress-iOS#14492
bundle exec pod install