flutter / flutter

Flutter makes it easy and fast to build beautiful apps for mobile and beyond
https://flutter.dev
BSD 3-Clause "New" or "Revised" License
166.19k stars 27.49k forks source link

Document that WillPopScope prevents swipe to go back on MaterialPageRoute #14203

Open dudeofawesome opened 6 years ago

dudeofawesome commented 6 years ago

Steps to Reproduce

  1. Add a MaterialPageRoute to your app's page stack
    return new MaterialPageRoute<Null>(
      settings: settings,
      builder: (BuildContext context) => new StoryPage(itemId: itemId),
    );
  2. Wrap that page's Scaffold in a WillPopScope widget
    return new WillPopScope(
      onWillPop: () async {
        return true;
      },
      child: new Scaffold(
        …
      ),
    );
  3. Try to swipe to go back on iOS. The page won't swipe.

This also occurs if you replace the MaterialPageRoute with a CupertinoPageRoute.

I think this might be an unintended side-affect of fixing #8269.

For a live example of this, see my repo here.

Flutter Doctor

[✓] Flutter (on Mac OS X 10.13.2 17C205, locale en-US, channel master)
    • Flutter version 0.0.21-pre.284 at /Users/DudeOfAwesome/Library/Developer/flutter
    • Framework revision 7cdfe6fa0e (2 days ago), 2018-01-20 00:36:00 -0800
    • Engine revision e45eb692b1
    • Tools Dart version 2.0.0-dev.16.0
    • Engine Dart version 2.0.0-edge.93d8c9fe2a2c22dc95ec85866af108cfab71ad06

[✓] Android toolchain - develop for Android devices (Android SDK 27.0.3)
    • Android SDK at /Users/DudeOfAwesome/Library/Android/sdk
    • Android NDK at /Users/DudeOfAwesome/Library/Android/sdk/ndk-bundle
    • Platform android-27, build-tools 27.0.3
    • ANDROID_HOME = /Users/DudeOfAwesome/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-915-b08)

[✓] iOS toolchain - develop for iOS devices (Xcode 9.2)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 9.2, Build version 9C40b
    • ios-deploy 1.9.2
    • CocoaPods version 1.3.1

[✓] Android Studio (version 3.0)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-915-b08)

[✓] IntelliJ IDEA Ultimate Edition (version 2017.2.5)
    • Flutter plugin version 18.1
    • Dart plugin version 172.4343.25

[✓] Connected devices
    • iPhone X • C927CB75-4D41-426B-9B32-D859B2FC5FFD • ios • iOS 11.2 (simulator)
Hixie commented 6 years ago

This is intended behavior. Is the documentation lacking? Can you elaborate on why you wouldn't want this?

dudeofawesome commented 6 years ago

The page that I'm trying to watch is a full page, not just a modal like in #8269. I see in the documentation that it sounds like this widget is only intended to wrap a modal.

The goal that I'm trying to achieve is to add a listener for when the page pops, so that I can save the scroll position to the my app's state store. Is there a better way to go about this?

Hixie commented 6 years ago

WillPopScope is about preventing people from popping.

If you just want to store something when the page pops, I recommend storing it all the time (so that you handle unexpected events like the app crashing). If that doesn't work for you, you could use a custom MaterialPageRoute subclass that exposes the relevant route lifecycle methods like didPush/didPop for you (you could even make that work with a widget like WillPopScope, using a similar implementation strategy). Or you could use a NavigatorObserver.

Hixie commented 6 years ago

We should make sure we explain WillPopScope in its docs, and either explain how to do the effect you want, or provide some dedicated widget to do so.

dudeofawesome commented 6 years ago

I got the idea to use WillPopScope from this blog post I found on the flutterdev subreddit. In lieu of that, I've instead added a scroll listener that I debounce so that I'm not writing to the store and SQLite every frame.

I think that it would be beneficial to have an easy way to detect a page pop for similar scenarios. For example, if the Android settings app were made with Flutter, you'd want to be able to turn off Bluetooth searching when the user leaves the Bluetooth page.

alanrussian commented 6 years ago

