Skip to content
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

[ go_router ] can't push page on the shell without key even though the page is outside shellroute #113001

Closed
DattatreyaReddy opened this issue Oct 6, 2022 · 10 comments · Fixed by flutter/packages#5497
Assignees
Labels
found in release: 3.4 Found to occur in 3.4 found in release: 3.5 Found to occur in 3.5 has reproducible steps The issue has been confirmed reproducible and is ready to work on p: go_router The go_router package P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. r: fixed Issue is closed as already fixed in a newer version team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team

Comments

@DattatreyaReddy
Copy link
Contributor

DattatreyaReddy commented Oct 6, 2022

Steps to Reproduce

  1. Execute flutter run on the code sample
  2. navigate as follow
    1. push /b
    2. go back
    3. go to /c
    4. push /d
    5. push /d/e

Expected results:

  • After step i the page should be on top of the shell route
  • In step v we should be able to go to /d/e

Actual results:

  • After step i the page is inside the shell
  • In step v we can't go to /d/e
Code sample
import 'package:flutter/material.dart';
import 'package:flutter_riverpod/flutter_riverpod.dart';
import 'package:go_router/go_router.dart';

void main() => runApp(const ProviderScope(child: MyApp()));

final GlobalKey<NavigatorState> _rootNavigatorKey =
    GlobalKey<NavigatorState>(debugLabel: 'root');
final GlobalKey<NavigatorState> _shellNavigatorKey =
    GlobalKey<NavigatorState>(debugLabel: 'shell');

final routesProvider = Provider<GoRouter>(
  (ref) {
    return GoRouter(
      debugLogDiagnostics: true,
      initialLocation: '/a',
      navigatorKey: _rootNavigatorKey,
      routes: [
        ShellRoute(
          navigatorKey: _shellNavigatorKey,
          builder: (context, state, child) => ShellWidget(child: child),
          routes: [
            GoRoute(
              path: '/a',
              builder: (context, state) => const DummyScreen(
                color: Colors.green,
                text: '/b',
              ),
            ),
            GoRoute(
              path: '/c',
              builder: (context, state) => const DummyScreen(
                color: Colors.green,
                text: '/d',
              ),
            ),
          ],
        ),
        GoRoute(
          path: '/d',
          parentNavigatorKey: _rootNavigatorKey,
          builder: (context, state) => const DummyScreen(
            color: Colors.red,
            text: '/d/e',
          ),
          routes: [
            GoRoute(
              // can't push this page at all
              path: 'e',
              builder: (context, state) => const DummyScreen(
                color: Colors.red,
                text: '/c',
              ),
            ),
          ],
        ),
        GoRoute(
          // can't push this page on the shell without key
          // even though it is outside of shell route
          path: '/b',
          builder: (context, state) => const DummyScreen(
            color: Colors.green,
            text: '',
          ),
        ),
      ],
    );
  },
);

class ShellWidget extends StatefulWidget {
  const ShellWidget({
    super.key,
    required this.child,
  });
  final Widget child;

  @override
  State<ShellWidget> createState() => _ShellWidgetState();
}

class _ShellWidgetState extends State<ShellWidget> {
  int index = 0;
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text("Hello")),
      bottomNavigationBar: BottomNavigationBar(
        currentIndex: index,
        onTap: (value) {
          setState(() {
            index = value;
            switch (value) {
              case 0:
                context.go('/a');
                break;
              case 1:
                context.go('/c');
                break;
              default:
            }
          });
        },
        items: const [
          BottomNavigationBarItem(
            icon: Icon(
              Icons.access_alarm,
            ),
            label: 'a',
          ),
          BottomNavigationBarItem(
            icon: Icon(
              Icons.access_alarm,
            ),
            label: 'c',
          ),
        ],
      ),
      body: widget.child,
    );
  }
}

class MyApp extends ConsumerWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final routes = ref.watch(routesProvider);
    return MaterialApp.router(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      routeInformationParser: routes.routeInformationParser,
      routeInformationProvider: routes.routeInformationProvider,
      routerDelegate: routes.routerDelegate,
    );
  }
}

class DummyScreen extends StatelessWidget {
  const DummyScreen({
    super.key,
    required this.color,
    required this.text,
  });
  final Color color;
  final String text;
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text(GoRouter.of(context).location)),
      backgroundColor: color,
      body: Center(
        child: ElevatedButton(
          onPressed: text.isNotEmpty ? () => context.push(text) : null,
          child: Text(text),
        ),
      ),
    );
  }
}
Logs
- go_router debugLogDiagnostics

[GoRouter] Full paths for routes:
             => /d
             =>   /d/e
             => /b
[GoRouter] setting initial location /a
[GoRouter] Using MaterialApp configuration
[GoRouter] pushing /b
[GoRouter] going to /c
[GoRouter] pushing /d
[GoRouter] pushing /d/e
% flutter analyze
Analyzing bug...                                                        
No issues found! (ran in 1.4s)
% flutter doctor -v
[✓] Flutter (Channel beta, 3.4.0-17.2.pre, on macOS 12.6 21G115 darwin-arm, locale en-IN)
    • Flutter version 3.4.0-17.2.pre on channel beta at /Users/padya/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision d6260f127f (2 weeks ago), 2022-09-21 13:33:49 -0500
    • Engine revision 3950c6140a
    • Dart version 2.19.0 (build 2.19.0-146.2.beta)
    • DevTools version 2.16.0

[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.0)
    • Android SDK at /Users/padya/Library/Android/sdk
    • Platform android-33, build-tools 33.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7772763)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 14.0.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 14A400
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7772763)

[✓] IntelliJ IDEA Community Edition (version 2022.1.3)
    • IntelliJ at /Applications/IntelliJ IDEA CE.app
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart

[✓] VS Code (version 1.72.0-insider)
    • VS Code at /Applications/Visual Studio Code - Insiders.app/Contents
    • Flutter extension version 3.50.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-arm64   • macOS 12.6 21G115 darwin-arm
    • Chrome (web)    • chrome • web-javascript • Google Chrome 106.0.5249.103

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!
video
Screen.Recording.2022-10-06.at.10.12.18.AM.mov
@DattatreyaReddy DattatreyaReddy changed the title [ go_router ] can't push page on the shell without key even though the page is outside of the shell [ go_router ] can't push page on the shell without key even though the page is outside shellroute Oct 6, 2022
@darshankawar darshankawar added the in triage Presently being triaged by the triage team label Oct 6, 2022
@darshankawar
Copy link
Member

@DattatreyaReddy Thanks for the report. From the given code sample, I see that you are using flutter_riverpod which is third party package.
Does the reported behavior occur without using that package ? There's a recent proposal to add redirection to go_router using flutter_riverpod #112915, so please confirm if the same behavior occurs only with go_router package. If so, please provide updated code sample.

@darshankawar darshankawar added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 6, 2022
@DattatreyaReddy
Copy link
Contributor Author

DattatreyaReddy commented Oct 6, 2022

Hi @darshankawar,
Thank you for the response.

Answering your question, Yes, the behavior is the same without flutter_riverpod.

Here is the updated code sample.
import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

void main() => runApp(MyApp());

final GlobalKey<NavigatorState> _rootNavigatorKey =
    GlobalKey<NavigatorState>(debugLabel: 'root');
final GlobalKey<NavigatorState> _shellNavigatorKey =
    GlobalKey<NavigatorState>(debugLabel: 'shell');

class ShellWidget extends StatefulWidget {
  const ShellWidget({
    super.key,
    required this.child,
  });
  final Widget child;

  @override
  State<ShellWidget> createState() => _ShellWidgetState();
}

