-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Tree Grid: use framer motion for all animations #33765
Conversation
Size Change: +2.15 kB (0%) Total Size: 1.03 MB
ℹ️ View Unchanged
|
5060cbe
to
7481f98
Compare
To help keep scope down, I think we'll want to tackle drag refactoring in another PR. The main blocker I'm seeing at the moment is that typing speed is quite affected with the persistent list view open. Going to profile a bit to see if I spot anything. |
f77e949
to
6bdf425
Compare
I ended up using memo on the ListViewSidebar to avoid the performance hit while typing. Previously I was seeing ~100ms on keypress vs 30-40ms. I haven't noticed any broken functionality so far (most child components appear to be driven from our data stores) but folks are welcome to try and break this branch. As a reminder, we should memo carefully, but I do think this is a good candidate since it renders often (on every keypress, re-renders with the same props, and is medium sized (on my machine it takes around 50ms to resolve). I'll focus on tidying up animations next week. |
This is just beautiful, thanks so much for tackling animation with such care! Here's a "before": Here's what I see in this branch: Just lovely. Specifically it becomes very clear here how framer treats the animation as a whole sequence of events where you scrub forwards and backwards on a timeline, as opposed to much CSS animation which is often bluntly toggled off or on. This is especially visible when you expand a node, then quickly collapse the group again before the animation has finished. This is great and really speaks to how animation can help reinforce points of origins and destinations in the user experience, rather than just be decorative. It also adheres to the most important rules of thumb I have for animation (many of which I compiled after listening to this wonderful podcast):
If those rules of thumbs are met, I find that there's almost no aspect of an interface that can't benefit from well-done animation. Everything from icons that animate (such as a check drawing itself when you click a checkbox) to live feedback in the canvas when hovering or moving. When done well, animation like this can help reinforces the behavior of elements, can give feedback on success or failure, and ultimately help orient you almost in a way similar to how a focus style can help you find your place. One question: when I open the panel initially, items fade in. I get the feeling that we get that fade "for free", as it's part of the lifycle of the animation. Is that something we can turn off somewhat easily, though? If we can, that would make it consistent with other panels that open, whose content also doesn't fade in. Fading on its own doesn't reinforce points of origin or destinations of items in the same way as motion does (#33132 is a great example of how replacing a fade with motion helps indicate what's about to happen), so I tend to be a little more sparing with the usage. Really fantastic work, thank you! |
9a47c2c
to
b20bceb
Compare
Yes, we can turn it off. It's defined here: 🤔 Normally I'd simply animate height from '0' to 'auto'. One thing I'm running into is that table rows treat the CSS height property as a min-height, which makes animating this a little interesting. I'll play around with this a bit more. |
This rings a bell for me for basic CSS animation also, animating from nothing to the intrinsic height. From what I recall the trick I've used in the past was to animate the |
Tables are built to take into account it's cell dimensions. Some options that could work:
I'll circle back to this, after I try and sort out #33717 |
f510e1f
to
9bb06cd
Compare
6bdf425
to
09bd076
Compare
Okay in b5c7b46 I went ahead and did a few things.
Since this component is built with tables, animating height is going to be annoying (since we end up needing to either scale the row or modify height on child cells). This is why I keep the opacity fade in effect for expand. This helps soften things as elements render in, and other existing branches move down. |
Drag explorations are getting pretty long here so I'll continue with that in #34022, and drop the experimental changes on this branch. |
715a6c0
to
3187402
Compare
🤔 I played around a bit with disabling animations while dragging to avoid the slowdown, it doesn't feel quite right / is a bit tricky to get state right. |
Even if it's still a work in progress, the work you're doing here is amazing. This is what I see: It feels like the current behavior very loyally mimics the in-canvas behavior:
As is also implied in #33950, this is a bit of a legacy behavior that, longer term, isn't necessarily what we want. That is, collapsing to a chip could still be necessary in the canvas itself when dragging very big items — but especially here in the list view, the value of collapsing to a chip is reduced. Which suggests that the dragging could be more instant/live. That might also be a mammoth task so I'm not necessarily suggesting that's what it has to be — even what's in this branch now is a vast improvement. Only clarifying so as to not limit your explorations here ✨ |
closing in favor of #34022 |
This PR updates the TreeGrid component to use framer-motion. When children are added/removed or deleted, we can note that these actions should be animated from one state to another with a simple
layout
prop.This enables behaviors like:
Animating on move (existing behavior)
movers.mp4
Animate expand/collapse of list items (new)
expand-collapse.mp4
Animations of new/removed nodes.
(new)suppressedThis is more of an unintended side effect but I'll play with this, we'll want to reduce transitions or turn off animation here I think.I turned off this interactionanimate.nodes.mp4
TODO:
cc heads up @talldan since I know you're also looking at list view improvements