Nested Routing sample code - Router

[johnpryan]

This example adds code for the Nested Routing scenario. The app displays a navigation bar, which is tied to the routes of the app.

[lulupointu]

Just 2 remark, following https://github.com/flutter/uxr/issues/35#issuecomment-811830740:

1. When navigating between Settings and Books, there is no animation. Since those are different routes we should expect a MaterialPageRoute transition I think
2. We should add another Navigator, here we have nested navigation but no nested navigator. Imagine that there is a login screen for example, you will need a Navigator wrapping the current one.

A few other remarks:

1. The url does not sync when switching tab
2. The tab displayed is not the right one if you type it in the url

Solution for 1:

_tabController.addListener(
  () => widget.onTabSelected(_tabController.index),
);

Inside initState

Solution for 2:

_tabController.index = widget.appPage == AppPage.newBooks ? 0 : 1;

Inside the didChangeDependencies (or inside build and in that case you can delete didUpdateWidget) Also you should add if (_page == s) return; inside the AppState.page setter, otherwise setState will be called inside build

Sorry to not suggest code change, I don't know if I am allowed to do so, so I prefer making suggestions here.

[xuanswe]

When navigating between Settings and Books, there is no animation. Since those are different routes we should expect a MaterialPageRoute transition I think

I agree that, switching tabs should have an animation. I did not run the code, but IMO, the animation when switching tabs should be managed by the TabBar and BottomNavigationBar. The navigators are children of TabBar and BottomNavigationBar, so navigators don't need to know about this transition. Navigator only manages pages inside it. TabBar and BottomNavigationBar manage switching between navigators.

According to material guideline,

A fade-through transition pattern is recommended for transitions between bottom navigation destinations.

Flutter should have default animation for tabs switching. This animation is not related to our navigation system.

[lulupointu]

Oh ok I agree that considering how this is implemented, we do not expect any animation. However if we had 2 different pages (instead of one with widget switching), don't you agree that, when switching between pages, people expect a page transition (which is customizable, throw a fade route transition if you want no problem there)? This is what would happen if this example did use 2 different pages and I think this is something worth testing (=page route transition for inner navigator)

[xuanswe]

However if we had 2 different pages (instead of one with widget switching), don't you agree that, when switching between pages, people expect a page transition (which is customizable, throw a fade route transition if you want no problem there)? This is what would happen if this example did use 2 different pages and I think this is something worth testing (=page route transition for inner navigator)

I am not sure if I understand your question. But here is my opinion: Basically, we we switching tabs, we are switching navigators => tabs should decide the right behaviors for themselves. When we move to next page inside a inner navigator, we are switching inner pages => navigator should decide how inner pages are animated.

You could run my example app to understand what I mean. You don't need to check the code, just run it as an end user before coming back to know the code.

[lulupointu]

I think we are saying the same thing !

What it comes down to in this example is to change this:

pages: [
  MaterialPage(
    key: ValueKey('BooksScreen'),
    child: AppScaffold(
      currentIndex: bottomBarIndex,
      onIndexChanged: (idx) {
        if (idx == 0) {
          _appState.page = AppPage.newBooks;
        } else {
          _appState.page = AppPage.settings;
        }
      },
      child: innerScreen,
    ),
  ),
],

to that:

pages: [
  (_appState.page == AppPage.settings) 
    ? MaterialPage(
        key: ValueKey('SettingsScreen'),
        child: AppScaffold(
          currentIndex: bottomBarIndex,
          onIndexChanged: (idx) {
            if (idx == 0) {
              _appState.page = AppPage.newBooks;
            } else {
              _appState.page = AppPage.settings;
            }
          },
          child: SettingsScreen(),
        ),
      )
    : MaterialPage(
        key: ValueKey('BooksScreen'),
        child: AppScaffold(
          currentIndex: bottomBarIndex,
          onIndexChanged: (idx) {
            if (idx == 0) {
              _appState.page = AppPage.newBooks;
            } else {
              _appState.page = AppPage.settings;
            }
          },
          child: BooksScreen(
            appPage: _appState.page,
            onTabSelected: _handleTabSelected,
          ),
        ),
      ),
],