class _ShellWidgetState extends State<ShellWidget> {
  int index = 0;
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text("Hello")),
      bottomNavigationBar: BottomNavigationBar(
        currentIndex: index,
        onTap: (value) {
          setState(() {
            index = value;
            switch (value) {
              case 0:
                context.go('/a');
                break;
              case 1:
                context.go('/c');
                break;
              default:
            }
          });
        },
        items: const [
          BottomNavigationBarItem(
            icon: Icon(
              Icons.access_alarm,
            ),
            label: 'a',
          ),
          BottomNavigationBarItem(
            icon: Icon(
              Icons.access_alarm,
            ),
            label: 'c',
          ),
        ],
      ),
      body: widget.child,
    );
  }
}

class MyApp extends StatelessWidget {
  MyApp({super.key});

  final routes = GoRouter(
      debugLogDiagnostics: true,
      initialLocation: '/a',
      navigatorKey: _rootNavigatorKey,
      routes: [
        ShellRoute(
          navigatorKey: _shellNavigatorKey,
          builder: (context, state, child) => ShellWidget(child: child),
          routes: [
            GoRoute(
              path: '/a',
              builder: (context, state) => const DummyScreen(
                color: Colors.green,
                text: '/b',
              ),
            ),
            GoRoute(
              path: '/c',
              builder: (context, state) => const DummyScreen(
                color: Colors.green,
                text: '/d',
              ),
            ),
          ],
        ),
        GoRoute(
          path: '/d',
          parentNavigatorKey: _rootNavigatorKey,
          builder: (context, state) => const DummyScreen(
            color: Colors.red,
            text: '/d/e',
          ),
          routes: [
            GoRoute(
              // can't push this page at all
              path: 'e',
              builder: (context, state) => const DummyScreen(
                color: Colors.red,
                text: '/c',
              ),
            ),
          ],
        ),
        GoRoute(
          // can't push this page on the shell without key
          // even though it is outside of shell route
          path: '/b',
          builder: (context, state) => const DummyScreen(
            color: Colors.green,
            text: '',
          ),
        ),
      ],
    );

  @override
  Widget build(BuildContext context) {
    return MaterialApp.router(
      title: 'Flutter Demo',
      theme: ThemeData(
        primarySwatch: Colors.blue,
      ),
      routeInformationParser: routes.routeInformationParser,
      routeInformationProvider: routes.routeInformationProvider,
      routerDelegate: routes.routerDelegate,
    );
  }
}

class DummyScreen extends StatelessWidget {
  const DummyScreen({
    super.key,
    required this.color,
    required this.text,
  });
  final Color color;
  final String text;
  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: Text(GoRouter.of(context).location)),
      backgroundColor: color,
      body: Center(
        child: ElevatedButton(
          onPressed: text.isNotEmpty ? () => context.push(text) : null,
          child: Text(text),
        ),
      ),
    );
  }
}

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 6, 2022
@darshankawar
Copy link
Member

Thanks for the updated code sample. Using it, I am seeing same behavior as reported.

stable, master flutter doctor -v

[✓] Flutter (Channel stable, 3.3.4, on macOS 12.2.1 21D62 darwin-x64, locale
    en-GB)
    • Flutter version 3.3.4 on channel stable at
      /Users/dhs/documents/fluttersdk/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision eb6d86ee27 (2 days ago), 2022-10-04 22:31:45 -0700
    • Engine revision c08d7d5efc
    • Dart version 2.18.2
    • DevTools version 2.15.0

