Skip to content

[animations] Add fillColor property to shared axis transitions #133

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.

Sign up for GitHub

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

Merged
shihaohong merged 10 commits into from
Apr 10, 2020
Merged

[animations] Add fillColor property to shared axis transitions #133

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3f39081
Add fillColor property to shared axis transitions
shihaohong Apr 9, 2020
448c7ce
Revert accidental changes to the example app
shihaohong Apr 9, 2020
5987ef6
Formart dart files
shihaohong Apr 10, 2020
968faa3
Add accidentally removed changes to fillColor
shihaohong Apr 10, 2020
46a191f
Third times the charm
shihaohong Apr 10, 2020
dd12425
Update CHANGELOG.md
shihaohong Apr 10, 2020
de91965
Wordsmithing of CHANGELOG instructions
shihaohong Apr 10, 2020
fb10277
Modify Changelog
shihaohong Apr 10, 2020
6053935
Fix CHANGELOG, since 1.0.0+6 was not actually released
shihaohong Apr 10, 2020
f54dc0e
Update pubspec.yaml
shihaohong Apr 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions packages/animations/lib/src/shared_axis_transition.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,17 @@ class SharedAxisPageTransitionsBuilder extends PageTransitionsBuilder {
/// Construct a [SharedAxisPageTransitionsBuilder].
const SharedAxisPageTransitionsBuilder({
this.transitionType,
this.fillColor,
});

/// Determines which [SharedAxisTransitionType] to build.
final SharedAxisTransitionType transitionType;

/// The color to use for the background color during the transition.
///
/// This defaults to the [Theme]'s [ThemeData.canvasColor].
final Color fillColor;

@override
Widget buildTransitions<T>(
PageRoute<T> route,
Expand All @@ -97,6 +103,7 @@ class SharedAxisPageTransitionsBuilder extends PageTransitionsBuilder {
animation: animation,
secondaryAnimation: secondaryAnimation,
transitionType: transitionType,
fillColor: fillColor,
child: child,
);
}
Expand Down Expand Up @@ -186,6 +193,7 @@ class SharedAxisTransition extends StatefulWidget {
@required this.animation,
@required this.secondaryAnimation,
@required this.transitionType,
this.fillColor,
this.child,
}) : assert(transitionType != null),
super(key: key);
Expand Down Expand Up @@ -215,6 +223,11 @@ class SharedAxisTransition extends StatefulWidget {
/// axis transition types.
final SharedAxisTransitionType transitionType;

/// The color to use for the background color during the transition.
///
/// This defaults to the [Theme]'s [ThemeData.canvasColor].
final Color fillColor;

/// The widget below this widget in the tree.
///
/// This widget will transition in and out as driven by [animation] and
Expand Down Expand Up @@ -341,6 +354,7 @@ class _SharedAxisTransitionState extends State<SharedAxisTransition> {
animation: _flip(widget.animation),
transitionType: widget.transitionType,
reverse: true,
fillColor: widget.fillColor,
child: child,
);
}
Expand All @@ -355,6 +369,7 @@ class _SharedAxisTransitionState extends State<SharedAxisTransition> {
return _ExitTransition(
animation: widget.secondaryAnimation,
transitionType: widget.transitionType,
fillColor: widget.fillColor,
child: child,
);
case AnimationStatus.dismissed:
Expand Down Expand Up @@ -453,13 +468,15 @@ class _ExitTransition extends StatelessWidget {
this.animation,
this.transitionType,
this.reverse = false,
this.fillColor,
this.child,
});

final Animation<double> animation;
final SharedAxisTransitionType transitionType;
final Widget child;
final bool reverse;
final Color fillColor;
final Widget child;