@Hixie mulligan just noticed this issue. We use WillPopScope, for example, to prevent a user from losing their changes on a form screen. When the user has no changes to lose, we return true.

In the cases where the user has no changes to return, they can press the back button to go to the previous screen; however, they can't use the swipe back gesture. I think from a user's perspective, I wouldn't understand why I can't swipe back.

I'm assuming the onWillPop callback isn't used to determine whether the back gesture should be displayed because it's asynchronous. If so, maybe it would be good to create a synchronous version of it to accomplish this task? In cases like the one I mentioned, we know the value synchronously.

What are your thoughts?

Hixie commented 6 years ago

Being async isn't necessarily a blocker (so long as it's fast enough to fit within a single frame).

@xster was looking at this recently I believe.

xster commented 6 years ago

@dudeofawesome: I think the WillPopScope is the wrong tool for the scenario you're describing. If you want to conduct cleanup task either at the route level or at the widget level to release resources or stop execution that could have a longer lifecycle than the UI, I'd use the NavigatorObserver like Ian suggested or the dispose() callback on State objects.

@alanrussian: I'd argue WillPopScope is likely the wrong UX pattern to use in your case too. Since the back swipe is from iOS, we can take iOS patterns as prior art reference. When you're in a 'route' on iOS where it's possible to enter data that can be lost by navigating away, that view controller should always be presented rather than pushed. In Flutter-land, that should always translate to a PageRoute<T>.fullscreenDialog being true which will always prevent back swipes regardless of whether there is data to be lost or not.

You can, for instance, reference the native iOS contacts app or calendar app and try to create a new entry. It's always a bottom up transition and a cancel button.

In other words, I think showing a [< Back] button is 'wrong' UX in the first place even without the gesture. In iOS, you should never press the [< Back] button which then triggers an action sheet that asks you do you want to discard data. The [< Back] button should always unconditionally succeed via tap or back swipe.

alanrussian commented 6 years ago

Thanks for the reply, @xster. I'm not super familiar with iOS design patterns these days so cc/ @johnfesa and @jklint-g.

We have one use case today where we use WillPopScope. It's on a screen that we call a "settings" screen but it serves two purposes: displaying detailed information about an entity and then allowing the user to edit a smaller set of details. The user can tap a pencil icon in the app bar which turns the screen turns into a form in place (same route & widget). The app bar left action does turn into an X in this case, but since the two screens are nearly identical, I think we thought it would look bad to have a transition between two screens.

In this case, we're not pushing a new route. In order to selectively use WillPopScope, we'd have to conditionally wrap the contents of the screen with the widget and use a GlobalKey to follow the StatefulWidget performance best practices

Avoid changing the depth of any created subtrees or changing the type of any widgets in the subtree. For example, rather than returning either the child or the child wrapped in an IgnorePointer, always wrap the child widget in an IgnorePointer and control the IgnorePointer.ignoring property. This is because changing the depth of the subtree requires rebuilding, laying out, and painting the entire subtree, whereas just changing the property will require the least possible change to the render tree (in the case of IgnorePointer, for example, no layout or repaint is necessary at all).

If the depth must be changed for some reason, consider wrapping the common parts of the subtrees in widgets that have a GlobalKey that remains consistent for the life of the stateful widget. (The KeyedSubtree widget may be useful for this purpose if no other widget can conveniently be assigned the key.)

I'd argue that if the pattern we have is common, this should be easier to do.

xster commented 6 years ago

The last usage you just described is correct right? In edit mode with the X button, you get a conditional close on the X tap and you can never swipe and native iOS would behave the same way (for instance when going into edit mode for a contact which turns the [< Back] button into Cancel).

Are you asking about a different question / feature request in this case than back swipe gestures?

A Navigator.of(context).pushReplacement seems acceptable in this case too. iOS does it with an in-place fade transition.

A semi-side-discussion, I wouldn't overly depend on all the data being UI state data in this case either (re: reshaping trees with GlobalKeys). Since the user might cancel the edit etc, you'll want your non-Flutter Dart application logic to harmonize between the 2 modes anyway (at which point it's cheap to recreate the UI on 2 different routes based on your application logic source of truth).

kf6gpe commented 6 years ago

@tvolkert Could we see about getting a volunteer to do a doc update for this as part of the doc fixit this week? Thanks!

tvolkert commented 6 years ago

It should get picked up in a sweep of issues tagged as api docs.

dark-chocolate commented 5 years ago

How to achieve back swipe when using WillPopScope? Have you guys mentioned that too somewhere?

sakchhams commented 5 years ago

We use WillPopScope to prevent users from going back to the previous page in a multipage survey.

However, at the end of the survey, we want the user to be able to the home screen to take another one if they wish so.

We're pushing routes in agreement to the abovementioned behavior so a pop on the final page would navigate users back to the home screen. The same works for Android when the user presses the back button, and we'd like the user to be able to do the same on iOS using the swipe gesture.

Any suggestions?

skela commented 5 years ago

I think many people just want to be able to set a navigator result so we don't have to set create our own backbutton, or disable swipe to go back. Having the ability to just do Navigator.onPopResult(context,whatever); which then would be used for the back gesture and the default back button, could go a long way. I'm just using WillPopScope as it gets me 80% of the way, the only downside is that the back swipe stops working, which isnt ideal.

cubissimo commented 5 years ago

Maybe instead of just prevent the gesture, a way to detect and handle the swipe back inside the WillPopScope give us more flexibility to accomplish more use cases

mhstoller commented 5 years ago

@Hixie @xster Is there any chance that we can get a WillPopScope property that can enable/disable the swipe-back gesture? I want to be able to use the widget so that I can send data back to a previous page using Navigator.pop(context, data); but don't want to lose the gesture to achieve this.

Hixie commented 5 years ago

you can set the route's data without using pop, which would avoid the issue

mhstoller commented 5 years ago

@dark-chocolate I found the way to do this without using WillPopScope, you can use the addScopedWillPopCallback method.

There's some more info here: https://api.flutter.dev/flutter/widgets/ModalRoute/addScopedWillPopCallback.html

The first section shows how to use the WillPopScope widget, but after that it shows how to register the ModalRoute callback manually.

jamesdixon commented 5 years ago

@Hixie @xster while using WillPopScope does prevent swiping back on iOS, I've noticed that the onWillPop callback is never actually called when performing a swipe. Conversely, if you tap on the back button in the app bar, onWillPop is triggered. Is this intended behavior?

w3ggy commented 5 years ago

To achieve this behavior you can override hasScopedWillPopCallback getter in the MaterialPageRoute.

class CustomMaterialPageRoute extends MaterialPageRoute {
  @protected
  bool get hasScopedWillPopCallback {
    return false;
  }
  CustomMaterialPageRoute({
    @required WidgetBuilder builder,
    RouteSettings settings,
    bool maintainState = true,
    bool fullscreenDialog = false,
  }) : super(
          builder: builder,
          settings: settings,
          maintainState: maintainState,
          fullscreenDialog: fullscreenDialog,
        );
}

And then replace MaterialPageRoute on CustomMaterialPageRoute in your Router. Then swipe on iOS will work and the WillPopScope widget will work.

gisinator commented 5 years ago

@w3ggy , thank you for looking into this. I'm not sure if I understand your posting correctly: When I use CustomMaterialPageRoute in Screen A to push to Screen B that is wrapped inside a WillPopScope, I can now swipe left on iOS devices to go back to Screen A. That's perfect. Unfortunately, onWillPop does NOT fire, so I can not send data back to Screen A. Am I doing it wrong or did I misunderstand your code? :) Thank you, sir

dark-chocolate commented 4 years ago

@w3ggy Your code works fine but onWillPop() never gets called when swiping however it does get called when going back using default back button in the AppBar. So, any idea to make that work?

jaddoescad commented 4 years ago

I want to dynamically stop swiping back when the page is loading, why is this feature not supported.

benoitskipr commented 4 years ago

@jaddoescad very dirty fix that blocks the back swipe when is loading :

        return isLoading
          ? WillPopScope(
            //onWillPop: () async => _bloc.isLoadingSync, // meh, never called
            child: child)
          : child;
