Milad-Akarie / auto_route_library

Flutter route generator
MIT License
1.59k stars 403 forks source link

Support PopScope #1799

Closed xVemu closed 7 months ago

xVemu commented 11 months ago

As WillPopScope is deprecated in Flutter 3.16, this package should support PopScope. Currently, when using PopScope with AutoRoute, onPopInvoked is called only if canPop is set to false.

Workaround:

PopScope(
    canPop: false,
    onPopInvoked: (didPop) {
        if(didPop) return;

        ...
        // Can't be context.popRoute(), because it leads to recursion.
        Navigator.of(context).pop();
    },
    child: ...
)
asemanTest commented 11 months ago

I have the same issue too

OptimisticSean commented 11 months ago

Call the popForced function of the current auto router you're in instead of the one from the default navigator.

PopScope(
    canPop: false,
    onPopInvoked: (didPop) {
        if(didPop) return;

        ...
       // Navigator.of(context).pop();
       context.router.popForced();
    },
    child: ...
)
jakobhec commented 11 months ago

In nested routers, I used to call .popRoute() on the nested router. I think it automatically popped a route on the parent router if the resulting stack of the nested router would become empty.

I needed to change my code in onPopInvoked to get my desired behavior. Please let me know if there are better options.

final parentRouter = context.router.parentAsStackRouter;
if (context.router.stack.length == 1 && parentRouter != null) {
  parentRouter.popForced();
} else {
  context.router.popForced();
}
fikretsengul commented 9 months ago

I have same issue. How about if I want to use popUntilRoute? Should I first call context.router.popForced(); and the one line below call context.router.popUntilRoot();?

Why auto_route doesn't trigger onPopInvoked if canPop is true like the navigator 2.0 of flutter? Why is this happening and is it okay? @Milad-Akarie

slavap commented 9 months ago

Linking related issue https://github.com/Milad-Akarie/auto_route_library/issues/1825

fikretsengul commented 9 months ago

This didn't solve the problem on the mobile platform. AutoRoute doesn't work properly with pop scope.

Will pop scope was working fine. Help is really appreciated.

slavap commented 9 months ago

@fikretsengul if you want to control when/if route should be closed - canPop must be false, then in onPopInvoked you will decide what exactly to do. About modals - do you have simple code example?

fikretsengul commented 9 months ago

Of course @slavap @Milad-Akarie . Here is the repo that demonstrates official pop scope implementation and auto_route implementation.

https://github.com/fikretsengul/auto_route_popscope