and also ideally, as you mentioned, the animation should be a fade so MaterialPage should be changed to ScaffoldPage where:

class ScaffoldPage extends Page {
  @override
  final LocalKey key;
  final Widget child;

  ScaffoldPage({@required this.key, @required this.child});

  @override
  Route createRoute(BuildContext context) {
    return PageRouteBuilder(
      pageBuilder: (context, opacity, _) => FadeTransition(
        opacity: opacity,
        child: child,
      ),
    );
  }
}

[xuanswe]

pages: [
  (_appState.page == AppPage.settings) 
    ? MaterialPage(
        key: ValueKey('SettingsScreen'),
        child: AppScaffold(
          currentIndex: bottomBarIndex,
          onIndexChanged: (idx) {
            if (idx == 0) {
              _appState.page = AppPage.newBooks;
            } else {
              _appState.page = AppPage.settings;
            }
          },
          child: SettingsScreen(),
        ),
      )
    : MaterialPage(
        key: ValueKey('BooksScreen'),
        child: AppScaffold(
          currentIndex: bottomBarIndex,
          onIndexChanged: (idx) {
            if (idx == 0) {
              _appState.page = AppPage.newBooks;
            } else {
              _appState.page = AppPage.settings;
            }
          },
          child: BooksScreen(
            appPage: _appState.page,
            onTabSelected: _handleTabSelected,
          ),
        ),
      ),
],

I think I don't want this change because it animate the whole screen, including the tab bar. I would like to have the transition only inside the body of the scaffold.

For me, a nested route is only a part of the whole screen. Changing the content inside the nested route should animate only the part it manages.

If we have a separated team for the part of UI, this team shouldn't make the whole screen animated. They are allowed to animate only their part.

Let's say I have a team to develop just the review section in the product page on amazon. If I open next review page, only the review section is moving on the screen.

[lulupointu]

Oh yes sorry I did not read carefully enough, the Navigator should be moved inside the Scaffold. This is what we should have:

AppScaffold(
  currentIndex: bottomBarIndex,
  onIndexChanged: (idx) {
    if (idx == 0) {
      _appState.page = AppPage.newBooks;
    } else {
      _appState.page = AppPage.settings;
    }
  },
  child: Navigator(
    key: navigatorKey,
    pages: [
      (_appState.page == AppPage.settings)
          ? MaterialPage(
              key: ValueKey('SettingsScreen'),
              child: SettingsScreen(),
            )
          : MaterialPage(
              key: ValueKey('BooksScreen'),
              child: BooksScreen(
                appPage: _appState.page,
                onTabSelected: _handleTabSelected,
              ),
            ),
    ],
    onPopPage: (route, result) {
      return route.didPop(result);
    },
  ),
);

with MaterialPage changed to ScaffoldPage if we want the fade animation.

[xuanswe]

I think now we are on the same page, but your implementation is needed a fix for the error:

I think we need to wrap AppScaffold inside another Navigator.

[xuanswe]

    return Navigator(
      key: navigatorKey,
      pages: [
        MaterialPage(
          key: ValueKey('RootScreen'),
          child: AppScaffold(
            currentIndex: bottomBarIndex,
            onIndexChanged: (idx) {
              if (idx == 0) {
                _appState.page = AppPage.newBooks;
              } else {
                _appState.page = AppPage.settings;
              }
            },
            child: Navigator(
              pages: [
                (_appState.page == AppPage.settings)
                    ? MaterialPage(
                  key: ValueKey('SettingsScreen'),
                  child: SettingsScreen(),
                )
                    : MaterialPage(
                  key: ValueKey('BooksScreen'),
                  child: BooksScreen(
                    appPage: _appState.page,
                    onTabSelected: _handleTabSelected,
                  ),
                ),
              ],
              onPopPage: (route, result) {
                return route.didPop(result);
              },
            ),
          ),
        ),
      ],
      onPopPage: (route, result) {
        return route.didPop(result);
      },
    );

[lulupointu]

Yes wrapping it in a navigator works.

In any case, I was asking in https://github.com/flutter/uxr/pull/34#issuecomment-812012287 to have a higher level navigator, in order to be able to test nested navigator (and not just nested navigation). I hope this will be implemented since it would make sense here and is a common pattern.

[xuanswe]

• Do we need to keep state when switching back to the old tab in this scenario? Maybe create page with a listview with full content to show that the scroll position is preserved.
• And yes, agree with @lulupointu that we need a higher level navigator to test behavior when pop and press system back button?

[InMatrix]

The implemented UI seems to be out of sync from the storyboard. We should either change the storyboard or the implementation, so we can compare across implementations of the scenario in different APIs.

[johnpryan]

@chunhtai I was able to get the Scaffold widget above the Navigator in the RouterDelegate (4c66531) but now the app can't display the "All" tab when the user manually edits the route from /books/new to /books/all. If I use a MaterialPage instead of a custom FadeTransitionPage it works.

[johnpryan]

@chunhtai This should be ready for another look.

[johnpryan]

I want to call out a few interesting choices we made for this snippet:

• The app displays a fade transition between /books/* and /settings and the default TabBarView animation for the inner section displaying /books/all and /books/new.
• The UI is nested, but the routes are not. There is a single Router, RouteInformationParser, and RouterDelegate. This scenario doesn't specify how it should be implemented (https://github.com/flutter/uxr/issues/35), and using nested Routers / Navigators is probably too much code for our participants to look at anyway.
• There is only one Navigator. The BottomNavigationBar is above the Navigator in the widget hierarchy, and the TabBar + TabBarView are managed by the BooksScreen widget, which displays either /books/all or /books/new.
• There is only one RouteInformationParser. It doesn't consider how sections of the app are composed together. For example, the case where one team maintains a Dart library responsible for the /books/all and /books/new pages, which is imported and used by the 'parent' app (see https://github.com/flutter/uxr/issues/35). It's not clear to me that this the current API offers any affordances here...
• An Overlay is required to display a Scaffold above the Navigator returned by the RouterDelegate, which is difficult to do if you're a beginner or intermediate developer
• The FadeTransitionPage class needs some boilerplate code in order to display the correct tab in the TabBarView in BooksScreen (https://github.com/flutter/flutter/issues/80804)

[lulupointu]

@johnpryan I feel like not having 2 Navigators is an important loss regarding the usability of an API. Any app with a scaffold needs this. Moreover, it would even simplify your example since you would get rid of the overlay and use a navigator instead. If you want a code snippet for Navigator 2.0 using this I can propose some changes to your code if you want. In any case this should be considered I think

[chunhtai]

@lulupointu I agree that we should have a sample code that uses multiple routers/navigators, but in this example the top level navigator will only be acting as the overlay widget for the bottomAppbar, and may be confusing to other library author. I feel this sample is not suitable for showcasing the multiple routers scenario.

[slovnicki]

@johnpryan @chunhtai I just noticed that if I go to /books/all, change bottom tab to Settings and then back to Books, the TabBar and URL will be at /books/new and not at /books/all as when I left it.

There's a quick solution if this is not a desired behavior:

1. add AppPage _lastBookPage = AppPage.newBooks; to AppState
2. add this to page setter, before _page = s;

   if (_page != AppPage.settings) {
   _lastBookPage = _page;
   }

3. use this in bottomNavigationBar.onTap

   if (idx == 0) {
   _appState.page = _appState._lastBookPage;
   } 

[johnpryan]

I feel like not having 2 Navigators is an important loss regarding the usability of an API... If you want a code snippet for Navigator 2.0 using this I can propose some changes to your code if you want.

@lulupointu I'm open to that.

I feel this sample is not suitable for showcasing the multiple routers scenario.

@chunhtai what needs to change in the scenario to showcase this?

[chunhtai]

@johnpryan I imagine the second router/navigator should be inside the bookscreens page.

It makes sense to put in another router/navigator either of the following is true 1.the bookscreensAll to bookscreensNews wants a page transition instead of just a tab transition.

2. a sub team is working on the bookscreens page and they want to handle their own sub route parsing.

[lulupointu]

@johnpryan I agree with @chunhtai that this might not be the best scenario to test this. But then the question becomes, should we use this scenario at all even if it does not test nesting navigator (only nesting navigation).

To me a scenario with only "Login", "Settings", "Books" where "Settings" and "Books" are nested inside a Scaffold whereas the "Login" is not

[chunhtai]

@lulupointu To me this scenario showcase the ability to have static content (Scaffold) around the routing content. I find it tricky to do this using other routing libraries without duplicating a lot of widget code in every pages.

[slovnicki]

But then the question becomes, should we use this scenario at all even if it does not test nesting navigator (only nesting navigation).

Good point @lulupointu. We'll have to clarify is this scenario then "nested routing" or "nested navigation", although both terms are vague by themselves and would need a proper definition;

• "nested navigation" meaning that navigation is nested within some content not a part of navigation stack
• "nested routing" meaning that multiple routers or navigators are involved

And then also connect this with https://github.com/flutter/uxr/issues/35

[lulupointu]

@chunhtai I agree that the part with the Scaffold has to be kept. What I propose is:

1. remove the tabView
2. Add a routing between widgets which cover the entire page (such as a login screen and the entire Scaffold)

If you want to keep this scenario as is and create a new one no problem, I just want this aspect of nesting routing not to be forgotten since it is (according to my experience) one which is tricky for Flutter devs.

Indeed I created a serie of medium posts covering many aspects of vanilla Navigator 2.0 and the one on nested navigator is the most read by far!

[slovnicki]

I find it tricky to do this using other routing libraries without duplicating a lot of widget code in every pages.

@chunhtai I'm not sure what you mean. Something like this is done with no tricks or duplication in beamer. full code

basically

  final _routerDelegates = [
    BeamerRouterDelegate(
      locationBuilder: (state) => ArticlesLocation(state),
    ),
    BeamerRouterDelegate(
      locationBuilder: (state) => BooksLocation(state),
    ),
  ];

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: IndexedStack(
        index: _currentIndex,
        children: [
          Beamer(routerDelegate: _routerDelegates[0]),
          Container(
            color: Colors.blueAccent,
            padding: const EdgeInsets.all(32.0),
            child: Beamer(routerDelegate: _routerDelegates[1]),
          ),
        ],
      ),
      bottomNavigationBar: BottomNavigationBar(
        currentIndex: _currentIndex,
        items: [
          BottomNavigationBarItem(label: 'A', icon: Icon(Icons.article)),
          BottomNavigationBarItem(label: 'B', icon: Icon(Icons.book)),
        ],
        onTap: (index) {
          setState(() => _currentIndex = index);
          _routerDelegates[_currentIndex].parent?.updateRouteInformation(
                _routerDelegates[_currentIndex].currentLocation.state.uri,
              );
        },
      ),
    );
  }

[chunhtai]

@lulupointu Yes, that looks quite easy in beamer, and I wasn't aware it can be used this way.

I think this is a quite common use case where users may need, and your code sample is probably the thing user may want to see.

[slovnicki]

@chunhtai it was me who commented, not @lulupointu :)

And the example of using bottom bar with all the body content handled by a single Navigator is briefly described here with code and gif here

As we see, there are 2 use cases regarding bottom bar navigation that are equally important:

• each bottom bar tab building a separate Navigator
• all handled by a single Navigator

In latter case, as this PR is also demonstrating, we have to put extra care for transitions to make them look like something we expect. But this might not be what all users want.

Some users might want to have the same material transitions for body when switching between bottom tabs. On the other hand, most users will probably want to handle the switch between bottom tabs with some specialized Widgets like PageView, IndexedStack or AnimatedCrossFade.

I believe we should be making 2 scenarios for bottom bar navigation:

• single Navigator (this PR) with default transition for all pages
• each bottom tab building a separate Navigator

We could keep this scenario with all the transition tweaks done already, but I think it will be hard to replicate this with higher level package using a single Navigator and package authors will probably take the approach of multiple Navigators for this scenario. But then we're in a kind of situation where router sample and <package> sample are not really doing the same thing, just looking like they are. If this is OK and only the user's visual experience is important, then no problem.

[chunhtai]

@slovnicki sorry I pinned the wrong person. I did remember you are the person who commented, but still type the wrong user id.

I am confused at why each bottom nav bar need to build a different navigators. If it is just about different route transition between bottom tabs, one can write their own custom page class with different transition. building a navigator in every tab content does not seem to help?

[slovnicki]

@chunhtai I agree that this is a good implementation choice for this example as it stands, but I'm not sure how easy/natural would it be to handle this special animation between bottom bar tabs if each view has multiple pages stacked that want to use a default transition between them.

For example, if on Books tab we have a page stack of [books, book1] (navigation from books to book1 was with default transition) and we click Settings, then we would also want that special fade transition. Generally, we would want that special fade transition no matter what page stack was built (using default transitions) on Books tab. Then a Settings tab can also have a stack of pages and so on.

This feels like a task for multiple Navigators because we have multiple page stacks whose state we would like to remember. Indeed, that doesn't at all solve the task of transition between those Navigators, so I simply propose to use a Widget like PageView, or whatever is standard to use with bottom navigation bar.

But... Maybe I'm thinking about bottom bar navigation too far in the "unusual direction". I see that many apps never show a stack of pages and bottom navigation bar at the same time and have bottom bar just for switching between single pages that later on can create an entire stack over bottom navigation, i.e. we no longer see bottom navigation bar when new pages are put on stack. In that case, one Navigator and fade transition works great.

It seems that there are too much use cases than we can cover here in time.

[chunhtai]

@slovnicki ah I see, I think you were describing the first point of https://github.com/flutter/uxr/pull/34#issuecomment-823645639 Each tab has their own route stack inside the content (maybe due to custom transitions or maintaining the navigating state when switching between bottom tab). In that case we do need to have multiple routers/navigators.

I also agree there is too many use cases around this topic. I feel this PR looks good as it is, it may be too confusing when trying to show case too many things at once. I will leave the decision on how we can cover (or not cover) the other multiple routers use case to @johnpryan .

[slovnicki]

then LGTM also :)

[InMatrix]

Before we merge this PR, can we address this comment from @slovnicki? This is a behavior demonstrated in the storyboard for this scenario. Would having a second Navigator make preserving the tab selection easier?

I just noticed that if I go to /books/all, change bottom tab to Settings and then back to Books, the TabBar and URL will be at /books/new and not at /books/all as when I left it.

Another question I had was what it takes to change the UI implemented by this snippet to the one in the storyboard? Is it easy to change from the bottom navigation bar to a left menu when the user switch from mobile to desktop, for example?

As a general note, I don't feel we should first decide we need to use multiple Navigators in the snippet and then look for a scenario to justify that. Inclusion of additional scenarios in this research should be justified by how common that scenario is in app UI (with special consideration given to web apps). It would be helpful if someone can point to some real-world examples where the single-navigator approach in this snippet wouldn't work or wouldn't be the easiest solution.

...I think it will be hard to replicate this with higher level package using a single Navigator and package authors will probably take the approach of multiple Navigators for this scenario. But then we're in a kind of situation where router sample and sample are not really doing the same thing, just looking like they are. If this is OK and only the user's visual experience is important, then no problem.

I think it's not a problem that packages implement the same scenario in different ways, as long as the implementation represents the best way to do it with that package. IMHO, the app developer's primary goal is to make their app work according to its spec, not to follow a pre-defined implementation method.

[johnpryan]

Is it easy to change from the bottom navigation bar to a left menu when the user switch from mobile to desktop, for example?

I think the recommended approach here is to use the adaptive_navigation package to switch between a left navigation menu and bottom navigation bar depending on the size of the screen. I'm not sure we want introduce a package dependency, though.

[johnpryan]

@chunhtai @slovnicki I don't think I'm going to add a fix for https://github.com/flutter/uxr/pull/34#issuecomment-823623869 - switching from /settings to /books/* using the bottom navigation bar could display the previously viewed sub-page, but it's not clear this is the correct thing to do. For example, if I'm viewing a GitHub pull request and I switch to the the Commits tab, I don't expect the commits tab to be selected the next time I visit the URL for this pull request.

The browser back button will always show the previously visited tab (/books/new or /books/all) so I think this is working as intended.

[chunhtai]

if I'm viewing a GitHub pull request and I switch to the the Commits tab, I don't expect the commits tab to be selected the next time I visit the URL for this pull request.

If you visit the page using URL, the app should update the state. for example you can only type /book/new or /book/all, and it will only lead you to new or all. The comment https://github.com/flutter/uxr/pull/34#issuecomment-823623869 should only apply when you click on tab to do in app navigation. It is up to you whether you think this is a requirement though.

[johnpryan]

If you visit the page using URL, the app should update the state.

The AppState stores the current page. When you change the URL the current page changes from newBooks to allBooks.

The comment #34 (comment) should only apply when you click on tab to do in app navigation.

This also happens when you change the URL manually. The comment says that going to /books/all and then to /settings, and then selecting the books tab will show /books/new, and not /books/all because the app state only stores the current page, not the previously visited subpage of the /books bottom bar item. This is true whether or not you are using the bottom navigation bar or manually editing the URL. (changing form /settings to books in the URL bar will always show /books/new since there is no state to store the previously visited sub-page of the /books URL)

The storyboard describes preserving the state (books/new or /books/all in this case) when the browser back button is pressed, which is why I say this is working as intended.

[slovnicki]

@johnpryan I didn't mean "or URL", just via bottom bar tabs.

When we navigate via URL, we obviously say new or all, but we just say books when tapping on Books bottom bar tab. (I'm not describing code, just user actions). It's a different use case than URL navigation and arbitrary decision can be made whether it will preserve previously selected tab.

Taking in consideration how little code change is needed for this to work (as @chunhtai and I showed in some snippets) I think it's better to do it than not, but if it's not in requirements, it's definitely good either way.

[johnpryan]

It's a different use case than URL navigation and arbitrary decision can be made whether it will preserve previously selected tab.

I meant to say that the app state and URL bar are synchronized via the RouterDelegate, so your comment applies to URL navigation and navigation via the user interacting with the bottom app bar.

Taking in consideration how little code change is needed for this to work I think it's better to do it than not

It's not about how much code it would take to implement (although adding more code unrelated to the scenario could create confusion for participants in the study.) but just a matter of how we should interpret the scenario. If we interpret the scenario such that we need to store the previous page in user code, this becomes more of a state management exercise, which isn't the focus of this study in my opinion.

[Milad-Akarie]

@chunhtai let's say I have a nested maps that looks something like this

/articles
/books 
      /all
      /new

if I'm at /books/all and type in /books/new in the browser's bar. my root page is still '/books' and page.canUpdate returns true, how do I rebuild the children with the sub-path?

[slovnicki]

If we interpret the scenario such that we need to store the previous page in user code, this becomes more of a state management exercise, which isn't the focus of this study in my opinion

Valid point.

[chunhtai]

@Milad-Akarie When page canUpdate return true, the Route.changedInternalState which will cause a rebuild on the ModalRoute.buildPage. This is where thing gets tricky, the content returns from the buildPage need to either come from app state higher up or comes from the Page (i.e. Route.settings). This makes things a bit hard to write, that is why we create MaterialPage and CupertinoPage to handle that, and also https://github.com/flutter/flutter/issues/80804.

I think we should fix this and make it easier to write custom page but I don't have anything concrete now

[Milad-Akarie]

@chunhtai How do I tell my Route that my settings (MyPage) has changed? will overriding MyPage's equality operator do the trick?

[chunhtai]

@Milad-Akarie The settings will change right after page.canUpdate return true and before the buildPage is called. Lets not spam people on this pr. If you have more question, feel free to ping me on discord. my discord id is the same as the github one

[andrefedev]

@chunhtai I have a question, how could we keep the status between items in a bottom navigation bar. I can't find a clean way to implement IndexedStack, PageStorage, or PageView.

[lulupointu]

@andrefedev You can use IndexedStack and have 1 navigator per page.

This is quite verbose but does work.

[andrefedev]

@lulupointu Do you have or know of any implementation that I can look at and learn about?

[chunhtai]

This is a relatively old pr, lets move the discussion to discord, feel free to ping me there.