Skip to content

[animations] Support for returning value from popped route #153

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 15 commits into from
Jun 1, 2020
Merged

[animations] Support for returning value from popped route #153

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e7db387
Added support for returning value from popped route
Melvin-Abraham May 9, 2020
b999469
Example for `onClosed` added
Melvin-Abraham May 9, 2020
b30c094
Formatted dart files
Melvin-Abraham May 9, 2020
2608d1b
Merge remote-tracking branch 'upstream/master'
Melvin-Abraham May 16, 2020
e8cb98f
Updated `open_container.dart`
Melvin-Abraham May 16, 2020
e5f7686
Added test for `onClosed` with popped route value
Melvin-Abraham May 16, 2020
7a04bbf
Updated `container_transition.dart`
Melvin-Abraham May 16, 2020
73b1bdd
Removed trailing whitespace in open_container.dart
Melvin-Abraham May 16, 2020
74e4c22
Updated open_container.dart
Melvin-Abraham May 19, 2020
0dd18c8
Updated container_transition.dart
Melvin-Abraham May 19, 2020
5b6e371
Updated open_container_test.dart
Melvin-Abraham May 19, 2020
0d70fac
Updated open_container.dart
Melvin-Abraham May 21, 2020
9fb86f8
Merge remote-tracking branch 'upstream/master'
Melvin-Abraham May 27, 2020
5265f21
Updated open_container.dart
Melvin-Abraham May 27, 2020
9bc240b
Merge remote-tracking branch 'upstream/master'
Melvin-Abraham May 28, 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
38 changes: 31 additions & 7 deletions packages/animations/example/lib/container_transition.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,10 @@ class _OpenContainerTransformDemoState

