-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Return value when pop #3368
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
Return value when pop #3368
Changes from 2 commits
f74ad0d
72db2d6
ba08b81
845dfbd
0105de8
4a6352c
ecee639
98e1216
1b5e37b
b734714
ba99cc5
9b556a3
8d34c15
0f0eafc
77f6ab9
251966a
ed87847
1661569
c3ed204
02a4f5c
a44a5cc
801c7c6
5d88336
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 |
|---|---|---|
|
|
@@ -68,4 +68,23 @@ Navigator.of(context).push( | |
| ); | ||
| ``` | ||
|
|
||
| ## Returning values | ||
| Waiting for a value to be returned: | ||
|
|
||
| ```dart | ||
| onTap: () { | ||
| final bool? result = await context.push<bool>('/page2'); | ||
| WidgetsBinding.instance.addPostFrameCallback((_) { | ||
|
Contributor
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. Why a post frame callback?
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. This was before context.mounted was implemented, and using context after the value was returned lead to some dirty context problems, but we can take it out |
||
| if(result ?? false)... | ||
| }); | ||
| } | ||
| ``` | ||
|
|
||
| Returning a value: | ||
|
|
||
| ```dart | ||
| onTap: () => context.pop(true) | ||
| ``` | ||
|
|
||
|
|
||
| [Named routes]: https://pub.dev/documentation/go_router/latest/topics/Named%20routes-topic.html | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ dependencies: | |
| go_router: | ||
| path: .. | ||
| logging: ^1.0.0 | ||
| package_info_plus_web: ^2.0.0 | ||
|
Contributor
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. can this be removed? |
||
| provider: ^6.0.0 | ||
| shared_preferences: ^2.0.11 | ||
| url_launcher: ^6.0.7 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,8 +76,9 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
| } | ||
|
|
||
| /// Pushes the given location onto the page stack | ||
| void push(RouteMatchList matches) { | ||
| Future<T?> push<T extends Object?>(RouteMatchList matches) async { | ||
| assert(matches.last.route is! ShellRoute); | ||
| final Completer<T?> completer = Completer<T?>(); | ||
|
|
||
| // Remap the pageKey to allow any number of the same page on the stack | ||
| final int count = (_pushCounts[matches.fullpath] ?? 0) + 1; | ||
|
|
@@ -91,10 +92,12 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
| error: matches.last.error, | ||
| pageKey: pageKey, | ||
| matches: matches, | ||
| completer: completer, | ||
|
Contributor
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. Why not let ImperativeRouteMatch to create the completer itself. |
||
| ); | ||
|
|
||
| _matchList.push(newPageKeyMatch); | ||
| notifyListeners(); | ||
| return completer.future; | ||
| } | ||
|
|
||
| /// Returns `true` if the active Navigator can pop. | ||
|
|
@@ -113,6 +116,7 @@ class GoRouterDelegate extends RouterDelegate<RouteMatchList> | |
| final _NavigatorStateIterator iterator = _createNavigatorStateIterator(); | ||
| while (iterator.moveNext()) { | ||
| if (iterator.current.canPop()) { | ||
| iterator.matchList.last.completer?.complete(result); | ||
|
Contributor
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. Instead of exposing the completer directly, can you add a method in RouteMatch something like didComplete()? and let ImperativeRouteMatch to override the method to call complete on the completer |
||
| iterator.current.pop<T>(result); | ||
| return; | ||
| } | ||
|
|
@@ -278,6 +282,7 @@ class ImperativeRouteMatch extends RouteMatch { | |
| required super.error, | ||
| required super.pageKey, | ||
| required this.matches, | ||
| super.completer, | ||
|
Contributor
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. Somewhere in the doc should mention about the completer and future and what it meant. also, please remove the todo for me. Thanks in advance
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. Done |
||
| }); | ||
|
|
||
| /// The matches that produces this route match. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'dart:async'; | ||
|
|
||
| import 'package:flutter/foundation.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
|
|
||
|
|
@@ -18,6 +20,7 @@ class RouteMatch { | |
| required this.extra, | ||
| required this.error, | ||
| required this.pageKey, | ||
| this.completer, | ||
| }); | ||
|
|
||
| // ignore: public_member_api_docs | ||
|
|
@@ -27,6 +30,7 @@ class RouteMatch { | |
| required String parentSubloc, // e.g. /family/f2 | ||
| required Map<String, String> pathParameters, | ||
| required Object? extra, | ||
| Completer<dynamic>? completer, | ||
|
Contributor
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. If you use method this property should be in ImerativeRouteMatch, and instead of dynamic, it should use generic type T |
||
| }) { | ||
| if (route is ShellRoute) { | ||
| return RouteMatch( | ||
|
|
@@ -35,6 +39,7 @@ class RouteMatch { | |
| extra: extra, | ||
| error: null, | ||
| pageKey: ValueKey<String>(route.hashCode.toString()), | ||
| completer: completer, | ||
| ); | ||
| } else if (route is GoRoute) { | ||
| assert(!route.path.contains('//')); | ||
|
|
@@ -56,6 +61,7 @@ class RouteMatch { | |
| extra: extra, | ||
| error: null, | ||
| pageKey: ValueKey<String>(route.hashCode.toString()), | ||
| completer: completer, | ||
| ); | ||
| } | ||
| throw MatcherError('Unexpected route type: $route', restLoc); | ||
|
|
@@ -75,4 +81,7 @@ class RouteMatch { | |
|
|
||
| /// Value key of type string, to hold a unique reference to a page. | ||
| final ValueKey<String> pageKey; | ||
|
|
||
| /// The completer for the promise returned by [GoRouter.push]. | ||
| final Completer<dynamic>? completer; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please move the item under ## NEXT to be under 6.3.0 as well