Conversation
gbrown3
left a comment
gbrown3
left a comment
There was a problem hiding this comment.
Great work here, the login page looks a million times better. There are a few more issues we need to address before we can merge this, most of them functional.
-
The Google signin button is a little messed up and I think it mostly has to do with the way I initially implemented it. The two main issues are that a prompt to sign in appears as soon as I open the page (without tapping anything) and then tapping the google sign in button doesn't do anything. I can't remember all the ins and outs of how to make it work, but this tutorial should serve you well.
-
We need to handle errors, both from firebase login and google sign in. For example, if the user puts in an email that isn't in a valid format, there should be a popup message that tells them that. I think most if not all of these warnings can just be a simple popup message (I think they're called Alerts or UIAlerts in iOS).
-
Just as good practice, I think we should have some tests for the isEmail() method. I put more detail about this in a comment below.
All in all, keep it up! (And don't worry about working on the caching thing, this seems like plenty to work on already)
| // verify that input is a valid email format (not specifically macalester) | ||
| // the text box expects email input, but as far as I can tell it only uses that to set the keyboard | ||
| // code from: https://stackoverflow.com/a/35789191 | ||
| static func isEmail(email:String) -> Bool { |
There was a problem hiding this comment.
This looks good, but would be even better if we had some tests for it too. In the "The Mac Weekly Tests" folder you can find a swift file that has a few tests in it already. If you could add a few basic tests that would help me feel a little better about this regex working, especially seeing as I have a hard enough time figuring it out when I write it myself lol.
3 participants
New layout for login page