@override
Widget build(BuildContext context) {
final GlobalKey<ScaffoldState> scaffoldKey = GlobalKey<ScaffoldState>();
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated

return Scaffold(
key: scaffoldKey,
appBar: AppBar(
title: const Text('Container transform'),
actions: <Widget>[
Expand Down Expand Up @@ -200,10 +203,16 @@ class _OpenContainerTransformDemoState
),
const SizedBox(height: 16.0),
...List<Widget>.generate(10, (int index) {
return OpenContainer(
return OpenContainer<bool>(
transitionType: _transitionType,
openBuilder: (BuildContext _, VoidCallback openContainer) {
return _DetailsPage();
return const _DetailsPage(includeMarkAsDoneBtn: true);
},
onClosed: (bool isMarkedAsDone) {
if (isMarkedAsDone) {
scaffoldKey.currentState.showSnackBar(
const SnackBar(content: Text('Marked as done!')));
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
}
},
tappable: false,
closedShape: const RoundedRectangleBorder(),
Expand All @@ -223,10 +232,10 @@ class _OpenContainerTransformDemoState
}),
],
),
floatingActionButton: OpenContainer(
floatingActionButton: OpenContainer<dynamic>(
transitionType: _transitionType,
openBuilder: (BuildContext context, VoidCallback _) {
return _DetailsPage();
return const _DetailsPage();
},
closedElevation: 6.0,
closedShape: const RoundedRectangleBorder(
Expand Down Expand Up @@ -263,10 +272,10 @@ class _OpenContainerWrapper extends StatelessWidget {

@override
Widget build(BuildContext context) {
return OpenContainer(
return OpenContainer<dynamic>(
transitionType: transitionType,
openBuilder: (BuildContext context, VoidCallback _) {
return _DetailsPage();
return const _DetailsPage();
},
tappable: false,
closedBuilder: closedBuilder,
Expand Down Expand Up @@ -453,10 +462,25 @@ class _InkWellOverlay extends StatelessWidget {
}

class _DetailsPage extends StatelessWidget {
const _DetailsPage({this.includeMarkAsDoneBtn = false});
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated

final bool includeMarkAsDoneBtn;

@override
Widget build(BuildContext context) {
return Scaffold(
appBar: AppBar(title: const Text('Details page')),
appBar: AppBar(
title: const Text('Details page'),
actions: <Widget>[
includeMarkAsDoneBtn
? IconButton(
icon: const Icon(Icons.done),
onPressed: () => Navigator.pop(context, true),
tooltip: 'Mark as done',
)
: Container()
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
],
),
body: ListView(
children: <Widget>[
Container(
Expand Down
31 changes: 21 additions & 10 deletions packages/animations/lib/src/open_container.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ enum ContainerTransitionType {
///
/// * [Transitions with animated containers](https://material.io/design/motion/choreography.html#transformation)
/// in the Material spec.
class OpenContainer extends StatefulWidget {
class OpenContainer<T> extends StatefulWidget {
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
/// Creates an [OpenContainer].
///
/// All arguments except for [key] must not be null. The arguments
/// [closedBuilder] and [closedBuilder] are required.
/// [openBuilder] and [closedBuilder] are required.
/// `T` refers to the type of data returned by the route,
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
/// which can be accessed in the `onClosed` function.
const OpenContainer({
Key key,
this.closedColor = Colors.white,
Expand Down Expand Up @@ -167,7 +169,15 @@ class OpenContainer extends StatefulWidget {
final ShapeBorder openShape;

/// Called when the container was popped and has returned to the closed state.
final VoidCallback onClosed;
/// The return value from the popped screen is passed to this function as an
Comment thread
Melvin-Abraham marked this conversation as resolved.
/// argument.
Comment thread
Melvin-Abraham marked this conversation as resolved.
///
/// ```
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
/// onClosed: (DataType data) {
/// ...
/// }
/// ```
final Function(T) onClosed;
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated

/// Called to obtain the child for the container in the closed state.
///
Expand Down Expand Up @@ -218,10 +228,10 @@ class OpenContainer extends StatefulWidget {
final bool useRootNavigator;

@override
_OpenContainerState createState() => _OpenContainerState();
_OpenContainerState<T> createState() => _OpenContainerState<T>();
}

class _OpenContainerState extends State<OpenContainer> {
class _OpenContainerState<T> extends State<OpenContainer<T>> {
// Key used in [_OpenContainerRoute] to hide the widget returned by
// [OpenContainer.openBuilder] in the source route while the container is
// opening/open. A copy of that widget is included in the
Expand All @@ -235,8 +245,9 @@ class _OpenContainerState extends State<OpenContainer> {
final GlobalKey _closedBuilderKey = GlobalKey();

Future<void> openContainer() async {
await Navigator.of(context, rootNavigator: widget.useRootNavigator)
.push(_OpenContainerRoute(
final T data =
await Navigator.of(context, rootNavigator: widget.useRootNavigator)
.push(_OpenContainerRoute<T>(
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
closedColor: widget.closedColor,
openColor: widget.openColor,
closedElevation: widget.closedElevation,
Expand All @@ -251,7 +262,7 @@ class _OpenContainerState extends State<OpenContainer> {
transitionType: widget.transitionType,
));
if (widget.onClosed != null) {
widget.onClosed();
widget.onClosed(data);
}
}

Expand Down Expand Up @@ -347,7 +358,7 @@ class _HideableState extends State<_Hideable> {
}
}

class _OpenContainerRoute extends ModalRoute<void> {
class _OpenContainerRoute<T> extends ModalRoute<T> {
_OpenContainerRoute({
@required this.closedColor,
@required this.openColor,
Expand Down Expand Up @@ -580,7 +591,7 @@ class _OpenContainerRoute extends ModalRoute<void> {
}

@override
bool didPop(void result) {
bool didPop(T result) {
_takeMeasurements(
navigatorContext: subtreeContext,
delayForSourceRoute: true,
Expand Down
44 changes: 22 additions & 22 deletions packages/animations/test/open_container_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ void main() {

await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
Comment thread
Melvin-Abraham marked this conversation as resolved.
Outdated
closedColor: Colors.green,
openColor: Colors.blue,
closedElevation: 4.0,
Expand Down Expand Up @@ -192,7 +192,7 @@ void main() {

await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedColor: Colors.green,
openColor: Colors.blue,
closedElevation: 4.0,
Expand Down Expand Up @@ -361,7 +361,7 @@ void main() {

await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedColor: Colors.green,
openColor: Colors.blue,
closedElevation: 4.0,
Expand Down Expand Up @@ -535,7 +535,7 @@ void main() {

await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedColor: Colors.green,
openColor: Colors.blue,
closedElevation: 4.0,
Expand Down Expand Up @@ -708,7 +708,7 @@ void main() {
(WidgetTester tester) async {
await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
tappable: false,
closedBuilder: (BuildContext context, VoidCallback _) {
return const Text('Closed');
Expand All @@ -732,7 +732,7 @@ void main() {
VoidCallback open, close;
await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
tappable: false,
closedBuilder: (BuildContext context, VoidCallback action) {
open = action;
Expand Down Expand Up @@ -767,7 +767,7 @@ void main() {
testWidgets('open widget keeps state', (WidgetTester tester) async {
await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
return const Text('Closed');
},
Expand Down Expand Up @@ -806,7 +806,7 @@ void main() {
VoidCallback open;
await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
open = action;
return Switch(
Expand Down Expand Up @@ -855,7 +855,7 @@ void main() {

testWidgets('closes to the right location when src position has changed',
(WidgetTester tester) async {
final Widget openContainer = OpenContainer(
final Widget openContainer = OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
return Container(
height: 100,
Expand Down Expand Up @@ -917,7 +917,7 @@ void main() {
testWidgets('src changes size while open', (WidgetTester tester) async {
final Widget openContainer = _boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
return const _SizableContainer(
initialSize: 100,
Expand Down Expand Up @@ -989,7 +989,7 @@ void main() {
(WidgetTester tester) async {
await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
return const Text('Closed');
},
Expand Down Expand Up @@ -1040,7 +1040,7 @@ void main() {
height: 400,
child: _boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
return const Text('Closed');
},
Expand Down Expand Up @@ -1098,7 +1098,7 @@ void main() {
height: 400,
child: _boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) {
return const Text('Closed');
},
Expand Down Expand Up @@ -1130,7 +1130,7 @@ void main() {
height: 400,
child: _boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
transitionDuration: const Duration(seconds: 2),
closedBuilder: (BuildContext context, VoidCallback action) {
return const Text('Closed');
Expand Down Expand Up @@ -1178,7 +1178,7 @@ void main() {
height: 400,
child: _boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedShape: RoundedRectangleBorder(
borderRadius: BorderRadius.circular(10),
),
Expand Down Expand Up @@ -1251,7 +1251,7 @@ void main() {

await tester.pumpWidget(_boilerplate(
child: Center(
child: OpenContainer(
child: OpenContainer<dynamic>(
closedColor: Colors.green,
openColor: Colors.blue,
closedElevation: 4.0,
Expand Down Expand Up @@ -1319,7 +1319,7 @@ void main() {
cacheExtent: 0,
controller: controller,
itemBuilder: (BuildContext context, int index) {
return OpenContainer(
return OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback _) {
return SizedBox(
height: 100,
Expand Down Expand Up @@ -1392,7 +1392,7 @@ void main() {
cacheExtent: 0,
controller: controller,
itemBuilder: (BuildContext context, int index) {
return OpenContainer(
return OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback _) {
return SizedBox(
height: 100,
Expand Down Expand Up @@ -1485,8 +1485,8 @@ void main() {
testWidgets('onClosed callback is called when container has closed',
(WidgetTester tester) async {
bool hasClosed = false;
final Widget openContainer = OpenContainer(
onClosed: () {
final Widget openContainer = OpenContainer<dynamic>(
onClosed: (dynamic _) {

@shihaohong shihaohong May 26, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is technically a breaking change for any implementation of OpenContainer since the function signature has changed. Do we just mention this in the changelog?

@Melvin-Abraham Melvin-Abraham May 27, 2020

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.

Since there is no other way to make the parameter optional, I think just mentioning it in the changelog would be the way to go.

@shihaohong shihaohong May 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That makes sense! Could you add an entry to the changelog with a migration guide?

It should just outline what a user would have to do to get back to the previous, working state of their code if they were to update their version of the animations package

@Melvin-Abraham Melvin-Abraham May 28, 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.

@shihaohong I had another look at it and I feel that the changes should not break existing codebase which is evident by the fact that not much changes are made to the open_container_test.dart file and still it works just fine.

  • It is not necessary to specify the type arguments for OpenContainer
  • Action callback in OpenContainer.openBuilder could either be CloseContainerActionCallback or the good old VoidCallback.
  • onClosed is a new feature as it was not available in 1.0.0+5. Also, specifying it is not necessary.

Still, let me know if there are any specific cases where it might break.

@shihaohong shihaohong Jun 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was mostly worried about existing users of the onClosed parameter, but as you pointed out, it was not available in 1.0.0+5 so it should be okay!

hasClosed = true;
},
closedBuilder: (BuildContext context, VoidCallback action) {
Expand Down Expand Up @@ -1540,7 +1540,7 @@ void main() {
settings: route,
builder: (BuildContext context) {
return Container(
child: OpenContainer(
child: OpenContainer<dynamic>(
useRootNavigator: useRootNavigator,
closedBuilder: (BuildContext context, _) {
return const Text('Closed');
Expand Down Expand Up @@ -1709,7 +1709,7 @@ class __RemoveOpenContainerExampleState
Widget build(BuildContext context) {
return removeOpenContainerWidget
? const Text('Container has been removed')
: OpenContainer(
: OpenContainer<dynamic>(
closedBuilder: (BuildContext context, VoidCallback action) =>
Column(
children: <Widget>[
Expand Down