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 6 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
6 changes: 6 additions & 0 deletions packages/animations/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## TBD

@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.

Maybe just have the header say ## [1.0.1-dev] - TBD and remove the explanation below. When we release, we can just remove the dev and insert the date and the next change after the release would just add the TBD header again.

(For the release version, we wouldn't want that explanation in the changelog.

@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.

Also, then we can decide for every change we add which version number to bump (major.minor.bugfix).

Update with every new PR. If there is a new release, add a TBD section
on top of this one and add the release version and the date of release.

* Add custom fillColor property to `SharedAxisTransition` and `SharedAxisPageTransitionsBuilder`.

## [1.0.0+6] - April 9, 2020

* Fix prefer_const_constructors lint in test and example.
Expand Down
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
Loading