[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

[✓] Flutter (Channel master, 3.5.0-3.0.pre.5, on macOS 12.2.1 21D62 darwin-x64,
    locale en-GB)
    • Flutter version 3.5.0-3.0.pre.5 on channel master at
      /Users/dhs/documents/fluttersdk/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 9ca314afd0 (2 hours ago), 2022-10-06 23:06:11 -0400
    • Engine revision a4174f29a7
    • Dart version 2.19.0 (build 2.19.0-286.0.dev)
    • DevTools version 2.18.0
    
[!] Xcode - develop for iOS and macOS (Xcode 12.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    ! Flutter recommends a minimum Xcode version of 13.
      Download the latest version or update via the Mac App Store.
    • CocoaPods version 1.11.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] VS Code (version 1.62.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.21.0

[✓] Connected device (5 available)
    • SM G975F (mobile)       • RZ8M802WY0X • android-arm64   • Android 11 (API 30)
    • Darshan's iphone (mobile)  • 21150b119064aecc249dfcfe05e259197461ce23 •
      ios            • iOS 14.4.1 18D61
    • iPhone 12 Pro Max (mobile) • A5473606-0213-4FD8-BA16-553433949729     •
      ios            • com.apple.CoreSimulator.SimRuntime.iOS-14-3 (simulator)
    • macOS (desktop)            • macos                                    •
      darwin-x64     • Mac OS X 10.15.4 19E2269 darwin-x64
    • Chrome (web)               • chrome                                   •
      web-javascript • Google Chrome 98.0.4758.80

[✓] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.



@darshankawar darshankawar added p: first party package flutter/packages repository. See also p: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on p: go_router The go_router package found in release: 3.4 Found to occur in 3.4 found in release: 3.5 Found to occur in 3.5 and removed in triage Presently being triaged by the triage team labels Oct 7, 2022
@stuartmorgan stuartmorgan added the P2 Important issues not at the top of the work list label Oct 10, 2022
@westito
Copy link

westito commented Dec 9, 2022

Same here. Can't push route on top of shell route Solved, see below

@westito
Copy link

westito commented Dec 9, 2022

There is two problem with your code:

  • Not added parentNavigatorKey: _rootNavigatorKey, to /b route. That's why not pushed on top of shell
  • You wrote e instead of /e in path

@DattatreyaReddy
Copy link
Contributor Author

DattatreyaReddy commented Dec 9, 2022

There is two problem with your code:

  • Not added parentNavigatorKey: _rootNavigatorKey, to /b route. That's why not pushed on top of shell

In the example route /d I've added parentNavigatorKey: _rootNavigatorKey then it is working but the sub route Is not working.

Also, the problem is that I need to add _rootNavigatorKey even thought it is in root navigation not the shell navigation (please, check comments in code)

  • You wrote e instead of /e in path

That is required to use subroutes in gorouter,

https://github.com/flutter/packages/blob/main/packages/go_router/doc/configuration.md#child-routes

Child routes

A matched route can result in more than one screen being displayed on a
Navigator. This is equivalent to calling push(), where a new screen is
displayed above the previous screen with a transition animation, and with an
in-app back button in the AppBar widget, if it is used.

To display a screen on top of another, add a child route by adding it to the
parent route's routes list:

GoRoute(
  path: '/',
  builder: (context, state) {
    return HomeScreen();
  },
  routes: [
    GoRoute(
      path: 'details',
      builder: (context, state) {
        return DetailsScreen();
      },
    ),
  ],
)

@omar-hanafy
Copy link

any updates on this, cause I am facing the same problem I can't go to the sub router (/referral/all) when I am outside of the shell route after giving (/referral)'s parentNavigatorKey the _rootNavigatorKey to be able to ignore shell router.
the observers show this but nothing happened:

[INFO] GoRouter: pushing /referral/all

and it actually changes the current location from /referral to /referral/all but the screen actually still on /referral.

print(GoRouter.of(this).location) // /referral/all

@flutter-triage-bot flutter-triage-bot bot added multiteam-retriage-candidate team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team labels Jul 8, 2023
@lucavenir
Copy link

Not added parentNavigatorKey: _rootNavigatorKey

Why must we do this? Shouldn't the parent be implicit? I don't understand this aspect of GoRouter.

@crystalleung926
Copy link

After absorbed the above answers, I wrote an example(pseudo code), I used go_router: ^12.0.0
using context.push or context.go will also work

You need to create key for each root or shell page, make sure to use the correct key

The structure is
/
/login
/home
/page-one
//a sub page of page-one but, not be inside the page-one shell
/page-one/account

Also, if someone is facing the route path won't change on web when navigate to sub route which belongs to different root route.
I found a solution from
https://codewithandrea.com/articles/flutter-navigation-gorouter-go-vs-push/
Add the below code on your main(), and everything works well
GoRouter.optionURLReflectsImperativeAPIs = true;

In the example, assume that I have a bottom Navigation Bar (Tabs :Home, Page One) , a button for navigating to account inside Page One and a back button from app bar inside account page.

  final _rootNavigatorKey = GlobalKey<NavigatorState>();
  final _shellNavigatorKeyHome = GlobalKey<NavigatorState>();
  final _shellNavigatorKeyShellOne = GlobalKey<NavigatorState>();

  GoRouter get router => GoRouter(
        initialLocation: '/login',
        navigatorKey: _rootNavigatorKey,
        routes: <RouteBase>[
          GoRoute(
            path: '/',
            pageBuilder: (context, state) => MaterialPage<void>(
              key: state.pageKey,
              child: Container(),
            ),
            routes: <RouteBase>[],
          ),
          GoRoute(
            name: 'Login',
            path: '/login',
            pageBuilder: (context, state) => MaterialPage<void>(
              key: state.pageKey,
              child: LoginPage(),
            ),
          ),
          StatefulShellRoute.indexedStack(
            builder: (context, state, navigationShell) {
              return ShellRootPage(
                navigationShell: navigationShell,
              );
            },
            branches: [
              StatefulShellBranch(
                  navigatorKey: _shellNavigatorKeyHome,
                  routes: [
                    GoRoute(
                      name: 'Home',
                      path: '/home',
                      pageBuilder: (BuildContext context, GoRouterState state) {
                        return const NoTransitionPage(
                            child: HomePage(), name: 'home');
                      },
                    ),
                  ]),
              StatefulShellBranch(
                  navigatorKey: _shellNavigatorKeyShellOne,
                  routes: [
                    GoRoute(
                      name: 'PageOne',
                      path: '/page-one',
                      pageBuilder: (BuildContext context, GoRouterState state) {
                        return const NoTransitionPage(
                            child: PageOne(), name: 'page1');
                      },
                  routes: [
                         //child page where it is not inside the shell, use _rootNavigatorKey
                          GoRoute(
                            parentNavigatorKey: _rootNavigatorKey,
                            path: 'account',
                            name: 'Account',
                            pageBuilder: (context, state) => NoTransitionPage(
                                child: AccountPage(), name: 'account'),
                          ),
                    ),
                  ]),])

ChopinDavid pushed a commit to wwt/flutter-packages that referenced this issue Dec 28, 2023
This pr refactor RouteMatchList to be a tree structure.

Added a common base class RouteMatchBase. It is extended by both RouteMatch and ShellRouteMatch.

The RouteMatch is for GoRoute, and is always a leaf node

The ShellRouteMatch is for ShellRouteBase, and is always and intermediate node with a list of child RouteMatchBase[s].

This pr also redo how push is processed. Will add a doc explain this shortly.

This is a breaking change, will write a migration guide soon.

fixes flutter/flutter#134524
fixes flutter/flutter#130406
fixes flutter/flutter#126365
fixes flutter/flutter#125752
fixes flutter/flutter#120791
fixes flutter/flutter#120665
fixes flutter/flutter#113001
fixes flutter/flutter#110512
@darshankawar darshankawar added the r: fixed Issue is closed as already fixed in a newer version label Jan 2, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
found in release: 3.4 Found to occur in 3.4 found in release: 3.5 Found to occur in 3.5 has reproducible steps The issue has been confirmed reproducible and is ready to work on p: go_router The go_router package P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. r: fixed Issue is closed as already fixed in a newer version team-go_router Owned by Go Router team triaged-go_router Triaged by Go Router team
Projects
No open projects
Status: No status
Development

Successfully merging a pull request may close this issue.

8 participants