huybuidac commented 4 years ago

The simplest solution is to change onWillPop to null after loading.

class _SwipablePageState extends State<SwipablePage> {
  @override
  Widget build(BuildContext context) {
    return FutureBuilder(
      builder: (context, snapshot) {
        final loading = snapshot.connectionState == ConnectionState.waiting;
        return WillPopScope(
          onWillPop: loading ? () => Future.value(false) : null,
          child: Container(),
        );
      },
      future: Future.delayed(Duration(seconds: 1)),
    );
  }
}
pedromassangocode commented 4 years ago

Still valid in the current version of the WillPopScope's documentation.

https://api.flutter.dev/flutter/widgets/WillPopScope-class.html

urusai88 commented 4 years ago

any updates?

mees-brenzie commented 4 years ago

I simply want to return a value when the user returns to the previous page, while maintaining the swipe gesture. I cannot find a solution for this..

you can set the route's data without using pop, which would avoid the issue

@xster you're saying you are able to set the return data, without pop, and therefore return the data when the user swipes to navigate. Could you maybe inlighten us on how to do so?

@dark-chocolate I found the way to do this without using WillPopScope, you can use the addScopedWillPopCallback method.

There's some more info here: https://api.flutter.dev/flutter/widgets/ModalRoute/addScopedWillPopCallback.html

The first section shows how to use the WillPopScope widget, but after that it shows how to register the ModalRoute callback manually.

@mhstoller you found a solution, but with using the addScopedWillPopCallback the swipe gesture also seems to be disabled. What does your callback look like?

mhstoller commented 4 years ago

@mees-brenzie I just re-tested the addScopedWillPopCallback method and you're correct; it does disable the swipe gesture, I'm not sure if that worked previously and broke or what exactly I was doing. I don't have that code anymore because the whole workflow with WillPopScope was subpar.

I use a BLoC pattern and send all events through streams instead. Maybe that would work for your use case as well if you just need to pass data between the pages. I'm providing a very crude example of this, where there is initial data in the BLoC that can be updated and passed to either page where the new data will be shown (tap the text widgets to update the string). If you're going to adopt a BLoC pattern for your app you can also use a package like provider to not have to pass the BLoC instance through each widget directly. I'm using rxdart's BehaviorSubject for the stream because it's easier for me to manage than the built-in ones.

Hopefully this is something that could be useful for your use case.

import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
import 'package:rxdart/subjects.dart';

void main() {
  runApp(MyApp());
}

class MyApp extends StatelessWidget {
  final ExampleBloc bloc = ExampleBloc();
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(),
      home: Scaffold(
        body: Page1(bloc),
      ),
    );
  }
}

class ExampleBloc {
  String _currentData = "initial data";
  BehaviorSubject<String> _dataStream = BehaviorSubject<String>();
  Stream<String> get dataStream => _dataStream.stream;
  String get currentData => _currentData;

  updateData(String newData) {
    _currentData = newData;
    _dataStream.add(_currentData);
  }

  dispose() {
    _dataStream.close();
  }
}

class Page1 extends StatelessWidget {
  final ExampleBloc bloc;
  Page1(this.bloc);
  @override
  Widget build(BuildContext context) {
    return Center(
      child: Column(
        mainAxisAlignment: MainAxisAlignment.center,
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          GestureDetector(
            onTap: () async {
              await Navigator.of(context)
                  .push(MaterialPageRoute(builder: (context) => Page2(bloc)));
              print("updated data: ${bloc.currentData}");
            },
            child: Container(
              height: 40,
              width: 60,
              color: Colors.blue,
              child: Center(child: Text("Push")),
            ),
          ),
          GestureDetector(
            onTap: () async {
              bloc.updateData("data from page 1");
            },
            child: StreamBuilder<Object>(
              stream: bloc.dataStream,
              initialData: bloc.currentData,
              builder: (context, snapshot) {
                return Text(snapshot.data);
              },
            ),
          ),
        ],
      ),
    );
  }
}