navigator_main.dart (Uses: https://api.flutter.dev/flutter/widgets/PopScope-class.html) auto_route_main.dart (Uses: auto_route)

The onPopInvoked parameter reports when system back gestures occur, regardless of whether or not they were successful.

If canPop is false, then a system back gesture will not pop the route off of the enclosing Navigator. onPopInvoked will still be called, and didPop will be false.

If canPop is true, then a system back gesture will cause the enclosing Navigator to receive a pop as usual. onPopInvoked will be called with didPop as true, unless the pop failed for reasons unrelated to PopScope, in which case it will be false.

In original navigator: onPopInvoked called no matter popScope is false or not (which how should be). But in auto_route when popScope is true onPopInvoked doesn't get called. Also using popForced is a must to pop the page or modal (is the popForced method viable way to pop them?) Anyway, it's not well documented and not acts like the original behavior of navigator 2.0.

Original:

https://github.com/Milad-Akarie/auto_route_library/assets/22684086/4de3c18a-6dbe-4ecf-afc4-c63bdb156158

Auto_route:

https://github.com/Milad-Akarie/auto_route_library/assets/22684086/37051fd0-aa3b-4ae3-9a1e-a92510ad86d8

fikretsengul commented 9 months ago

if you want to control when/if route should be closed - canPop must be false, then in onPopInvoked you will decide what exactly to do.

Also this isn't the case for original navigator 2.0. Following console logs printed whether canPop is true or not if Navigator.of(context).pop() is invoked:

flutter: Attempted to pop page. flutter: Attempted to pop modal.

But unfortunately this does not apply to auto_route.

yagmure15 commented 9 months ago

I have same issue.

talhakerpicci commented 9 months ago

I have the same issue too. Any plans on handling this?

iamwalter commented 9 months ago

The same issue here. Would be awesome if this was looked at

slavap commented 8 months ago

@fikretsengul

When we give canPop = true, onPopInvoked doesn't get invoked.

For me on web onPopInvoked called in both cases, i.e. does not matter canPop true or false. Try to use maybePop() instead of pop() and popForced() in your code.

Also, have you tried my code from https://github.com/Milad-Akarie/auto_route_library/issues/1825#issuecomment-1882418839 ?

fikretsengul commented 8 months ago

@fikretsengul

When we give canPop = true, onPopInvoked doesn't get invoked.

For me on web onPopInvoked called in both cases, i.e. does not matter canPop true or false. Try to use maybePop() instead of pop() and popForced() in your code.

Also, have you tried my code from #1825 (comment) ?

Did you read my comment and tried the example project? There is no maybePop for auto_route (Navigator.of(context).maybePop() doesn't do anything for auto_route routes and modals.). Pop doesn't do anything if canPop false, the only way to close the page when canPop false is popForced.

Also your comment didn't changed anything on mobile.

EDIT: Maybe you can provide us a working example for pages and modals with auto_route using pop scope.

karimSalahx commented 8 months ago

I have the same issue

Fudal commented 8 months ago

+1

AndSpark commented 8 months ago

+1

cavin-7span commented 8 months ago

+1

Milad-Akarie commented 7 months ago

@fikretsengul I've run some tests and it seems PopScope works as intended in auto_route, router.pop() = Navigator.maybePop, I have plans to rename pop to maybePop but I'm afraid it might be misleading for some, anyways, I've peeked into the implementation of maybePop and it seems it only calls onPopInvokes when canPop is set to false which translates to RoutePopDisposition.doNotPop

image

so I'm not sure what needs to be fixed here?

P-Jay357 commented 7 months ago

@Milad-Akarie PopScope doesn't work if you wrap the MaterialApp.router with a PopScope either though. And this is without calling context.pop(), just by pressing the Android back button.

I can get PopScope to work only if I wrap each page's scaffold, but not if I wrap the main MaterialApp.router. I'm not sure what the solution for that would be (as each page having it's own PopScope isn't ideal in my use-case as I mainly use it in conjunction with a global loading overlay, as I don't want pops to occur while loading is in progress).

Milad-Akarie commented 7 months ago

Of course it won't, because the navigator which handles the pop events is built inside the router delegate inside the material App, maybe try providing your own route provider and intercept didPop event globally, I imagine that can also be done by playing with the back button delegate

karimSalahx commented 7 months ago

OnPopInvoked should be called either canPop is set to false or true, this is the problem, please check @Milad-Akarie

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

Milad-Akarie commented 7 months ago

Have you tested with the native navigator? If you check the implementation of router.pop you would see that it calls navigator.maybePop internally, so I don't think it would be any different?

P-Jay357 commented 7 months ago

@Milad-Akarie

Of course it won't, because the navigator which handles the pop events is built inside the router delegate inside the material App, maybe try providing your own route provider and intercept didPop event globally, I imagine that can also be done by playing with the back button delegate

But the package docs says to use routeConfig with MaterialApp.router, but when you look at routeConfig it says "If the routerConfig is provided, the other router related delegates, routeInformationParser, routeInformationProvider, routerDelegate, and backButtonDispatcher, must all be null.".

How could I get AutoRoute to work while intercepting pops? I can't see any other docs or posts about if you want a global loading overlay which can prevent pops while loading is in progress anywhere....

Milad-Akarie commented 7 months ago

PopScope issue should be fixed in v 7.9.0 please confirm the fix so we can close issue

Milad-Akarie commented 7 months ago

@P-Jay357 don't pass the config, pass the router.delegate and router.routeParser your self.

xVemu commented 7 months ago

Yep it's fixed in 7.9.0

AlexDochioiu commented 7 months ago

@Milad-Akarie On Flutter 3.19.4, auto_route 7.9.2, auto_route_generator: 7.3.2, the issue is still present for me. Same in version 8.0.0. P.s. I am trying on flutter web using context.maybePop to close the route.

This is how I setup the app/router in case it's relevant:

final materialApp = MaterialApp.router(
      debugShowCheckedModeBanner: false,
      routerDelegate: AutoRouterDelegate(
        appRouter,
        navigatorObservers: () => [
          ObservableNavigator(),
          HeroController(),
        ],
      ),
      localizationsDelegates: AppLocalizations.localizationsDelegates,
      supportedLocales: AppLocalizations.supportedLocales,
      builder: (context, child) => _MainWidget(child, viewIsBlurredNotifier: viewIsBlurredNotifier),
      routeInformationParser: appRouter.defaultRouteParser(),
      title: 'My App',
      theme: kAppLightTheme,
      darkTheme: kAppDarkTheme,
      themeMode: themeMode,
    );

    return materialApp;
sproott commented 2 months ago

Can confirm that this issue is still present in v9 for me. I had to use @jakobhec's workaround for it to start working.