-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix(angular/menu): remove dependency on animations module #2472
base: main
Are you sure you want to change the base?
Conversation
This should allow us to avoid some of the pitfalls of the animations module like occasional memory leaks and reduce the bundle size.
3444617
to
a710ab0
Compare
} | ||
to { | ||
width: 288px; | ||
max-height: 100%; |
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.
Using max-height
to animate the user menu's height creates a less fluid animation compared to directly animating its actual height.
In Angular animations, we achieved this smoothly by using height: '*'
, where the wildcard dynamically applies the element's height before the animation starts.
@jeripeierSBB, do you have any suggestions for improving the CSS transition?
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.
I got this working by using grid-template-rows
for the height animation and by adding a <div class="sbb-menu-panel-content" />
wrapper element around the menu content.
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.
LGTM 👍
Nice work! 😃
This should allow us to avoid some of the pitfalls of the animations module like occasional memory leaks and reduce the bundle size.