-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[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.
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
Changes from all commits
e7db387
b999469
b30c094
2608d1b
e8cb98f
e5f7686
7a04bbf
73b1bdd
74e4c22
0dd18c8
5b6e371
0d70fac
9fb86f8
5265f21
9bc240b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1486,7 +1486,7 @@ void main() { | |
| (WidgetTester tester) async { | ||
| bool hasClosed = false; | ||
| final Widget openContainer = OpenContainer( | ||
| onClosed: () { | ||
| onClosed: (dynamic _) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is technically a breaking change for any implementation of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Still, let me know if there are any specific cases where it might break. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was mostly worried about existing users of the |
||
| hasClosed = true; | ||
| }, | ||
| closedBuilder: (BuildContext context, VoidCallback action) { | ||
|
|
@@ -1525,6 +1525,51 @@ void main() { | |
| expect(hasClosed, isTrue); | ||
| }); | ||
|
|
||
| testWidgets( | ||
| 'onClosed callback receives popped value when container has closed', | ||
| (WidgetTester tester) async { | ||
| bool value = false; | ||
| final Widget openContainer = OpenContainer<bool>( | ||
| onClosed: (bool poppedValue) { | ||
| value = poppedValue; | ||
| }, | ||
| closedBuilder: (BuildContext context, VoidCallback action) { | ||
| return GestureDetector( | ||
| onTap: action, | ||
| child: const Text('Closed'), | ||
| ); | ||
| }, | ||
| openBuilder: | ||
| (BuildContext context, CloseContainerActionCallback<bool> action) { | ||
| return GestureDetector( | ||
| onTap: () => action(returnValue: true), | ||
| child: const Text('Open'), | ||
| ); | ||
| }, | ||
| ); | ||
|
|
||
| await tester.pumpWidget( | ||
| _boilerplate(child: openContainer), | ||
| ); | ||
|
|
||
| expect(find.text('Open'), findsNothing); | ||
| expect(find.text('Closed'), findsOneWidget); | ||
| expect(value, isFalse); | ||
|
|
||
| await tester.tap(find.text('Closed')); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(find.text('Open'), findsOneWidget); | ||
| expect(find.text('Closed'), findsNothing); | ||
|
|
||
| await tester.tap(find.text('Open')); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(find.text('Open'), findsNothing); | ||
| expect(find.text('Closed'), findsOneWidget); | ||
| expect(value, isTrue); | ||
| }); | ||
|
|
||
| Widget _createRootNavigatorTest({ | ||
| @required Key appKey, | ||
| @required Key nestedNavigatorKey, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.