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] Nested stateful navigation with ShellRoute #2650

Closed
wants to merge 159 commits into from

Conversation

tolo
Copy link

@tolo tolo commented Sep 27, 2022

Added functionality for building route configuration with support for preserving state in nested navigators. This change introduces a new shell route class called StatefulShellRoute, that uses separate navigators for its child routes as well as preserving state in each navigation branch. This is convenient when for instance implementing a UI with a BottomNavigationBar, with a persistent navigation state for each tab (i.e. building a Navigator for each tab).
An example showcasing a UI with BottomNavigationBar and StatefulShellRoute has also been added (stateful_shell_route.dart).

Other examples of using StatefulShellRoute are also available in these repositories:


Below is a short example of how a `StatefulShellRoute` can be setup:
StatefulShellRoute(
  /// Each separate stateful navigation tree (i.e. Navigator) is represented by
  /// a StatefulShellBranch, which defines the routes that will be placed on that
  /// Navigator. StatefulShellBranch also makes it possible to configure
  /// things like an (optional) Navigator key, the default location (i.e. the
  /// location the branch will be navigated to when loading it for the first time) etc.
  branches: <StatefulShellBranch>[
    StatefulShellBranch(navigatorKey: optionalNavigatorKey, routes: <RouteBase>[
      GoRoute(
        path: '/a',
        builder: (BuildContext context, GoRouterState state) =>
        const RootScreen(label: 'A'),
        routes: <RouteBase>[
          GoRoute(
            path: 'details',
            builder: (BuildContext context, GoRouterState state) =>
            const DetailsScreen(label: 'A'),
          ),
        ],
      ),
    ]),
    /// The default location of a branch will by default be the first of the
    /// configured routes. To configure a different route, provide the
    /// defaultLocation parameter.
    StatefulShellBranch(defaultLocation: '/b/detail', routes: <RouteBase>[
      GoRoute(
        path: '/b',
        builder: (BuildContext context, GoRouterState state) =>
        const RootScreen(label: 'B'),
        routes: <RouteBase>[
          GoRoute(
            path: 'details',
            builder: (BuildContext context, GoRouterState state) =>
            const DetailsScreen(label: 'B'),
          ),
        ],
      ),
    ]),
  ],
  /// Like ShellRoute, the builder builds the navigation shell around the
  /// sub-routes, but with StatefulShellRoute, this navigation shell is able to
  /// maintain the state of the Navigators for each branch. The navigation shell
  /// could for instance use a BottomNavigationBar or similar.
  builder: (BuildContext context, StatefulShellRouteState state, Widget child) =>
      ScaffoldWithNavBar(shellState: state, body: child),
)

This fixes issue flutter/flutter#99124.
It also (at least partially) addresses flutter/flutter#112267.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@tolo
Copy link
Author

tolo commented Sep 28, 2022

Sorry, I found some additional documentation that needed updating, and also decided to improve the example and add an additional unit test.

I was going to answer the question about the assert of the builder field on ShellRoute, but I didn't expect that comment to disappear after my new push (maybe I did something wrong).
Anyway, the assert that builder must be null when nestedNavigationBuilder is used is just because they are mutually exclusive (i.e. builder will not be used if nestedNavigationBuilder is set). It would just lead to confusion I you were to be allowed to use them simultaneously, since you would in effect be writing dead code in the passed builder function.

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this approach! It not only enables the preserve state functionality, but it also opens up a lot more opportunities for other fancy features. I left some suggestions on the some of the APIs. Let me know if you have questions.

packages/go_router/lib/src/route.dart Outdated Show resolved Hide resolved
packages/go_router/lib/src/route.dart Outdated Show resolved Hide resolved
packages/go_router/lib/src/route.dart Outdated Show resolved Hide resolved
packages/go_router/lib/src/route.dart Outdated Show resolved Hide resolved
@chunhtai
Copy link
Contributor

cc @johnpryan

@tolo
Copy link
Author

tolo commented Sep 30, 2022