static final Animatable<double> _fadeOutTransition = FlippedCurveTween(
curve: accelerateEasing,
Expand Down Expand Up @@ -487,7 +504,7 @@ class _ExitTransition extends StatelessWidget {
return FadeTransition(
opacity: _fadeOutTransition.animate(animation),
child: Container(
color: Theme.of(context).canvasColor,
color: fillColor ?? Theme.of(context).canvasColor,

@goderbauer goderbauer Apr 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we have to give this a background color at all? Couldn't it just be unopinionated and let whatever color is behind it shine through?

@shihaohong shihaohong Apr 9, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was an oversight. Would it be better to just remove the color parameter from the Container altogether?

@goderbauer goderbauer Apr 9, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. It seems the transition shouldn't really have an opinion about the background color.

@shihaohong shihaohong Apr 10, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually remember why now. This becomes a problem if you use SharedAxisPageTransitionsBuilder without a Container with some sort of fill color. What ends up happening is while the transition is occurring, you'll get a black background if there's nothing behind those routes already, since both become transparent. This isn't a problem when using the PageTransitionSwitcher because it does have a background color behind the coming and going widgets.

@goderbauer goderbauer Apr 10, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that makes sense.

child: Transform.translate(
offset: slideOutTransition.evaluate(animation),
child: child,
Expand All @@ -504,7 +521,7 @@ class _ExitTransition extends StatelessWidget {
return FadeTransition(
opacity: _fadeOutTransition.animate(animation),
child: Container(
color: Theme.of(context).canvasColor,
color: fillColor ?? Theme.of(context).canvasColor,
child: Transform.translate(
offset: slideOutTransition.evaluate(animation),
child: child,
Expand All @@ -516,7 +533,7 @@ class _ExitTransition extends StatelessWidget {
return FadeTransition(
opacity: _fadeOutTransition.animate(animation),
child: Container(
color: Theme.of(context).canvasColor,
color: fillColor ?? Theme.of(context).canvasColor,
child: ScaleTransition(
scale: (!reverse ? _scaleUpTransition : _scaleDownTransition)
.animate(animation),
Expand Down
227 changes: 226 additions & 1 deletion packages/animations/test/shared_axis_transition_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import 'package:flutter/animation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart';
import 'package:vector_math/vector_math_64.dart';
import 'package:vector_math/vector_math_64.dart' hide Colors;

void main() {
group('SharedAxisTransitionType.horizontal', () {
Expand Down Expand Up @@ -505,6 +505,80 @@ void main() {
expect(find.byKey(const ValueKey<String>(topRoute)), findsNothing);
},
);

testWidgets(
'default fill color',
(WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
const String bottomRoute = '/';
const String topRoute = '/a';

// The default fill color should be derived from ThemeData.canvasColor.
final Color defaultFillColor = ThemeData().canvasColor;

await tester.pumpWidget(
_TestWidget(
navigatorKey: navigator,
transitionType: SharedAxisTransitionType.horizontal,
),
);

expect(find.text(bottomRoute), findsOneWidget);
Finder fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, defaultFillColor);

navigator.currentState.pushNamed(topRoute);
await tester.pump();
await tester.pumpAndSettle();

fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/a')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, defaultFillColor);
}
);

testWidgets(
'custom fill color',
(WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
const String bottomRoute = '/';
const String topRoute = '/a';

await tester.pumpWidget(
_TestWidget(
navigatorKey: navigator,
fillColor: Colors.green,
transitionType: SharedAxisTransitionType.horizontal,
),
);

expect(find.text(bottomRoute), findsOneWidget);
Finder fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, Colors.green);

navigator.currentState.pushNamed(topRoute);
await tester.pump();
await tester.pumpAndSettle();

fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/a')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, Colors.green);
}
);
});

group('SharedAxisTransitionType.vertical', () {
Expand Down Expand Up @@ -1004,6 +1078,80 @@ void main() {
expect(find.byKey(const ValueKey<String>(topRoute)), findsNothing);
},
);

testWidgets(
'default fill color',
(WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
const String bottomRoute = '/';
const String topRoute = '/a';

// The default fill color should be derived from ThemeData.canvasColor.
final Color defaultFillColor = ThemeData().canvasColor;

await tester.pumpWidget(
_TestWidget(
navigatorKey: navigator,
transitionType: SharedAxisTransitionType.vertical,
),
);

expect(find.text(bottomRoute), findsOneWidget);
Finder fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, defaultFillColor);

navigator.currentState.pushNamed(topRoute);
await tester.pump();
await tester.pumpAndSettle();

fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/a')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, defaultFillColor);
}
);

testWidgets(
'custom fill color',
(WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
const String bottomRoute = '/';
const String topRoute = '/a';

await tester.pumpWidget(
_TestWidget(
navigatorKey: navigator,
fillColor: Colors.green,
transitionType: SharedAxisTransitionType.vertical,
),
);

expect(find.text(bottomRoute), findsOneWidget);
Finder fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, Colors.green);

navigator.currentState.pushNamed(topRoute);
await tester.pump();
await tester.pumpAndSettle();

fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/a')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, Colors.green);
}
);
});

group('SharedAxisTransitionType.scaled', () {
Expand Down Expand Up @@ -1396,6 +1544,80 @@ void main() {
expect(find.byKey(const ValueKey<String>(topRoute)), findsNothing);
},
);

testWidgets(
'default fill color',
(WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
const String bottomRoute = '/';
const String topRoute = '/a';

// The default fill color should be derived from ThemeData.canvasColor.
final Color defaultFillColor = ThemeData().canvasColor;

await tester.pumpWidget(
_TestWidget(
navigatorKey: navigator,
transitionType: SharedAxisTransitionType.scaled,
),
);

expect(find.text(bottomRoute), findsOneWidget);
Finder fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, defaultFillColor);

navigator.currentState.pushNamed(topRoute);
await tester.pump();
await tester.pumpAndSettle();

fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/a')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, defaultFillColor);
}
);

testWidgets(
'custom fill color',
(WidgetTester tester) async {
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
const String bottomRoute = '/';
const String topRoute = '/a';

await tester.pumpWidget(
_TestWidget(
navigatorKey: navigator,
fillColor: Colors.green,
transitionType: SharedAxisTransitionType.scaled,
),
);

expect(find.text(bottomRoute), findsOneWidget);
Finder fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, Colors.green);

navigator.currentState.pushNamed(topRoute);
await tester.pump();
await tester.pumpAndSettle();

fillContainerFinder = find.ancestor(
matching: find.byType(Container),
of: find.byKey(const ValueKey<String>('/a')),
).last;
expect(fillContainerFinder, findsOneWidget);
expect(tester.widget<Container>(fillContainerFinder).color, Colors.green);
}
);
});
}

Expand Down Expand Up @@ -1462,11 +1684,13 @@ class _TestWidget extends StatelessWidget {
this.navigatorKey,
this.contentBuilder,
this.transitionType,
this.fillColor,
});

final Key navigatorKey;
final _ContentBuilder contentBuilder;
final SharedAxisTransitionType transitionType;
final Color fillColor;

@override
Widget build(BuildContext context) {
Expand All @@ -1477,6 +1701,7 @@ class _TestWidget extends StatelessWidget {
pageTransitionsTheme: PageTransitionsTheme(
builders: <TargetPlatform, PageTransitionsBuilder>{
TargetPlatform.android: SharedAxisPageTransitionsBuilder(
fillColor: fillColor,
transitionType: transitionType,
),
},
Expand Down