tomgilder / routemaster

Easy-to-use Navigator 2.0 router for web, mobile and desktop. URL-based routing, simple navigation of tabs and nested routes.
https://pub.dev/packages/routemaster
MIT License
328 stars 60 forks source link

Close dialog with back button? #305

Open sampaioletti opened 1 year ago

sampaioletti commented 1 year ago

Is it possible to close a dialog (i.e. shown with showDialog) with the back button...currently it keeps the dialog open and performs the navigation below it, I tried on Web and Android. Is this the intended behavior?

Thanks!

markphillips100 commented 1 year ago

I experience the same issue with all dialogs and bottomsheets, and additionally if I'm on the first page in a stack, selecting the back button will close the app rather than the sheet or dialog.

I've made sure the dialogs and bottomsheets use the root navigator, and I also use a global navigator key which is provided to the RoutemasterDelegate but the app still closes rather than the dialog or sheet.

You can replicate the experience with the examples mobile_app project. Press Login, and then press the "Push non-Page route" button (which uses Navigator.of() like dialogs and bottom sheets do). Tapping the Back button will close the app rather than pop the "non-Page" page.

markphillips100 commented 1 year ago

I forked here with something that seems to handle the back button better. Only tried on Android emulator.

In short, I attempt to find the current route from the root navigator and only perform the history.back() call if the route is a PageRoute. Other routes, like DialogRoute, ModalBottomSheet, etc, will divert to directly calling maybePop() on the navigator and notify listeners.

NOTE: I didn't use the PageStack maybePop() for 2 reasons; firstly, it always tries to pop a page first regardless if the nav has a route entry for a dialog or bottom sheet, and secondly, the SynchronousFuture seemed to prevent the dialog from actually dismissing even though maybePop() returns true.

Calling maybePop() on the nav directly from popRoute() and notifiying listeners works fine though.

As a side note, the extension I added to find the current route is a hack-way but it appears there's no other way of finding a current route from the navigator.

lukehutch commented 11 months ago

@markphillips100 I'm applying your patch to the latest git version, but I noticed Routemaster defines:

  /// The current global route.
  RouteData get currentRoute => _state.delegate.currentConfiguration!;

Is there a way to get a Routemaster reference in NavigatorState, so that this getter can be called, to save using the workaround you have written?

markphillips100 commented 11 months ago

@lukehutch It's been a long time since I looked at this so difficult for me to give a worthy answer as to whether I tried that approach.

lukehutch commented 11 months ago

For the record, I figured out a way to solve this without the (awesome) patch provided by @markphillips100 ...

final navigatorKey = GlobalKey<NavigatorState>();

final router = RoutemasterDelegate(
  navigatorKey: navigatorKey,
  ...);

class MyBackButtonDispatcher extends RootBackButtonDispatcher {
  @override
  Future<bool> didPopRoute() async {
    // Close any open modal
    if (navigatorKey.currentState!.canPop()) {
      navigatorKey.currentState!.pop();
      return true;
    }
    return super.didPopRoute();
  }
}

void main() async {
  runApp(
    MaterialApp.router(
      backButtonDispatcher: MyBackButtonDispatcher(),
      routerDelegate: router,
      routeInformationParser: const RoutemasterParser(),
    ),
  );
}

Credit where credit is due: the solution was provided by GitHub Copilot!

markphillips100 commented 11 months ago

@lukehutch that's cool. I was seconds away from asking how you went.

lukehutch commented 11 months ago

I couldn't believe Copilot was able to figure this out, when Google searches could not. Copilot amazes me more every day.

I still have an issue in a more complex app, where the rood back button dispatcher is not even being called, but the dialog closing part is solved now for simpler apps, at least.

lukehutch commented 11 months ago

OK, I solved my final issue with my Back button not working.

If you put android:enableOnBackInvokedCallback="true" in your application element of your android/app/src/main/AndroidManifest.xml, then you are enabling a new predictive gesture-based back mode, which requires support on the Android side. This disables the standard Flutter back button handling:

https://stackoverflow.com/a/73782321/3950982

So if Back is not working at all, look to see if you have this XML attribute in the manifest file.