-
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
Magic Links: Add SPNavigationController #1193
Conversation
Generated by 🚫 Danger |
✅ Code looks great One tiny detail. Notice what happens if you click on Notice that the Window Height grows as you drill down, but doesn't shrink when you go back Demo.mov |
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.
NSLayoutConstraint.activate([ | ||
backButton.leadingAnchor.constraint(equalTo: view.leadingAnchor, constant: 10), | ||
backButton.topAnchor.constraint(equalTo: spacerView.bottomAnchor), | ||
backButton.widthAnchor.constraint(equalToConstant: 50) |
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.
Magic numbers in L51, L57 and here
This is happening because the window has a set height when it is created and is not constrained to the views, so when it shrinks the view doesn't get smaller. We are discussing changes to the views them selves which will fix this issue for the login. This shouldn't be a cause of the navigation controller being added here. Should be able to fix this in the next PR for the auth view |
Fix
For the magic links setup to work we need to reshape how the auth view looks. Right now it is a single screen to sign in or an email me a link screen which has a button on it for showing the password field. This is fine for now, but doesn't really work with the new layout designs that we are putting into the other apps which has a series of screens depending on how the user wants to login and where they are in the process.
To be able to manage the transition from one screen to another we need some sort of view management system. Natively MacOS doesn't support this, so in this PR I have created a new view controller that can act similar to an iOS navigation controller. This will permit us to travel through a stack of views and return back to them if need be.
In this PR I have not made any changes to how the ui currently works except that we are pushing to the email view from sign up instead of cross fading.
Note that the sizes between the sign up and login views are a little off so the animation is a bit wonky, but that is a problem for a future PR this is just about getting the views transitioning.
Open to suggestions and feedback on how the animation works. Currently for a push the new view slides in and the old view fades out. For a pop the current view slides out and the next view fades in.
Test
(Required) List the steps to test the behavior. For example:
Review
(Required) Add instructions for reviewers. For example:
Release
(Required) Add a concise statement to
RELEASE-NOTES.txt
if the changes should be included in release notes. Include details about updating the notes in this section. For example: