Closed tahaafzal5 closed 3 years ago
I think instead of creating a new segue in the onSignUp()
function, you can just use the segue that I already set up by calling the transitionToMain()
function. Is there a different reason you created a new segue?
Also, something that we should talk about tomorrow before merging this in is that we need more information when signing the user up (like all the properties in the User.swift
class). Not sure if this should be done on a separate screen, but something to discuss before closing this issue. What do you think?
I didn't see the transitionToMain() function at first, but have made the change now.
And yes I agree to wait and discuss before closing this.
Thanks for making this change!
Issue #41 is dependent on this being merged in. So, I can either wait on doing that, or we can merge this branch in and I can work on it because I feel signing in is important.
@nashirj , I have implemented the changes we talked about. When you review it, lmk if there is anything to change. Once merged, I can start working on the log in functionality.
I think we're going to burn all our time if we focus on making Auto Layout look good, but there are definitely some weird things going on. Can we agree that we'll just make this look good on say, the iPhone 12 Pro max and iPhone 12 mini? I think those are two different enough screen sizes that if we get the auto layout looking good on both of those we can just call it good enough for now.
You can see that the Signup modal popover looks a bit strange on the iPhone 12 mini, so something to address there. There should also be a way to dismiss the modal pop over if the user decides they don't want to sign up
Also, the password should be hidden by default during sign up and log in. This is an option that you can set directly on the storyboard
You also should have a separate view controller for the sign up screen
I made some changes based on all the things I said above, and also implemented logging in, so this PR will also close #41. If you can download this and it all looks ok, feel free to merge
The auto layout issues are interesting. I think it worked fine for me on the iPhone 12 mini when I did it, but I can see how auto layout can be tricky. I also learned there is a way to by-pass auto layout by creating views programmatically -- but we should leave it out for now. For the demo, I agree on just making it work on the iPhone 12 Pro max and iPhone 12 mini.
And thanks for making those changes, it looks good and I will merge this in.
That's weird, it came out looking weird on my end but looked great on the iphone 12 Pro Max. I guess as long as we stick to using those two phone types that should be fine for the demo's sake. We can adapt to more phone geometries later.
Sure thing thanks for merging!
Also programmatically making views seems really cool! I am definitely interested in doing that at some point later
closes #42