I've refactored the implementation a bit now, and decided to implement the new behaviour outside ShellRoute, in a separate route class. Here are some of changes:

  • Created a common base class for routes providing a UI shell: ShellRouteBase.
  • Added PartitionedShellRoute - a shell route class that uses separate ("partitioned") navigators for its sub routes.
  • Added factory constructor stackedNavigation on PartitionedShellRoute for building a partitioned navigation shell based on an index stack, using new widget class StackedNavigationScaffold (see below).
  • Added StackedNavigationScaffold - A scaffold (or shell) that provides support for managing multiple navigators via an IndexStack, in a stateful way that responds to state changes. Using this removes the need for a lot of boilerplate when creating a UI with a BottomNavigationBar for instance.
  • Updated the stateful nested navigation example to use PartitionedShellRoute and StackedNavigationScaffold.

Have a look and see what you think.
Need to add some more documentation (and tests), but I wanted to hear what you thought about this implementation first.

Added (and fixed) animation when switching tabs.
…stedNavigationBuilder` on ShellRoute.

Added unit test.
Added reference to example in readme.
Conflicts:
packages/go_router/CHANGELOG.md
packages/go_router/pubspec.yaml
…navigators manually.

Renamed example file to stateful_nested_navigation.dart.
…ute class PartitionedShellRoute as well new base class (ShellRouteBase) shared with ShellRoute.

Introduced new widget (StackedNavigationScaffold) for building stacked, stateful navigation based on an IndexStack, as well as factory constructor (stackedNavigation) for this on PartitionedShellRoute.
…n't seem possible to use GoRouterState for this (after performing pop at least).

Made transitionDuration optional instead of using default value.
Minor clean up, assertion and doc updates.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 23, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 23, 2023
flutter/packages@83959fb...d449a17

2023-05-22 stuartmorgan@google.com [various] Remove unnecessary null checks (flutter/packages#4060)
2023-05-22 stuartmorgan@google.com [ci] Add a legacy Android build-all test (flutter/packages#4005)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from ab57304 to 3437189 (5 revisions) (flutter/packages#4062)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@Reprevise
Copy link

Are we going to see any integration with go_router_builder?

@chunhtai
Copy link
Contributor

filed flutter/flutter#127406

@tolo
Copy link
Author

tolo commented May 24, 2023

Are we going to see any integration with go_router_builder?

Yes, I was meaning to open an issue about this, but @chunhtai beat me to it 😄

@erdzan12
Copy link
Contributor

@tolo, do you think it would be possible to utilize the new StatefulShellRoute to prevent unnecessary rerenders on the main screen when navigating to other screens?

The current situation involves a main view that is heavy to render. This view allows the opening of bottom sheets, which are configured as routes in go_router. However, when a bottom sheet is pushed, the main view is rerendered, and the same occurs when the bottom sheet is closed. This process negatively impacts performance.

To address this, I considered using different branches, with the main view on one branch and the bottom sheets on separate branches. However, implementing this approach requires some hacky workarounds. For instance, closing the bottom sheet becomes problematic since it exists alone in the stack, and the content behind it appears as white, rendering the main view invisible.

@omar-hanafy
Copy link
Contributor

Hi @tolo , I'm really impressed with your work, and I'm grateful for your contributions.

I'm currently working on a new feature that requires me to push to a new screen globally and ignore the navigation bar. I've tried to create a GoRoute in the routes of GoRouter along with StatefulShellRoute.indexedStack, but when I use pushNamed to that screen, I expect the pushed screen to take the full screen and ignore the StatefulShellRoute. However, this doesn't seem to be happening.

Would you be able to take a look at this and let me know what I'm doing wrong?

Thank you for your time and consideration.

@sovetski
Copy link

sovetski commented May 24, 2023

Hi @omar-hanafy, maybe it will absolutely don't help for your problem, but just to be sure, did you try this solution?

https://stackoverflow.com/a/74998164/7186622

I think it can also work with StatefulShellRoute, you just need to play with "parentNavigatorKey" as explained on StackOverflow

|_ GoRoute
  |_ parentNavigatorKey = _parentKey   👈 Specify key here
|_ ShellRoute
  |_ GoRoute                            // Needs Bottom Navigation                  
    |_ parentNavigatorKey = _shellKey   
  |_ GoRoute                            // Needs Bottom Navigation
    |_ parentNavigatorKey = _shellKey   
|_ GoRoute                              // 👈 Full Screen which doesn't need Bottom Navigation
  |_parentNavigatorKey = _parentKey

@lokesh-5050
Copy link

I've been through it, you can create a new goRoute outside the Statefullshell route and push to that screen , though it is a temporary solution

@omar-hanafy
Copy link
Contributor

I think it can also work with StatefulShellRoute, you just need to play with _parentNavigatorKey.

using the _rootNavigatorKey of the GoRouter in each GoRoute's parentNavigatorKey worked 👍🏻, now I can choose to push globally and ignore the navigation bar. thanks @sovetski .

@omar-hanafy
Copy link
Contributor

omar-hanafy commented May 24, 2023

you can create a new goRoute outside the Statefullshell route and push to that screen.

I actually did the same but it did not work until I passed the root key to the GoRoute just like that:

 final _rootKey = GlobalKey<NavigatorState>(debugLabel: 'root');

 final GoRouter router = GoRouter(
   navigatorKey: _rootKey,
   initialLocation: AppRoute.home.routePath,
   routes: <RouteBase>[
     StatefulShellRoute.indexedStack(
       builder: (
         BuildContext context,
         GoRouterState state,
         StatefulNavigationShell ns,
       ) {
         return const MobileNavigationBar();
       },
       branches: <StatefulShellBranch>[
         // other branches with other keys
         ...
       ],
     ),
     // screen that I want to push globally and ignore the navigation bar
     GoRoute(
       // passing the same key of the GoRouter
       parentNavigatorKey: _rootKey,
       path: '/path',
       ...
     ),
   ],
 );

CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
flutter/packages@1e214d7...83959fb

2023-05-22 JeroenWeener@users.noreply.github.com [in_app_purchases] Fix mismatching method signature strings (flutter/packages#4040)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from 077d644 to ab57304 (18 revisions) (flutter/packages#4051)
2023-05-19 engine-flutter-autoroll@skia.org Roll Flutter from 5ae6438 to 077d644 (23 revisions) (flutter/packages#4043)
2023-05-19 stuartmorgan@google.com [local_auth] Migrate iOS to Pigeon (flutter/packages#3974)
2023-05-19 hamadyalghanim@gmail.com [go_router] fix context extension for replaceNamed (flutter/packages#3927)
2023-05-19 JeroenWeener@users.noreply.github.com [image_picker] Fix crash due to `SecurityException` (flutter/packages#4004)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
flutter/packages@83959fb...d449a17

2023-05-22 stuartmorgan@google.com [various] Remove unnecessary null checks (flutter/packages#4060)
2023-05-22 stuartmorgan@google.com [ci] Add a legacy Android build-all test (flutter/packages#4005)
2023-05-22 engine-flutter-autoroll@skia.org Roll Flutter from ab57304 to 3437189 (5 revisions) (flutter/packages#4062)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)
2023-05-22 tobias@leafnode.se [go_router] Nested stateful navigation with ShellRoute (flutter/packages#2650)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@rddewan
Copy link

rddewan commented May 25, 2023

This is cool feature , I have some doubt, how can we clear the state. Like when user navigate to Cart , I would like to call the api but currently it only call the api for the first time subsequent visit to Cart displays the cache state.
Any way to exclude it from stateful route or clear the state for specific route?

Screenshot_1685004167

@omar-hanafy
Copy link
Contributor

omar-hanafy commented May 25, 2023

Any way to exclude it from stateful route or clear the state for specific route?

@rddewan If I were you I would use visibility detector, and with a proper state management, we can refresh the cart api each time the cart page is visible.

Tip: you can use timer to prevent overcalls on the api.

@rddewan
Copy link

rddewan commented May 25, 2023

@omar-hanafy thank you for your recommendation. I was looking for build in feature , I hope this will be added in future

@Dohmanlechx
Copy link

Hi @tolo and everyone, I am migrating from using a regular IndexedStack as a hack solution into actually using StatefulShellRoute.indexedStack. So far so good but it looks like the widget inside of the StatefulShellBranch doesn't initiate until you actually press the related bottom navigation item.

This is a big problem for us because, in the previous approach, we could initiate all the screens immediately and hence load their UI etc, so the user doesn't notice any of the loadings since it happened when the user still was on the "Home" screen. Now, when the user presses the related bottom navigation item all the UI rendering starts and the user has to watch the app loading.

Any advice about how to solve this?

@hazzo
Copy link

hazzo commented May 26, 2023

Hi @tolo and everyone, I am migrating from using a regular IndexedStack as a hack solution into actually using StatefulShellRoute.indexedStack. So far so good but it looks like the widget inside of the StatefulShellBranch doesn't initiate until you actually press the related bottom navigation item.

This is a big problem for us because, in the previous approach, we could initiate all the screens immediately and hence load their UI etc, so the user doesn't notice any of the loadings since it happened when the user still was on the "Home" screen. Now, when the user presses the related bottom navigation item all the UI rendering starts and the user has to watch the app loading.

Any advice about how to solve this?

Here are my 2 cents. I think what you where using as a feature with the previous hack it's actually a "bug". Now you have granularity and that's better. If you don't want to make the first fetch on each tab change and you want "all the data at once" you could easily ask for it, cache it and use it on first navigation to each section. I think this is more an architecture and pattern problem than one of go_router. Yes you benefited of the previous implementation but a "normal" behavior would be not to retrieve a huge payload for a page/tab you might actually never see. You still can do it but you'll have to rethink your approach.

@jaween
Copy link

jaween commented May 26, 2023

Hi @tolo and everyone, I am migrating from using a regular IndexedStack as a hack solution into actually using StatefulShellRoute.indexedStack. So far so good but it looks like the widget inside of the StatefulShellBranch doesn't initiate until you actually press the related bottom navigation item.

This is a big problem for us because, in the previous approach, we could initiate all the screens immediately and hence load their UI etc, so the user doesn't notice any of the loadings since it happened when the user still was on the "Home" screen. Now, when the user presses the related bottom navigation item all the UI rendering starts and the user has to watch the app loading.

Any advice about how to solve this?

Preloading branches was removed from this PR, but see the last paragraph in #2650 (comment), @tolo has mentioned he plans to add it back in a subsequent PR.

@l1qu1d
Copy link

l1qu1d commented May 26, 2023

@chunhtai I believe this PR needs to be locked. This PR has been closed. Most conversations and feedback on here should be turned into a new discussion, PR request, or asked on something like StackOverflow. This way they can be tracked, organized, and be searchable.

@tolo
Copy link
Author

tolo commented May 28, 2023

First of all - sorry that I've not been able to respond to all of your questions here lately.

@l1qu1d, I agree, it's perhaps best to continue discussions and support around StatefulShellRoute somewhere else. Also, as I said before, I'm planning on writing together a a blog post where I will cover some of the different use cases and examples. And perhaps more importantly, flutter/flutter#127209 will hopefully make it easier to get started with StatefulShellRoute (plan on working on/contributing to that issue).

And of course, discussions related to things like preload can of course continue in the related issues and PRs (will file this soon).

@JDongKhan
Copy link

The parentNavigatorKey only be set _sectionANavigatorKey?

   StatefulShellBranch(
            navigatorKey: _sectionANavigatorKey,
            routes: <RouteBase>[
              GoRoute(
                // The screen to display as the root in the first tab of the
                // bottom navigation bar.
                path: '/a',
                parentNavigatorKey: _rootNavigatorKey,
                builder: (BuildContext context, GoRouterState state) =>
                    const RootScreen(label: 'A', detailsPath: '/a/details'),
                routes: <RouteBase>[
                  // The details screen to display stacked on navigator of the
                  // first tab. This will cover screen A but not the application
                  // shell (bottom navigation bar).
                  GoRoute(
                    path: 'details',
                    builder: (BuildContext context, GoRouterState state) =>
                        const DetailsScreen(label: 'A'),
                  ),
                ],
              ),
            ],
          ),

GoRoute(
path: '/a',
parentNavigatorKey: _rootNavigatorKey,
),

[ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: 'package:go_router/src/route.dart': Failed assertion: line 802 pos 18: 'route.parentNavigatorKey == null ||
E/flutter ( 5456):               route.parentNavigatorKey == branch.navigatorKey': is not true.
E/flutter ( 5456): #0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:51:61)
E/flutter ( 5456): #1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)
E/flutter ( 5456): #2      StatefulShellRoute._debugValidateParentNavigatorKeys (package:go_router/src/route.dart:802:18)
E/flutter ( 5456): #3      new StatefulShellRoute (package:go_router/src/route.dart:662:16)
E/flutter ( 5456): #4      new StatefulShellRoute.indexedStack (package:go_router/src/route.dart:681:8)
E/flutter ( 5456): #5      new NestedTabNavigationExampleApp (package:go_router_examples/stateful_shell_route.dart:31:26)
E/flutter ( 5456): #6      main (package:go_router_examples/stateful_shell_route.dart:19:10)
E/flutter ( 5456): #7      _runMain.<anonymous closure> (dart:ui/hooks.dart:131:23)
E/flutter ( 5456): #8      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:297:19)
E/flutter ( 5456): #9      _RawReceivePort._handleMessage (dart:isolate-patch/isolate_patch.dart:192:26)

assert(route.parentNavigatorKey == null ||
route.parentNavigatorKey == branch.navigatorKey);

@carlitosxx
Copy link

carlitosxx commented May 29, 2023

a question, which happens if I want to start from a GoRouter and not from a StatefulShellRoute.indexedStack, for example from a splash or login page and then derive to a sheelRoute, which code I write in a button to go to the page that has the bottomNavigatorVar.

final GoRouter appRouter = GoRouter(
  navigatorKey: _rootNavigatorKey,
  // initial route
  initialLocation: '/splash',

  routes: [
    GoRoute(path: '/splash', builder: (_, __) => const SplashPage()),
    GoRoute(
      path: '/company',
      name: 'company',
      builder: (_, __) => const CompanyPage(),
    ),
    GoRoute(
      path: '/login',
      name: 'login',
      builder: (_, __) => const LoginPage(),
    ),
    StatefulShellRoute.indexedStack(
      builder: (
        BuildContext context,
        GoRouterState state,
        StatefulNavigationShell navigationShell,
      ) {
        return HomePage(navigationShell: navigationShell);
      },
      branches: [
        StatefulShellBranch(
          navigatorKey: _sectionANavigatorKey,
          routes: [
            GoRoute(
              name: 'my_documents',
              path: '/my_documents',
              builder: (BuildContext context, GoRouterState state) =>
                  const MyDocumentsPhoneView(),
              // routes: [],
            ),
          ],
        ),
        StatefulShellBranch(
          routes: [
            GoRoute(
              path: '/validate_documents',
              builder: (BuildContext context, GoRouterState state) =>
                  const ValidateDocumentsPhoneView(),
            ),
          ],
        ),
        StatefulShellBranch(
          routes: [
            GoRoute(
              path: '/pending_documents',
              builder: (BuildContext context, GoRouterState state) =>
                  const PendingDocumentsPhoneView(),
            ),
          ],
        ),
      ],
    ),
  ],
);

@omar-hanafy
Copy link
Contributor

omar-hanafy commented May 29, 2023

which code I write in a button to go to the page that has the bottomNavigatorVar.

The first child path of the StatefulShellBranch.

context.go('/my_documents');

however, I recommend using redirections to handle session/token availability.

@chunhtai
Copy link
Contributor

I am locking this pr as resolved, please create a new issue if you would like to discussion future improvements

@flutter flutter locked as resolved and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet