-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Platform channel for predictive back #39208
Conversation
6dd0953
to
205d1e5
Compare
This is currently waiting on the framework PR (flutter/flutter#120385) to sort out some dependencies on existing APIs. For example, things like WillPopScope need to be ported to work ahead-of-time. See that PR and the design doc for the latest. |
I tried this on a very old device (API 24) and the regular back button worked fine (obviously no predictive back). |
I was thinking I might have to implement this again for FlutterFragment and/or FlutterFragmentActivity. I'm concluding that no, I don't, but any reviewers let me know if I'm wrong. It seems like there is no OnBackInvokedDispatcher in Fragments. The PR that moved to back callbacks didn't touch those files either. Edit: I really tried to run this in an add-to-app scenario, but ran into a problem (flutter/flutter#128090). If any reviewers know for sure if I should be adding this to FlutterFragment.java too or not, please let me know. |
@@ -643,8 +645,6 @@ protected void onCreate(@Nullable Bundle savedInstanceState) { | |||
|
|||
lifecycle.handleLifecycleEvent(Lifecycle.Event.ON_CREATE); | |||
|
|||
registerOnBackInvokedCallback(); |
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.
Now that this is removed, it means that by default, Flutter will not handle back gestures. So if you start up a Flutter app that does nothing, has no Navigator or WidgetsApp etc., root predictive back will work. The framework will have to first call setFrameworkHandlesBacks in order to prevent predictive back and to handle back gestures itself. This will be handled automatically in Flutter apps that use WidgetsApp.
@@ -698,6 +700,15 @@ public void onBackInvoked() { | |||
} | |||
: null; | |||
|
|||
@Override | |||
public void setFrameworkHandlesBacks(boolean frameworkHandlesBacks) { |
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.
How do people feel about this name?
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.
suggestion: setFrameworkHandlesBack (without the plural s)?
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.
Maybe something to indicate it registers/unregisters a callback based on frameworkHandlesBacks
since it isn't actually setting that, e.g. toggleOnBackRegistry
?
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'm going to go with setFrameworkHandlesBack
for now.
I kind of wanted to be generic about what is actually happening under the hood in case that changes, or in case we ever needed to do this on another platform that handles it differently. I think setFrameworkHandlesBack
works well enough since it at least sounds declarative, like it's setting some state that decides what should happen to back gestures.
Let me know if anyone has other thoughts though.
@reidbaker I added you as a reviewer to make sure someone on the Android team is aware of this, but feel free to point me elsewhere if you're not the right person to review this. |
On vacation for 2 weeks. @camsim99 for Android |
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
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, but please get input from an Android expert.
@@ -217,6 +217,8 @@ public class FlutterActivity extends Activity | |||
implements FlutterActivityAndFragmentDelegate.Host, LifecycleOwner { | |||
private static final String TAG = "FlutterActivity"; | |||
|
|||
private boolean hasRegisteredCallback = false; |
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.
nit: should this get a more specific name as to what callback has been registered for context?
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.
Changed to: hasRegisteredBackCallback
@@ -698,6 +700,15 @@ public void onBackInvoked() { | |||
} | |||
: null; | |||
|
|||
@Override | |||
public void setFrameworkHandlesBacks(boolean frameworkHandlesBacks) { |
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.
suggestion: setFrameworkHandlesBack (without the plural s)?
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! Just left a suggestion on naming
@@ -698,6 +700,15 @@ public void onBackInvoked() { | |||
} | |||
: null; | |||
|
|||
@Override | |||
public void setFrameworkHandlesBacks(boolean frameworkHandlesBacks) { |
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.
Maybe something to indicate it registers/unregisters a callback based on frameworkHandlesBacks
since it isn't actually setting that, e.g. toggleOnBackRegistry
?
From Engine PR triage: It looks like the mac_unopt presubs failed due to a signing issue in the scenario test. This PR probably needs to be rebased, and to run the presubs again. |
I think I tried rerunning it without seeing your comment and it looks like it worked 🤷 |
Adds a platform channel method for enabling/disabling Android's predictive back feature.
…128612) flutter/engine@071e1fb...488876e 2023-06-09 [email protected] Roll ANGLE from a185cb8c8924 to 3e4f4caebcb0 (2 revisions) (flutter/engine#42701) 2023-06-09 [email protected] [Impeller] Add CPU implementations for all color filters (flutter/engine#42692) 2023-06-09 [email protected] [Impeller] add explicit VMA flush to device memory writes. (flutter/engine#42685) 2023-06-09 [email protected] [Impeller] Makes validation layers flag work for android (flutter/engine#42625) 2023-06-09 [email protected] Platform channel for predictive back (flutter/engine#39208) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This PR aims to support Android's predictive back gesture when popping the entire Flutter app. Predictive route transitions between routes inside of a Flutter app will come later. <img width="200" src="https://user-images.githubusercontent.com/389558/217918109-945febaa-9086-41cc-a476-1a189c7831d8.gif" /> ### Trying it out If you want to try this feature yourself, here are the necessary steps: 1. Run Android 33 or above. 1. Enable the feature flag for predictive back on the device under "Developer options". 1. Create a Flutter project, or clone [my example project](https://github.com/justinmc/flutter_predictive_back_examples). 1. Set `android:enableOnBackInvokedCallback="true"` in android/app/src/main/AndroidManifest.xml (already done in the example project). 1. Check out this branch. 1. Run the app. Perform a back gesture (swipe from the left side of the screen). You should see the predictive back animation like in the animation above and be able to commit or cancel it. ### go_router support go_router works with predictive back out of the box because it uses a Navigator internally that dispatches NavigationNotifications! ~~go_router can be supported by adding a listener to the router and updating SystemNavigator.setFrameworkHandlesBack.~~ Similar to with nested Navigators, nested go_routers is supported by using a PopScope widget. <details> <summary>Full example of nested go_routers</summary> ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:go_router/go_router.dart'; import 'package:flutter/material.dart'; import 'package:flutter/scheduler.dart'; void main() => runApp(_MyApp()); class _MyApp extends StatelessWidget { final GoRouter router = GoRouter( routes: <RouteBase>[ GoRoute( path: '/', builder: (BuildContext context, GoRouterState state) => _HomePage(), ), GoRoute( path: '/nested_navigators', builder: (BuildContext context, GoRouterState state) => _NestedGoRoutersPage(), ), ], ); @OverRide Widget build(BuildContext context) { return MaterialApp.router( routerConfig: router, ); } } class _HomePage extends StatelessWidget { @OverRide Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: const Text('Nested Navigators Example'), ), body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: <Widget>[ const Text('Home Page'), const Text('A system back gesture here will exit the app.'), const SizedBox(height: 20.0), ListTile( title: const Text('Nested go_router route'), subtitle: const Text('This route has another go_router in addition to the one used with MaterialApp above.'), onTap: () { context.push('/nested_navigators'); }, ), ], ), ), ); } } class _NestedGoRoutersPage extends StatefulWidget { @OverRide State<_NestedGoRoutersPage> createState() => _NestedGoRoutersPageState(); } class _NestedGoRoutersPageState extends State<_NestedGoRoutersPage> { late final GoRouter _router; final GlobalKey<NavigatorState> _nestedNavigatorKey = GlobalKey<NavigatorState>(); // If the nested navigator has routes that can be popped, then we want to // block the root navigator from handling the pop so that the nested navigator // can handle it instead. bool get _popEnabled { // canPop will throw an error if called before build. Is this the best way // to avoid that? return _nestedNavigatorKey.currentState == null ? true : !_router.canPop(); } void _onRouterChanged() { // Here the _router reports the location correctly, but canPop is still out // of date. Hence the post frame callback. SchedulerBinding.instance.addPostFrameCallback((Duration duration) { setState(() {}); }); } @OverRide void initState() { super.initState(); final BuildContext rootContext = context; _router = GoRouter( navigatorKey: _nestedNavigatorKey, routes: [ GoRoute( path: '/', builder: (BuildContext context, GoRouterState state) => _LinksPage( title: 'Nested once - home route', backgroundColor: Colors.indigo, onBack: () { rootContext.pop(); }, buttons: <Widget>[ TextButton( onPressed: () { context.push('/two'); }, child: const Text('Go to another route in this nested Navigator'), ), ], ), ), GoRoute( path: '/two', builder: (BuildContext context, GoRouterState state) => _LinksPage( backgroundColor: Colors.indigo.withBlue(255), title: 'Nested once - page two', ), ), ], ); _router.addListener(_onRouterChanged); } @OverRide void dispose() { _router.removeListener(_onRouterChanged); super.dispose(); } @OverRide Widget build(BuildContext context) { return PopScope( popEnabled: _popEnabled, onPopped: (bool success) { if (success) { return; } _router.pop(); }, child: Router<Object>.withConfig( restorationScopeId: 'router-2', config: _router, ), ); } } class _LinksPage extends StatelessWidget { const _LinksPage ({ required this.backgroundColor, this.buttons = const <Widget>[], this.onBack, required this.title, }); final Color backgroundColor; final List<Widget> buttons; final VoidCallback? onBack; final String title; @OverRide Widget build(BuildContext context) { return Scaffold( backgroundColor: backgroundColor, body: Center( child: Column( mainAxisAlignment: MainAxisAlignment.center, children: <Widget>[ Text(title), //const Text('A system back here will go back to Nested Navigators Page One'), ...buttons, TextButton( onPressed: onBack ?? () { context.pop(); }, child: const Text('Go back'), ), ], ), ), ); } } ``` </details> ### Resources Fixes #109513 Depends on engine PR flutter/engine#39208 ✔️ Design doc: https://docs.google.com/document/d/1BGCWy1_LRrXEB6qeqTAKlk-U2CZlKJ5xI97g45U7azk/edit# Migration guide: flutter/website#8952
This PR enables Android's predictive back animation when leaving a Flutter app.
How
In order to show the predictive back animation when a back gesture happens, Android needs to know ahead of time that it, not Flutter, should handle the back gesture. This is communicated with the following new platform channel method:
setFrameworkHandlesBacks(boolean frameworkHandlesBacks)
In the framework, this will be set to true whenever the framework has a reason to prevent native predictive back, such as when there are multiple routes in the navigation history. Otherwise it will be set to false and predictive back will be enabled, such as when there is only a single route of navigation history.
By default, no back handler is registered for Flutter and predictive back is enabled.
References
Part of flutter/flutter#109513
Framework PR: flutter/flutter#120385
Design doc: https://docs.google.com/document/d/1BGCWy1_LRrXEB6qeqTAKlk-U2CZlKJ5xI97g45U7azk/edit#