Site editor: Rework navigation panel animations #26158
Conversation
|
Size Change: -11 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
|
I think this might introduce some performance concerns, as the sidebar stays mounted all the times, even when not strictly needed. I've noticed that we use I've tried it and it seems it can do the same as the simple transition (bar some CSS adjustments) plus mounting and unmounting as needed. Example: // navigation-sidebar/index.js
<CSSTransition
in={ isNavigationOpen }
classNames="edit-site-navigation-panel"
mountOnEnter
timeout={ 100 }
unmountOnExit
>
<NavigationPanel />
</CSSTransition>// navigation-panel/style.scss
.edit-site-navigation-panel-enter {
width: 0;
}
.edit-site-navigation-panel-enter-active {
width: $nav-sidebar-width;
transition: width 1000ms linear;
}
.edit-site-navigation-panel-exit {
width: $nav-sidebar-width;
}
.edit-site-navigation-panel-exit-active {
width: 0;
transition: width 1000ms linear;
}I guess we could also keep the (Note: the This said, maybe my concerns are unnecessary? 🤔 |
|
I don't think keeping it mounted is bad at all. Mounting is much more expensive than keeping it mounted. I don't want to get into the same trouble as we did with the inserter panel. (Opening the inserter drops to 3-8 fps) |
What I'm worried about is that we are unnecessarily fetching templates at first load, when we arguably want to keep things as snappy as possible. At the same time what you're saying totally makes sense, and I guess eventually the performance issues will be limited by the sidebar pagination/filtering/search/whatever. |
|
This feels very nice. |
|
@Copons Fixed the known issues! It doesn't feel too hacky for me, but I want to know what you think. We can always revert if it's too hacky. |
| transition: width 100ms linear; | ||
| @include reduce-motion("transition"); | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm not super into this spacer... 😅
I've tried animating the header padding instead, and imho the result is a good middle ground:
(animation slowed to 10%)
.edit-site-header {
padding-left: 60px;
transition: padding-left 100ms linear;
}
body.is-navigation-sidebar-open {
.edit-site-header {
padding-left: 0;
transition: padding-left 20ms linear;
}
}I guess we might poke around with the transitions times to find an even sweeter spot for the slide out, but still... What do you think?
Also I wonder if we these header transitions should be moved to the sidebar's styles instead. 🤔
There was a problem hiding this comment.
This was my very first idea. I ditched it because it seemed to be impossible to get it perfect 😄 And another reason is, we can't really use anything other than linear transitions. Anyway, if we stick to linear then it's perfect:
slide-in
The slide in transition can be calculated easily. Since our toggle is 60px and our sidebar is 300px. The sidebar takes this time to reach the toggle's width: toggleWidth / sidebarWidth * transitionTime, in our case 60px / 300px * 100ms which is 20ms. We want to shrink padding-left until the sidebar reached the toggle's width. We start the transition at the same time as we start to transition the sidebar (since the toggle is on the left side), for the time it takes to reach the toggle's width:
transition: padding-left 20ms linear;
transition-delay: 0ms;slide-out
Using the math above, we can calculate when we should start transitioning the padding-left and for how long. Since our sidebar takes 20ms to reach toggle's width, we need to delay the transition by transitionTime - timeToToggleWidth, in our case 100ms - 20ms. It takes 80ms to reach the toggle's width from a fully opened sidebar. If we delay our transition by 80ms, that means we aren't shrinking the padding-left until we reached the toggle's width. Once we reached, we need to run the transition for 20ms, which is how long it takes to go from toggle's width to zero:
transition: padding-left 20ms linear;
transition-delay: 80ms;Result:
packages/edit-site/src/components/navigation-sidebar/navigation-toggle/style.scss
Show resolved
Hide resolved
david-szabo97
force-pushed
the
update/navigation-panel-animations
branch
2 times, most recently
from
83b0517 to
bf82194
Compare
jeyip
force-pushed
the
update/navigation-panel-animations
branch
from
bf82194 to
bd29228
Compare
Copons
force-pushed
the
update/navigation-panel-animations
branch
from
bd29228 to
ce55683
Compare
|
Rebased again. I'm almost sure the failing e2e are unrelated, but let's see what happens. |
|
@david-szabo97 I'm fairly certain all the failing e2e tests are unrelated to this PR, so just go ahead and merge it. 👍 |
|
Cool work here. As a small request, I would appreciate considering the mobile experience. While the overall template building experience on mobile devices is likely not going to be the best in the near future, at some point it nevertheless needs to be considered, as it is a menu item available to mobile users. The sooner we start testing every PR for it, the easier it will be. A lot of the time it's enough to wrap desktop-specific affordances in a |
|
@jasmussen Thanks for mentioning it! |
Fixes: #26119
Description
Reworks navigation panel animations. Instead of animation it uses transition which also let us have nice closing transition.
Known issues
60px60pxwidth and shows a grey outlineHow has this been tested?
Screenshots
Types of changes
Bug fix
Checklist: