jb3rndt / PersistentBottomNavBarV2

A highly customizable persistent bottom navigation bar for Flutter
https://pub.dev/packages/persistent_bottom_nav_bar_v2
BSD 3-Clause "New" or "Revised" License
47 stars 48 forks source link

[Bug]: Android physical back button exits the app #149

Open bugrevealingbme opened 2 months ago

bugrevealingbme commented 2 months ago

Version

5.2.3

Flutter Doctor Output

--

What platforms are you seeing the problem on?

Android, iOS

What happened?

I have 2 problems. 1- If I click on the current tab, it returns to the previous tab. This problem came after an update. 2- If I go to the next page with the navbar (Navigator.push), Android's return button takes me directly out of the application. I'm using GoRouter (if I go with gorouter without navbar there is no problem, but this time there is no navbar).

Steps to reproduce

1- If I click on the current tab, it returns to the previous tab. This problem came after an update. 2- If I go to the next page with the navbar (Navigator.push), Android's return button takes me directly out of the application. I'm using GoRouter (if I go with gorouter without navbar there is no problem, but this time there is no navbar).

Code to reproduce the problem

PersistentTabView(
                  controller: persistentTabController,
                  hideNavigationBar: viewModel.splashView,
                  navBarBuilder: (p0) =>
                      buildBottomAppBar(t, context, themeData, viewModel),
                  tabs: viewModel.viewList,
                  onTabChanged: (value) {
                    viewModel.currentIndex = value;
                    viewModel.currentIndexNotify.value = value;
                  },
                  //screenTransitionAnimation: const ScreenTransitionAnimation.none(),
                  handleAndroidBackButtonPress: true,
                  resizeToAvoidBottomInset: false,
                )

making handleAndroidBackButtonPress false will not solve the problem. This time my function to click on the same tab is not working.

Relevant log output

No response

Screenshots

No response

bugrevealingbme commented 2 months ago

https://github.com/jb3rndt/PersistentBottomNavBarV2/assets/22865739/f0487bb0-f444-455f-8786-b9984eac6e2b

jb3rndt commented 2 months ago

Hi, I see the issue you are having and I want to solve it.

Unfortunately, I need more information to be able to reproduce this specific behavior to then investigate what causes it. There are so many parts involved that currently I can only randomly guess what is going wrong. So please provide a minimal reproducible example (a file, that I can run as is, without the need to adjust anything) that triggers this behavior. Boiling down your application to the least code necessary to showcase the problem might need some work on your side - I am sorry for that - (especially the contents of the screens are unnecessary). If you have issues with that, a good approach is to take the example app from this project and iteratively add some code fragments of your app.

If I click on the current tab, it returns to the previous tab. This problem came after an update.

Can you please specify which update caused that? What was the old version?

jiqimao3528 commented 2 months ago

Hi, I see the issue you are having and I want to solve it.

Unfortunately, I need more information to be able to reproduce this specific behavior to then investigate what causes it. There are so many parts involved that currently I can only randomly guess what is going wrong. So please provide a minimal reproducible example (a file, that I can run as is, without the need to adjust anything) that triggers this behavior. Boiling down your application to the least code necessary to showcase the problem might need some work on your side - I am sorry for that - (especially the contents of the screens are unnecessary). If you have issues with that, a good approach is to take the example app from this project and iteratively add some code fragments of your app.

If I click on the current tab, it returns to the previous tab. This problem came after an update.

Can you please specify which update caused that? What was the old version?

@jb3rndt Hi, I simulated the code, you need to replace the main.dart code in your example with the following code to reproduce the bug.

import "package:example/interactive_example.dart";
import "package:example/screens.dart";
import "package:flutter/material.dart";
import "package:flutter/services.dart";
import "package:persistent_bottom_nav_bar_v2/persistent_bottom_nav_bar_v2.dart";

void main() {
  WidgetsFlutterBinding.ensureInitialized();
  // make navigation bar transparent
  SystemChrome.setSystemUIOverlayStyle(
    const SystemUiOverlayStyle(
      systemNavigationBarColor: Colors.transparent,
    ),
  );
  // make flutter draw behind navigation bar
  SystemChrome.setEnabledSystemUIMode(SystemUiMode.edgeToEdge);
  runApp(const PersistenBottomNavBarDemo());
}

class PersistenBottomNavBarDemo extends StatelessWidget {
  const PersistenBottomNavBarDemo({super.key});

  @override
  Widget build(BuildContext context) => MaterialApp(
        title: "Persistent Bottom Navigation Bar Demo",
        home: Builder(
          builder: (context) => Center(
            child: Column(
              mainAxisSize: MainAxisSize.min,
              children: [
                ElevatedButton(
                  onPressed: () =>
                      Navigator.of(context).pushReplacementNamed("/minimal"),
                  child: const Text("Show Minimal Example"),
                ),
                const SizedBox(height: 16),
                ElevatedButton(
                  onPressed: () =>
                      Navigator.of(context).pushNamed("/interactive"),
                  child: const Text("Show Interactive Example"),
                ),
              ],
            ),
          ),
        ),
        routes: {
          "/minimal": (context) => const MinimalExample(),
          "/interactive": (context) => const InteractiveExample(),
        },
      );
}

class MinimalExample extends StatelessWidget {
  const MinimalExample({super.key});

  List<PersistentTabConfig> _tabs() => [
        PersistentTabConfig(
          screen: const MainScreen(),
          item: ItemConfig(
            icon: const Icon(Icons.home),
            title: "Home",
          ),
        ),
        PersistentTabConfig(
          screen: const MainScreen(),
          item: ItemConfig(
            icon: const Icon(Icons.message),
            title: "Messages",
          ),
        ),
        PersistentTabConfig(
          screen: const MainScreen(),
          item: ItemConfig(
            icon: const Icon(Icons.settings),
            title: "Settings",
          ),
        ),
      ];

  @override
  Widget build(BuildContext context) => PersistentTabView(
        tabs: _tabs(),
        navBarBuilder: (navBarConfig) => Style1BottomNavBar(
          navBarConfig: navBarConfig,
        ),
        avoidBottomPadding: true,
        backgroundColor: Colors.white,
        handleAndroidBackButtonPress: false,
        resizeToAvoidBottomInset: true,
        stateManagement: true,
        popAllScreensOnTapOfSelectedTab: false,
        popActionScreens: PopActionScreensType.all,
        screenTransitionAnimation: const ScreenTransitionAnimation.none(),
      );
}

Thanks~

jb3rndt commented 2 months ago

Hi, thank you for providing some code. Unfortunately, I dont see how this reproduces the issues mentioned by @bugrevealingbme. When setting handleAndroidBackButton: true like @bugrevealingbme did, I cannot reproduce any of the described behavior.

bugrevealingbme commented 2 months ago

My application is too big so I can't share a code. However, if it is convenient for you, I can have my codes reviewed by remote connection

bugrevealingbme commented 2 months ago

I think this problem is related to gorouter

lukehutch commented 2 months ago

This is an issue with new versions of Flutter and Android, because of the introduction of predictive back gestures, which is changing the whole way the back button is handled in Android. Basically if your app doesn't support predictive back gestures yet, then hitting Back will pop the whole app (close the app). Took me forever to figure this out.

Here is the issue: https://github.com/flutter/flutter/issues/135654

Here is the way I solved it -- call navigatorPop rather than Navigator.of(context).pop:

/// Used for obtaining a global [NavigatorState].
final navigatorKey = GlobalKey<NavigatorState>();

/// Get the global [NavigatorState] (for opening modals).
NavigatorState get navigator {
  final navigatorState = navigatorKey.currentState;
  if (navigatorState == null) {
    // Should not happen as long as there is a Navigator in the router
    throw 'navigatorKey.currentState is null';
  }
  return navigatorState;
}

/// Pop the current route if possible.
bool navigatorPop<T>([T? result]) {
  if (navigatorKey.currentState?.canPop() ?? false) {
    navigatorKey.currentState!.pop(result);
    return true;
  }
  return false;
}

Basically this pops the top modal or route, but then stops popping when there are no more routes or modals left to pop within the app. Then at that point, if you hit Back one more time, the app will pop.

Then in your MaterialApp use:

      navigatorKey: navigatorKey,
jb3rndt commented 1 month ago

Is there anything this package can do differently to reduce problems with the predictive back?

lukehutch commented 1 month ago

I wish I had an answer to that question, but all the information I could find about this fundamental change was hazy at best. The Flutter issue link is a good place to start. Manually handling Back events with the code I shared solved the problem for me.

There is one other thing: there is an Android manifest value that can be set to enable predictive back, and if you enable that, the regular back event doesn't even reach Flutter. So there must be an actual API for predictive back that needs to be implemented.

Sorry, a lot of this is guesswork, or fuzzy memories, and I don't have time to go back and research this again!

jb3rndt commented 1 month ago

Thanks for the reply and the investigation.

Unfortunately, I still dont entirely understand where the problem comes from. Can you confirm that the example of this repo works as inteded for you? If so, what is different in your application that requires the code patch you posted above? (Lets ignore the android manifest value to enable predictive back).

Basically if your app doesn't support predictive back gestures yet, then hitting Back will pop the whole app (close the app).

In which case does an app not support predictive back? When the app uses the deprecated WillPopScope?

lukehutch commented 1 month ago

In which case does an app not support predictive back? When the app uses the deprecated WillPopScope?

No, I wasn't using WillPopScope, but at some point I was dealing with the whole app being popped whenever Back was pressed. Unfortunately it was so long ago that I don't remember if I even got to the bottom of why this was happening in my app and not other apps. I did not have the new predictive back enablement attribute set in the Android manifest at the time.

I do know that at one point, I was using the following code, appropriately hooked into MaterialApp IIRC, but I ended up replacing it with the code I gave previously instead, with the same effect. Without one of these two fixes, the whole app would pop.

/// Make back button close any open dialogs first, and only pop the route
/// if there are no dialogs open. (Default behavior is to pop the app on
/// every back button press)
class FixedBackButtonDispatcher extends RootBackButtonDispatcher {
  @override
  Future<bool> didPopRoute() async {
    // Close any open modal dialog before attempting to pop route
    if (navigatorKey.currentState?.canPop() ?? false) {
      navigatorKey.currentState!.pop();
      return true;
    }
    // Pop route, or pop app if at top level
    return super.didPopRoute();
  }
}

I wish I could help further but my app has evolved far past the point where I was dealing with this, and I just don't remember any other details.

bugrevealingbme commented 1 month ago

Could this be related?

https://docs.flutter.dev/release/breaking-changes/android-predictive-back#migrating-from-willpopscope-to-navigatorpophandler-for-nested-navigators

https://github.com/flutter/flutter/issues/140296

bugrevealingbme commented 1 month ago

Applying pageTransitionsTheme solved my problem.

return MaterialApp(
  theme: ThemeData(
    brightness: Brightness.light,
    pageTransitionsTheme: const PageTransitionsTheme(
      builders: {
        // Use PredictiveBackPageTransitionsBuilder to get the predictive back route transition!
        TargetPlatform.android: PredictiveBackPageTransitionsBuilder(),
      },
    ),
  ),
  home: const MyApp(),
);
lukehutch commented 1 month ago

Applying pageTransitionsTheme solved my problem.

return MaterialApp(
  theme: ThemeData(
    brightness: Brightness.light,
    pageTransitionsTheme: const PageTransitionsTheme(
      builders: {
        // Use PredictiveBackPageTransitionsBuilder to get the predictive back route transition!
        TargetPlatform.android: PredictiveBackPageTransitionsBuilder(),
      },
    ),
  ),
  home: const MyApp(),
);

Thanks for finding this, this is exactly the predictive back API that I suspected existed, but didn't know where to find!

The back problem recurred for me recently, despite the fixes I put in place already. I tried adding this PredictiveBackPageTransitionsBuilder fix, and it did not solve the problem. My earlier fixes only work with a stack of modals -- it does not solve the back behavior for tab switching, because tab switching does not push a modal onto the route stack.

So the problem I have now is that if I switch tabs multiple times, then press Back, about 50% of the time the entire app is popped immediately -- the other 50% of the time, the app will switch between tabs in the reverse order that they were selected. There is no rhyme or reason as to when Back works properly and when it immediately pops the app.

lukehutch commented 1 month ago

I think there's something funky going on here, in persistent_tab_view.dart:

  @override
  Widget build(BuildContext context) {
    if (_contextList.length != widget.tabs.length) {
      _contextList = List<BuildContext?>.filled(widget.tabs.length, null);
    }
    if ((widget.handleAndroidBackButtonPress || widget.onWillPop != null) &&
        widget.navigationShell == null) {
      return PopScope(
        canPop: canPop,
        onPopInvoked: (didPop) async {
          if (didPop) {
            return;
          }
          final navigator = Navigator.of(context);
          final shouldPop = await _canPopTabView();
          // This is only used when onWillPop is provided
          if (shouldPop) {
            if (navigator.canPop()) {
              navigator.pop();
            } else {
              await SystemNavigator.pop();
            }
          }
        },
        // ...

and

  Future<bool> _canPopTabView() async {
    if (!widget.handleAndroidBackButtonPress && widget.onWillPop != null) {
      return widget.onWillPop!(_contextList[_controller.index]!);
    } else {
      final navigator = _navigatorKeys[_controller.index].currentState!;
      if (_controller.historyIsEmpty() && !navigator.canPop()) {
        if (widget.handleAndroidBackButtonPress && widget.onWillPop != null) {
          return widget.onWillPop!(_contextList[_controller.index]!);
        }
        // CanPop should be true in this case, so we dont return true because the pop already happened
        return false;
      } else {
        if (navigator.canPop()) {
          navigator.pop();
        } else {
          _controller.jumpToPreviousTab();
        }
        return false;
      }
    }
  }

And, in custom_tab_view.dart:

  @override
  Widget build(BuildContext context) => Navigator(
        key: widget.navigatorConfig.navigatorKey,
        initialRoute: widget.navigatorConfig.initialRoute,
        onGenerateRoute: _onGenerateRoute,
        onUnknownRoute: _onUnknownRoute,
        observers: _navigatorObservers,
      );
      // ...

There are some very complex things going on here, but I suspect that the interactions between the app's root Navigator, each CustomTabView's own Navigator, the value of handleAndroidBackButtonPress, and the presence or absence of onWillPop, mean that ultimately _controller.jumpToPreviousTab(); is not called when it should be. However, that still doesn't explain why the behavior that I observe is nondeterministic (sometimes the whole app pops, sometimes the previous tab is shown).

Why on earth is this so complex? Why not just use the root Navigator for all tabs, for starters?

lukehutch commented 1 month ago

OK, I found a solution to the issue that I mentioned in the previous comment -- wrapping PersistentTabView in PopScope (with the default value of canPop: true):

PopScope(
  child: PersistentTabView(
          // ...

This seems to make PersistentBottomNavBarV2 work as expected (possibly in combination with the navigatorPop fix that I described earlier, since I still have that in place -- this pops modals on Back until there are no more modals to pop, ensuring that the app doesn't pop with modals on the stack).

With the above PopScope change, now when there are no more modals open, _controller.jumpToPreviousTab() is called on every Back action, until there are no more previous tabs -- then the whole app is popped. This is the expected behavior (and I think the correct behavior, according to the Material Design style guide).

(Note that defining onPopInvoked in PopScope may break the Back behavior again, if you define it wrongly, so just leave it undefined.)

Also, I removed popAllScreensOnTapOfSelectedTab: true and popActionScreens: PopActionScreensType.all from my definition of PersistentTabView, which I think is also needed to get the correct behavior.

The fact that this fixes the problem seems to confirm to me that somewhere in all the complex logic of PersistentBottomNavBarV2, there is a bug (i.e. the bug is not in Android).

jb3rndt commented 4 weeks ago

I'm sorry, after trying around quite some time I still cant reproduce the error. Can anyone provide some example code? (that I can run as-is)