-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Reland root predictive back #132249
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
Reland root predictive back #132249
Conversation
| /// [Android's predictive back](https://developer.android.com/guide/navigation/predictive-back-gesture) | ||
| /// feature will not animate when this boolean is false. | ||
| /// {@endtemplate} | ||
| final bool canPop; |
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.
With the old WillPopScope, the API was expecting a Future, which enabled hooking this into showing a dialog before popping the route. How would we support that here?
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.
I forgot to respond here because we discussed this on Discord, but for the record, I've added an example to the migration guide about this: flutter/website#9244. It's similar to the pop_scope.0.dart example in this PR.
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.
@justinmc it work in case pop to previous route. But can you please example how to hide the app when double-back at first route?
Before
Future<bool> onWillPop() {
DateTime now = DateTime.now();
if (_currentBackPressTime == null ||
now.difference(_currentBackPressTime!) > const Duration(seconds: 2)) {
_currentBackPressTime = now;
DialogUtil.showSnackBar(context, content: 'Press back again to exit');
return Future.value(false);
}
return Future.value(true);
}
I can not do it with new PopScope, the only way required setState to change canPop
Update: Fixed by using SystemNavigator.pop();
…s being called after the app is detached
66374fb to
d7a302a
Compare
| onNotification: widget.onNavigationNotification == null ? null : (NavigationNotification notification) { | ||
| // By default, don't do anything with navigation notifications if | ||
| // there is no engine attached. | ||
| if (widget.onNavigationNotification == WidgetsApp.defaultOnNavigationNotification |
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.
This is weird can we work around this by making this non static? or a static method that takes in a widget state or changenotifier and returns a function ?
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.
It turns out I do need this fix. I've cleaned it up and made it non-static in this commit: b332896
39c9b56 to
56c8777
Compare
chunhtai
left a comment
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.
LGTM
A user requested this example be added to the migration guide [on Discord](https://discord.com/channels/608014603317936148/969301283825659914/1140422179511603290). I think it's a reasonable request. Follow up on: #8952 Framework PR: flutter/flutter#132249 --------- Co-authored-by: Parker Lougheed <[email protected]>
| // Avoid dispatching a notification in the middle of a build. | ||
| switch (SchedulerBinding.instance.schedulerPhase) { | ||
| case SchedulerPhase.postFrameCallbacks: | ||
| notification.dispatch(subtreeContext); |
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.
Is it intentional that there's no break here? If so, can you add a comment that you're intentionally falling through?
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.
Root predictive back (#120385) was reverted in #132167. This PR is an attempt to reland it.
The reversion happened due to failed Google tests (b/295073110).