class Page2 extends StatelessWidget {
  final ExampleBloc bloc;
  Page2(this.bloc);
  @override
  Widget build(BuildContext context) {
    return Material(
      child: Center(
        child: GestureDetector(
          onTap: () {
            bloc.updateData("data from page 2");
          },
          child: StreamBuilder(
            stream: bloc.dataStream,
            initialData: bloc.currentData,
            builder: (context, snapshot) {
              return Text(snapshot.data);
            },
          ),
        ),
      ),
    );
  }
}
huybuidac commented 4 years ago

I simply want to return a value when the user returns to the previous page, while maintaining the swipe gesture

@mees-brenzie It is not normal UX, if user swipe back, it means cancel or discard action, not submit action.

I'm not sure your app business, but you can try one of this:

  1. If it is confirmation page, you can push modal (user can not swipe back)
  2. If swipe back is required, you should apply state management.
  3. Or by the dirty solution: set value to argument
    // push page
    final arg = {"result": "demo"};
    await Navigator.pushNamed(context, 'DemoPage', arguments: arg);
    final result = arg["result"];
    ...
    // DemoPage
    final arg = ModalRoute.of(context).settings.arguments;
    ...
    arg["result"] = "THIS IS RESULT"
mees-brenzie commented 4 years ago

@huybuidac @mhstoller thank you for the detailed replies!

It is not normal UX, if user swipe back, it means cancel or discard action, not submit action. I'm not sure your app business, but you can try one of this:

The reason for this is based on a list of events and when the user taps on one of the events, it will open a details page where the user can register. When registered, I'd like the original list to be updated.

Setting up an entirely new Bloc for this seems a bit excessive, since it is only a flag if the page should reload.

The way I decided to solve the problem it by referencing the bloc used by the event list and call the reload from the details page, completely skipping the need for any flags or variables to be passed. This also means that the page is already reloaded when the user returns, instead of the page reloading when the page is visible again.

Thanks again for the help.

quentinleguennec commented 4 years ago

The current behaviour on iOS sounds broken to me, if onWillPop returns true I expect to be able to navigate to the previous page using the back button on Android (wich is working) and the swipe gesture on iOS (which doesn't).

Those 2 gestures (back button and swipe) are equivalent for the 2 platforms, and should behave the same. WillPopScope should not result in 2 different UX for 2 equivalent gestures.

benPesso commented 4 years ago

The current behaviour on iOS sounds broken to me, if onWillPop returns true I expect to be able to navigate to the previous page using the back button on Android (wich is working) and the swipe gesture on iOS (which doesn't).

Those 2 gestures (back button and swipe) are equivalent for the 2 platforms, and should behave the same. WillPopScope should not result in 2 different UX for 2 equivalent gestures.

I agree.

Also, the onWillPop should be called when the user attempts the swipe back gesture. That will allow the developer to show a dialog (or any other UI change) to alert the user why his gesture is not working. Currently, the swipe gesture is just disabled, and there's no way to catch it.

benPesso commented 3 years ago

I just released a new package - CupertinoWillPopScope - which should solve these issues. If anyone wants to give it a try and let me know what they think...

Bligoubloups commented 3 years ago

Any updates ?

Did someone found a workaround to simply call the onWillPop when we swipe back ?

benPesso commented 3 years ago

@Bligoubloups Did you check out my package? It solves all of these issues. https://pub.dev/packages/cupertino_will_pop_scope

Bligoubloups commented 3 years ago

@benPesso Yes I did. Swiping back called the onwillpop but did not swipe the page. It went back to the page even if it returns true. I'll try again with your demo project and ll raise an issue on your project if it doesn't work.

benPesso commented 3 years ago

@Bligoubloups Hmm That's very weird. I have it working in all of my projects, and didn't come across this. Please open an issue with some sample code, so I can have a look at it.

Make sure to use the ConditionalWillPopScope widget, if you're not conditionally setting the onWillPop property on the stock WillPopScope widget.

Bligoubloups commented 3 years ago

@benPesso Okay I just try your demo project :) Unfortunately it doesn't call the onWillPop if there is changes. I wanted that the swipe back calls the onWillPop in every case because I do some cleanup in this function. So if I have changes or not I want to call some functions.

Am I right ?

Apparently this is not the purpose of the onWillPop but it is still not normal that the back button calls it in every case and not the swipe back. As @quentinleguennec said, they should just do the same.

Thank you for your answers! :)

benPesso commented 3 years ago

@Bligoubloups I'm not sure I follow what you're saying. The package has two purposes:

  1. Prevent back navigation when there are changes.
  2. Make a callback when navigation is prevented.

If there are no changes, the user should simply be allowed to go back. (You shouldn't have any cleanup to do, since there are no changes to handle.)

Am I missing something here?

huybuidac commented 3 years ago

This is very simple by https://github.com/flutter/flutter/issues/14203#issuecomment-644239576 Just changing the callback to null is enough.

benPesso commented 3 years ago

@huybuidac Yup. For me, it was missing the UX aspect of communicating the "Action Cancellation" to the user, which is why I created the package. (And also because conditional operators shouldn't be a requirement for such rudimentary functionality. 😜 )

lucasjinreal commented 3 years ago

@benPesso I want swipeback on iOS as well as do some "saving stuff" in onwillpop callback, on android it was easy, but iOS breaks. I need your package to do this on iOS, but since you brrowed code from official one, can u keep maintain it make it realiable? otherwise deperating maintainance after 2 months is very painful as flutter envolves very fast.

benPesso commented 3 years ago

@jinfagang Not sure I understand what you're saying... Is my package behaving differently than the original widget in a bad way? The behavior of the Flutter WillPopScope widget is that if the onWillPop property is not null, it disables the swipe gesture and the back navigation.

My package does the same, but with three major advantages:

  1. It allows the user to partially swipe back before the navigation is cancelled, giving a visual queue that something is "blocking their action".
  2. It allows the developer to set the onWillPop property once, and specify under which conditions this callback should be triggered, by using the shouldAddCallbacks property.
  3. It makes the callback for both the AppBar "back" button and the swipe gesture, while Flutter only triggers the callback on the AppBar button tap.

I didn't quite get your last point about deprecation of maintenance, but as you can see, I'm trying to help and improve the package as much as I can. Just help me understand what it is you want to achieve with it. (And maybe post it on my package, instead of here.) 🙂

lucasjinreal commented 3 years ago

@benPesso Unfortunately, your package not work at all... Suggest other people won't waste time on this package,

danielgomezrico commented 3 years ago

Are there any intention to fix this?

This is pretty annoying, we can't intercept the back from iOS devices without destroying the UX on there, and that limits the pop results from views, it is making the app UX awful on our case.

danielgomezrico commented 3 years ago

The current behaviour on iOS sounds broken to me, if onWillPop returns true I expect to be able to navigate to the previous page using the back button on Android (wich is working) and the swipe gesture on iOS (which doesn't).

Those 2 gestures (back button and swipe) are equivalent for the 2 platforms, and should behave the same. WillPopScope should not result in 2 different UX for 2 equivalent gestures.

I totally agree with @quentinleguennec it is weird that it behaves different on each platform

darkstarx commented 3 years ago

I think everybody agree with the statement: Flutter team MUST make onWillPop being called on iOS swipe back (asynchronously preventing a swipe if onWillPop returns false), and let it be a breaking change in a new version of Flutter.

Replace this: image

With async call to route.willPop(). For that you just have to make _isPopGestureEnabled asynchronous as well as _CupertinoBackGestureDetector.enabledCallback, and then in _CupertinoBackGestureDetectorState

  void _handlePointerDown(PointerDownEvent event) {
    if (widget.enabledCallback())
      _recognizer.addPointer(event);
  }

the call _recognizer.addPointer(event); make deferred with check on mounted.

But I'm not sure if deferred adding of a pointer won't lead to mistakes in handling of drag events and _backGestureController. Whatever comes from it.. guys, you should try to do that.

wwwdata commented 3 years ago

Now there is at least Navigator 2.0. My approach now to prevent navigating back is: I just replace the nav stack and remove the pages that are behind the current page where I want to restrict navigation. When I am done and want to allow navigation, I put them back in the stack. Easy :)