Magic Links: Fix window resizing animation + some clean up #1204
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
charliescheer
changed the base branch from
feature/1187-magic-links
to
lantean/1187-debt-fixes
| override func loadView() { | ||
| guard let initialViewController = topViewController else { | ||
| fatalError() | ||
| } | ||
|
|
||
| view = NSView() | ||
| heightConstraint = view.heightAnchor.constraint(equalToConstant: .zero) | ||
| heightConstraint?.isActive = true |
There was a problem hiding this comment.
Since we're now doing this, the minimumHeightConstraint workaround isn't required anymore. We can:
- Maybe convert this constraint into an IUO? or not?
- Move the
.activateto L57 instead (wheremiminumHeightConstraintlives now)
Also I'm ... slightly worried about setting it to .zero by default. It is possible, at some point in time, the Window will actually be .zero height.
Base automatically changed from
lantean/1187-debt-fixes
to
feature/1187-magic-links
July 26, 2024 19:48
Co-authored-by: Jorge Leandro Perez <[email protected]>
Co-authored-by: Jorge Leandro Perez <[email protected]>
jleandroperez
approved these changes
Jul 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix
We had noticed some wonky animations when pushing and popping from one view to another during the auth login/signup flow. This was causing the size of the window to jump to meet the new view size. Didn't look great. In this PR we have updated the animations so that it will animate smoothly to the new size of the view loaded into the navigation controller. Huge T/Y to @jleandroperez for the assist on this one.
Also while I was in there I fixed an issue where if you don't animate the navigation controller push/pop the views didn't size and the back button didn't hide correctly (we don't really need this, but if we are gonna build the component might as well make it work correctly)
and I did a bit of cleanup to make the methods simpler.
relies on #1202
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.txtif the changes should be included in release notes. Include details about updating the notes in this section. For example: