Add animated opacity to the flex layout children. #1390
Conversation
DaveShuckerow
force-pushed
the
layout-entrance-animation
branch
from
a21b3a1 to
71f4e68
Compare
| child: AnimatedBuilder( | ||
| animation: entranceController, | ||
| builder: buildEntranceAnimation, | ||
| child: WidgetVisualizer( |
There was a problem hiding this comment.
This widget tree is too deeply nested to be easily understood. Since changing any part of it also changes the indentation of the entire tree, we'll get more merge conflicts working on it together.
A helpful way to manage this is to break the method up into helper methods or to create new helper widget classes.
There was a problem hiding this comment.
I agree Dave, initially I was going to make a helper widget class for the child but decided not to do that because I was still changing it a lot and I want to avoid 'prop drilling' into the class and just make use of the scope.
But since the implementation has become more complex, I agree splitting it is helpful.
Should I take this one then? @DaveShuckerow
There was a problem hiding this comment.
No need now, just pointing it out for future improvements. I want you focused on making this work off of Flutter master.
It's perfectly OK to write simple code then realize you need to refactor code later on.
There was a problem hiding this comment.
Alright, will do then, thanks Dave 👍
albertusdev
left a comment
albertusdev
left a comment
There was a problem hiding this comment.
Thanks Dave for all the help.
| .map((renderSize) => renderSize.height)) | ||
| : maxHeight, | ||
| ), | ||
| ).normalize(), |
There was a problem hiding this comment.
I was running into an issue where resizing the screen would sometimes break the box constraints here. I suspect it was a precision glitch in the constraint calculations, and normalizing has fixed it.
There was a problem hiding this comment.
Yes the cross axis empty space calculation is still buggy now, I'll fix that soon, thanks Dave :-)
| horizontal: (renderSize.width - size.width) / 2, | ||
| vertical: (renderSize.height - size.height) / 2, |
There was a problem hiding this comment.
very nit: * 0.5 instead of / 2 for consistency ?
There was a problem hiding this comment.
There's nothing wrong with using division to express division.
I'm also not sure where else there's a *0.5 in this PR.
There was a problem hiding this comment.
it's in inspector_data_models.dart where I do the computation for the empty space. But yes it's fine
| super.initState(); | ||
| entranceController = AnimationController( | ||
| vsync: this, | ||
| duration: const Duration(milliseconds: 500), |
There was a problem hiding this comment.
nit: make the duration into const maybe?
There was a problem hiding this comment.
Sorry, I mean, extract it into top level const?
| const defaultMaxRenderHeight = 300.0; | ||
|
|
||
| /// The size to shrink a widget by when animating it in. | ||
| const entranceMargin = 60; |
There was a problem hiding this comment.
nit: 60.0 to avoid casting into double since margin is double?
DaveShuckerow
changed the title
2 participants
cc @albertusangga