Adds animations package with open container transition #30
Conversation
| /// Signature for a function that creates a [Widget] to be used within an | ||
| /// [OpenContainer]. | ||
| /// | ||
| /// The `action` callback provided to [OpenContainer.closedBuilder] can be used |
| /// The `action` callback provided to [OpenContainer.closedBuilder] can be used | ||
| /// to open the container. The `action` callback provided to | ||
| /// [OpenContainer.closedBuilder] can be used to close the container again. | ||
| typedef OpenContainerChildBuilder = Widget Function( |
There was a problem hiding this comment.
Maybe just call this OpenContainerBuilder? That would forgo any discussion about if what was being built was actually the container's child, or a descendant.
| VoidCallback action, | ||
| ); | ||
|
|
||
| /// Container, that grows to fill the screen to reveal new content when tapped. |
There was a problem hiding this comment.
A container that grows...
| /// Container, that grows to fill the screen to reveal new content when tapped. | ||
| /// | ||
| /// While the container is closed, it shows the [Widget] returned by | ||
| /// [closedBuilder]. When the container is tapped the container opens: It grows |
There was a problem hiding this comment.
When the container is tapped it grows
| @override | ||
| Widget build(BuildContext context) { | ||
| return _Hideable( | ||
| key: _hidableKey, |
There was a problem hiding this comment.
Consistency is key :-) _hideable
| Size get placeholder => _placeholder; | ||
| Size _placeholder; | ||
| set placeholder(Size value) { | ||
| if (_placeholder == value) { |
There was a problem hiding this comment.
Assert value != null here?
There was a problem hiding this comment.
null is totally ok here, it means "don't use the placeholder" as per doc comment.
|
|
||
| class _HideableState extends State<_Hideable> { | ||
| /// When non-null the child is replaced by a [SizedBox] of the set size. | ||
| Size get placeholder => _placeholder; |
There was a problem hiding this comment.
This property sounds like a widget value, maybe placeholderSize?
| /// | ||
| /// The value of this property is ignored when [placeholder] is non-null (i.e. | ||
| /// [isInTree] returns false). | ||
| bool get visible => _visible; |
There was a problem hiding this comment.
Maybe isVisible because bool?
| } | ||
|
|
||
| @override | ||
| Duration get transitionDuration => const Duration(milliseconds: 300); |
There was a problem hiding this comment.
maybe this should be a parameter
| } | ||
|
|
||
| @override | ||
| Duration get transitionDuration => const Duration(milliseconds: 300); |
| /// See also: | ||
| /// | ||
| /// * [Material.elevation], which is used to implement this property. | ||
| final double closedElevation; |
There was a problem hiding this comment.
Maybe open and closed elevation parameters should be Animatable<double>, so instead of
closedElevation: 0,
openElevation: 4,You'd write
elevation: Tween<double>(begin: 0, end: 4),The advantage is that you the developer could define an arbitrary elevation change (could be defined with a TweenSequence for example). The disadvantage is that it's not as clear: is "begin" open or closed?.
The open and closed shape properties could be handled similarly.
There was a problem hiding this comment.
Talking this over, we decided to not add more configurability to this for now.
| /// `action` callback that is provided to the [closedBuilder]. | ||
| final bool tappable; | ||
|
|
||
| /// The time it will take to animate the container from its closed to its |
There was a problem hiding this comment.
Maybe provide open and closed durations?
There was a problem hiding this comment.
The underlying ModalRoute would have to be extended for this to support different durations.
| /// to its original size while the widget returned by [openBuilder] is faded out | ||
| /// and the widget returned by [openBuilder] is faded back in. | ||
| /// | ||
| /// By default, the container is in the closed state. |
There was a problem hiding this comment.
If I'm understanding things correctly, during the transition the widgets returned by the open and closed builders exist at the same time. Do we have to warn developers that these widgets cannot include the same global key?
| final Rect rect = _insetsTween.evaluate(curvedAnimation); | ||
| final Size size = _sizeTween.evaluate(curvedAnimation); | ||
|
|
||
| return Container( |
There was a problem hiding this comment.
Could be a Padding widget
| final Size size = _sizeTween.evaluate(curvedAnimation); | ||
|
|
||
| return Container( | ||
| padding: EdgeInsets.fromLTRB( |
There was a problem hiding this comment.
I wonder why we don't have EdgeInsets.fromRect()
There was a problem hiding this comment.
That seems like a good addition. Filed flutter/flutter#45576.
| height: size.height, | ||
| child: Material( | ||
| clipBehavior: Clip.antiAlias, | ||
| animationDuration: Duration.zero, |
There was a problem hiding this comment.
Maybe we need a Material.noImpliciAnimation() constructor - except with a better name.
There was a problem hiding this comment.
Yes, we do! flutter/flutter#45577
| @@ -0,0 +1,15 @@ | |||
| # Fancy pre-built Animations for Flutter | |||
There was a problem hiding this comment.
Eventually we should up with a less fanciful (ha) description of this package. Here and elsewhere.
| matching: find.byType(Material), | ||
| ), | ||
| ); | ||
| final Material srcMaterial = srcMaterialElement.widget; |
There was a problem hiding this comment.
Not sure why we don't use tester.widget here. Wouldn't this work?
final Material material = tester.widget<Material>(
find.ancestor(
of: find.text('Closed'),
matching: find.byType(Material),
),
);There was a problem hiding this comment.
We could do that here. But further down I am checking how the Widget configuration of this element is changing over time and it this makes the test more symmetric.
* Fix gradient regressions, OoO support for use/gradients - Defer resolution to build and assert resolution doesn't happen during parsing. - Fix regressions introduced in the last commit around userSpaceOnUse with transforms for linear gradients, and regressions around radial gradients in general. - Tests Things that are still wrong: - I noticed while working on flutter_svg's "devil" SVG test that blends are not being accurately applied to individual paths. I'll try to address that in a separate PR. - userSpaceOnUse gradients with transforms are not being applied correctly. This has been an issue in this repo forever, not a regression. Gotta figure out how to properly transform the gradient to the transformed path coordinate space. See https://github.com/dnfield/flutter_svg/pull/54/files#r216222991 and the test files added in that PR, which do not compile correctly today. * fix inheritence for gradient properties
This adds a new
package:animationsto the repository, which will contain fancy pre-built animations that can be customized with content and dropped into any application to delight its users.The first animation added in this patch is Material's Open Container Transition. It displays some kind of a container (e.g. a Card or Button) on screen and when tapped the container expands its size to take up the entire space of the surrounding navigator to reveal more content.
I am currently working on adding tests for this. In a follow-up PR I also plan to add an example application in which users can try out these effects.