-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
Reconsider margin transition on sidenav #404
Comments
The sidenav already animates using I think a good middle ground is the way |
@hansl The sidenav is only animating the content element's margin when in |
Yes, but we still set the |
Shouldn't make a difference as long as it's not actually changing the margin, though, right? |
That was the rationale, but I don't remember how much research I did on that subject. |
@jadjoubran The margin should only be animating in |
In the demo app the margin does change on the It is indeed necessary that the margin changes in |
Yeah, what @sebakerckhof said |
We took out some test devices and did find that the relayouts in the |
@jelbourn sounds great! It's actually visible on my macbook pro |
Have you looked at calc? i.e width: calc(100vw - $sidebarWidthVar) |
It's about animation performance, so I don't see where calc would help here. + I think the components should be as composable / nestable as possible, so I think it should not rely on viewport units. |
@jelbourn Thanks for the fix! But animating with I can submit a PR if you want 😄 |
Ah, I didn't notice that it wasn't already using |
@jelbourn Sure - going to use |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Bug
Poor performance for sidenav toggle transition
@jelbourn's material2 demo and sidenav template show that transition is targeting
margin-left
.I would expect animating
transform: translateX
unless there's a reason that I missedSmoother toggle transition
Material alpha 4
I would love to fix this if you agree
Thanks!
The text was updated successfully, but these errors were encountered: