Shared Z Axis Transition #57
Conversation
|
|
||
| import 'utils/curves.dart'; | ||
|
|
||
| /// Used by [PageTransitionsTheme] to define a page route transition animation |
There was a problem hiding this comment.
I think mentioning [SharedZAxisTransition] here would be worthwhile.
| /// ), | ||
| /// ); | ||
| /// }, | ||
| /// '/a' : (BuildContext context) { |
| /// return Container( | ||
| /// color: Colors.red, | ||
| /// child: Center( | ||
| /// child: MaterialButton( |
There was a problem hiding this comment.
Please don't use MaterialButton (here and below), it's a leftover. RaisedButton, FlatButton, or OutlineButton would be OK.
| } | ||
| } | ||
|
|
||
| /// Defines a transition in which outgoing and incoming elements share a scale |
There was a problem hiding this comment.
This is a true statement however maybe it's possible to craft something that's a little more specific? Here and above.
| /// | ||
| /// The shared axis pattern provides the transition animation between UI elements | ||
| /// that have a spatial or navigational relationship. For example, | ||
| /// transitioning from one page of a sign up page to the next one. |
| super.dispose(); | ||
| } | ||
|
|
||
| static final Tween<double> _flippedTween = Tween<double>( |
There was a problem hiding this comment.
Tweens are mutable. Even though you don't, it might be a little safer (future) to just create the Tween inline in _flip().
| void dispose() { | ||
| widget.animation.removeStatusListener(_animationListener); | ||
| widget.secondaryAnimation.removeStatusListener(_secondaryAnimationListener); | ||
| super.dispose(); |
There was a problem hiding this comment.
Could this widget have been disposed while the animation (the AnimatedBuilder) is still running?
I think this is safe, since dispose implies that the AnimatedBuilder is disposed as well. It would be good to test this part of the implementation (rebuild a widget with and then without the transition, while the transition is underway).
There was a problem hiding this comment.
I added a test for this. PTAL
| final Animation<double> animation; | ||
| final Widget child; | ||
|
|
||
| static Animatable<double> fadeInTransition = CurveTween(curve: decelerateEasing) |
There was a problem hiding this comment.
Similar comment about static tweens (here and elsewhere). Maybe just create these in the build method.
| // Jump 3/10ths of the way through the transition, bottom route | ||
| // should be be completely faded out while the top route | ||
| // is also completely faded out. | ||
| // Transition time: 300ms, 3/10 * 300ms = 90ms |
| _TestWidget(navigatorKey: navigator), | ||
| ); | ||
|
|
||
| navigator.currentState.pushNamed('/a'); |
| @override | ||
| void didUpdateWidget(SharedZAxisTransition oldWidget) { | ||
| super.didUpdateWidget(oldWidget); | ||
| void _updateAnimationListener(SharedZAxisTransition oldWidget) { |
There was a problem hiding this comment.
This is fine however what I meant (but muddled) was:
void _updateAnimationListener(Animation<double> oldAnimation, Animation<double> animation) {
if (oldAnimation != animation) {
oldAnimation.removeStatusListener(_animationListener);
animation.addStatusListener(_animationListener);
_animationListener(animation.status);
}
}
void didUpdateWidget(SharedZAxisTransition oldWidget) {
super.didUpdateWidget(oldWidget);
_updateAnimationListener(oldWidget.animation, widget.animation);
_updateAnimationListener(oldWidget.secondaryAnimation, widget.secondaryAnimation);
}|
|
||
| static Animatable<double> scaleInTransition = Tween<double>(begin: 0.80, end: 1.00) | ||
| .chain(CurveTween(curve: standardEasing)); | ||
| static Animatable<double> scaleInTransition = |
There was a problem hiding this comment.
Two-space indent ... I think this would be a little more conventional formatting-wise (here and below):
static Animatable<double> fadeOutTransition = FlippedCurveTween(curve: accelerateEasing)
.chain(CurveTween(curve: const Interval(0.0, 0.3)));There was a problem hiding this comment.
Done -- the formatted seemed to have removed my original formatting
| @@ -49,8 +49,8 @@ class FlippedCurveTween extends CurveTween { | |||
| /// Creates a vertically flipped [CurveTween]. | |||
| FlippedCurveTween({ | |||
| @required Curve curve, | |||
| }) : assert(curve != null), | |||
| super(curve: curve); | |||
| }) : assert(curve != null), | |||
There was a problem hiding this comment.
extra spaces ended up here
There was a problem hiding this comment.
What's weird is that the formatter suggested two spaces instead of one before the colon :. If not, the formatted complained and I'm unsure if that's a normal expectation for non-flutter/flutter projects or not
There was a problem hiding this comment.
The Dart formatter doesn't really do conventional Flutter SDK formatting.
Implement the shared z axis transition:
