lulupointu / vrouter

MIT License
202 stars 38 forks source link

Preserve stack when switching between nested routes #32

Closed smkhalsa closed 3 years ago

smkhalsa commented 3 years ago

My app uses tabbed navigation, in which each tab maintains its own independent stack. When switching between tabs, the route stack should be preserved for the given tab.

Here is what I have so far. However, I can't figure out how to get each tab to maintain its route stack when switching back.

Any guidance would be much appreciated.

  final _tabRoutes = [
    VWidget(path: '/kriyas', widget: AllKriyasScreen()),
    VWidget(path: '/kriyas/:kriyaId', widget: KriyaDetailsScreen('')),
  ];

return VRouter(
      theme: theme,
      title: 'Kundalini',
      initialUrl: '/practice',
      routes: [
        VNester(
          path: null,
          widgetBuilder: (child) => AppTabsScaffold(child: child),
          nestedRoutes: [
            VWidget(
              widget: PracticeScreen(),
              path: '/practice',
              stackedRoutes: _tabRoutes,
            ),
            VWidget(
              widget: ListenScreen(),
              path: '/listen',
              stackedRoutes: _tabRoutes,
            ),
            VWidget(
              widget: LearnScreen(),
              path: '/learn',
              stackedRoutes: _tabRoutes,
            ),
          ],
        )
      ],
    )

Where AppTabScaffold is

class AppTabsScaffold extends StatelessWidget {
  final Widget child;

  AppTabsScaffold({required this.child});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: SafeArea(
        top: false,
        child: child,
      ),
      bottomNavigationBar: Material(
        elevation: 8.0,
        child: Column(
          mainAxisSize: MainAxisSize.min,
          children: <Widget>[
            MiniPlayer(),
            BottomNavigationBar(
                type: BottomNavigationBarType.fixed,
                currentIndex: 0,
                onTap: (index) {
                  switch (index) {
                    case 0:
                      return context.vRouter.push('/practice');
                    case 1:
                      return context.vRouter.push('/listen');
                    case 2:
                      return context.vRouter.push('/learn');
                  }
                },
                items: [
                  BottomNavigationBarItem(
                    icon: Icon(MdiIcons.yoga),
                    backgroundColor: Theme.of(context).primaryColor,
                    label: 'Practice',
                  ),
                  BottomNavigationBarItem(
                    icon: Icon(Icons.music_note_rounded),
                    backgroundColor: Theme.of(context).primaryColor,
                    label: 'Listen',
                  ),
                  BottomNavigationBarItem(
                    icon: Icon(Icons.menu_book_rounded),
                    backgroundColor: Theme.of(context).primaryColor,
                    label: 'Learn',
                  ),
                ]),
          ],
        ),
      ),
    );
  }
}
lulupointu commented 3 years ago

When you say "maintain its route stack when switching back." you mean that when you tap the bottom navigation bar, the pushed path should be different depending on where you left the tab you are going to is that right?

Also could you tell me if you are targeting the web or not at all?

smkhalsa commented 3 years ago

When you say "maintain its route stack when switching back." you mean that when you tap the bottom navigation bar, the pushed path should be different depending on where you left the tab you are going to is that right?

Yes

Also could you tell me if you are targeting the web or not at all?

No. I know that this pattern (i.e. multiple parallel stacks) is very common on mobile but doesn't really fit with browser navigation.

I should also mention that my tabs need to be able to share some routes (i.e. you can reach the same screen from multiple tabs).

lulupointu commented 3 years ago

Do you mean that "AllKriyasScreen" and "KriyaDetailsScreen" are the same for the three tabs?

Let's ignore "KriyaDetailsScreen"and talk about "AllKriyasScreen" since both are the same navigation-wise. Basically what you want to do is to be able to have a stack Tab1 -> AllKriyasScreen And change only the bottom widget for example when you tap on Tab2 you will have Tab2->AllKriyasScreen

Sorry for all the question but I would rather understand what you want than showing you 10 different methods which don't meet your needs

smkhalsa commented 3 years ago

Do you mean that "AllKriyasScreen" and "KriyaDetailsScreen" are the same for the three tabs?

Yes. These are just two screens, but my actual app has >50, any of which could be pushed onto the stack of any tab. The primary difference between the tabs is the starting screen. For example, my "practice" tab will start on the "PracticeScreen", "listen" tab will start on the "ListenScreen" screen, and "learn" will start on the "LearnScreen".

So, let's assume we have an "AlbumDetailsScreen". This is most likely to be pushed onto the "listen" tab stack since the starting "ListenScreen" has more navigation paths that lead to "AlbumDetailsScreen". However, the user might be practicing an exercise on the "practice" tab and tap into a linked album, in which case it would push the "AlbumDetailsScreen" onto the "practice" tab's stack.

Let's ignore "KriyaDetailsScreen"and talk about "AllKriyasScreen" since both are the same navigation-wise. Basically what you want to do is to be able to have a stack Tab1 -> AllKriyasScreen And change only the bottom widget for example when you tap on Tab2 you will have Tab2->AllKriyasScreen

No. Each tab should have an independent stack. However, the same route could be pushed onto any of the tabs. For example, my app might have the following route stack state:

practice tab's stack PracticeScreen -> AllKriyasScreen listen tab's stack ListenScreen -> AllAlbumsScreen -> AlbumDetailsScreen learn tab's stack LearnScreen

Assuming I'm on the practice tab, tapping the listen tab would show the AlbumDetailsScreen. If I were to then tap the back button, AlbumDetailsScreen would be popped off the listen tab, and I'd see the AllAlbumsScreen. If I then tap the practice tab, I'd see the AllKriyasScreen again (i.e. the top of the practice stack).

Sorry for all the question but I would rather understand what you want than showing you 10 different methods which don't meet your needs

No worries. I appreciate the assistance.

lulupointu commented 3 years ago

Ok so I think I found a solution. I don't have access to my computer until Monday so I'll try to write down my idea but code snippets will be briefs.

The idea is to have a map inside the class containing your VRouter. This map will keep track of were you are in each one of the three stack, and will be used in your Scaffold.onTap.

Here is the initialisation of the map:

var tabStack = {
  'practice': '/practice', 
  'listen' : '/listen', 
  'learn' : '/learn', 
} 

To keep the stack in sync,yyou can you three VGuard, one for each routes:

VNester(
  path: null,
  widgetBuilder: (child) => AppTabsScaffold(child: child,  tabStack: tabStack),
    nestedRoutes: [
      VGuard(
        afterEnter: (_, __, to) => tabStack['profile'] = to;
        afterUpdate: (_, __, to) => tabStack['profile'] = to;
        VWidget(
          widget: PracticeScreen(),
          path: '/practice',
          stackedRoutes: _tabRoutes,
        ),
     ), 
    // same for listen
    // same for learn
  ],
) 

Also note that I give tabStack to AppTabsScaffold.

And then when you press tap on the bottom navigation bar:

onTap: (index) {
  switch (index) {
    case 0:
      return context.vRouter.push(tabStack['profile']) 
    // same for 1 and 2
  }
},

Also note that your routes in _tabRoutes the paths should be relative. This is a pain (especially for large project). I am trying to solve this issue by allowing nested VRouter, however this will not be ready before quite some time because this is a huge task that require a lot of thought.

Anyway hope this helps!

theweiweiway commented 3 years ago

Hey Lulu, would this method be able to preserve state between each stack as well? It seems like it's only remembering the location of each stack, not the state of each stack

lulupointu commented 3 years ago

Yes this would only remember the location but I think this is what @smkhalsa wants.

Remembering the state is not the job of VRouter I think, this can be achieve in (at least) two ways though:

What is comes down to is what the needs are. Looking at this very simple example I would use indexed stack but maybe the is not enough

theweiweiway commented 3 years ago

OK, good to know - yes I agree, I think indexed stack would be the best way to accomplish that

esDotDev commented 3 years ago

I think this is true in general (the state should be hoisted above the view), but is there really no way we could just signal that we want to retain state for a route?

This would be quite nice for things like TextFields and ScrollingLists, but also allows you to use StatefulWidget to store state if you want.

Consider a simple example like this, it would be quite nice to maintainState somehow in the built widgets:


class SimpleRouterTest extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return VRouter(
      initialUrl: "/home1",
      routes: [
        VWidget(path: "/home1", widget: _HomeWidget(linkPath: "/home2")),
        VWidget(path: "/home2", widget: _HomeWidget(linkPath: "/home1")),
      ],
    );
  }
}

class _HomeWidget extends StatefulWidget {
  const _HomeWidget({Key? key, required this.linkPath}) : super(key: key);
  final String linkPath;

  @override
  _HomeWidgetState createState() => _HomeWidgetState();
}

class _HomeWidgetState extends State<_HomeWidget> {
  List<String>? tweets;
  @override
  void initState() {
    super.initState();
    Future.delayed(Duration(seconds: 1), () => setState(() => tweets = List.generate(100, (index) => "Tweet$index")));
  }

  @override
  Widget build(BuildContext context) {
    return Center(
        child: tweets == null
            ? CircularProgressIndicator()
            : GestureDetector(
                onTap: () => context.vRouter.push(widget.linkPath),
                child: ListView.builder(itemBuilder: (_, index) => Text(tweets![index]))));
  }
}

As you switch between the routes, it is always re-running initState, having these routes be able to preserve state would be very useful.

lulupointu commented 3 years ago

The question of state handling is interesting and gave me a lot of thoughts.

Let my first give you two links showing what the React team is thinking about this:

This does not mean that they are right, but they sure partially guided me away from doing it.

Also when I think about state restoration I think about 2 different scenarios:

  1. A widget has some state when it is left, this state should be restored if it appears ever again
  2. Some information relative to the current route should be stored and restored if the user were to come back to that route using the browser back button

1. A widget has some state when it is left, this state should be restored if it appears ever again

This is the case of your example. As you perfectly illustrated, this is often use with application fetch data. In this case as you said this is more the job of a state management library.

In some cases, those situations are said to be common enough that flutter has built in state restoration mechanism, such as IndexedStack or AutomaticKeepAliveClientMixin. No need to create a new thing here.

2. Some information relative to the current route should be stored and restored if the user were to come back to that route using the browser back button

I think this is something really interesting in terms of usx which often gets overlooked, and is really hard to solve without the help of vrouter. This is why I did build something here contrary to point number (1) were a lot of solutions exists.

This situation arises in github for example. See those situations:

This is exactly what historyState is here for, and on the vrouter example of pub.dev you can see that this is shown for a counter. However this could be applied for scrolling, a form, ...

esDotDev commented 3 years ago

The various restoration are definitely cool, and necessary, but also by their nature very cumbersome and lots of work to implement right and test.

I was thinking of something simpler, where routes are simply maintained in memory and not rebuilt. If VRouter is somehow able to mark routes in memory, and rather than rebuilding the route, pass the previous instance, there would be no restoration needed and we could bring a really slick app paradigm over to the web.

React can't really do this as they are still bound tightly to the single-web page paradigm, but in Flutter we have a runtime, and we can stash things in ram or hide entire routes off-screen if we wanted.

As a point of comparison here, you could compare twitter.com and the Twitter App on Android.

I don't quite understand the suggestions to use Indexed stack for complicated routing needs, are you suggesting all the routes collapse down to one? So instead of having a /home, /explore/, /settings, we have simply /?tab=home which is basically a persistent route that is never removed?

lulupointu commented 3 years ago

I understand what you want and why you think this might be included in VRouter. I disagree for now but I am still experimenting with what is possible, but so far I cannot even find if what you ask is possible. I have experimented with Visibility.maintainState (which is basically an Offstage), and with AutomaticKeepAliveClientMixin (which is only for ListView I think but I tried anyway) but I could not get anything working.

If you have any idea, maybe show me an example without VRouter I would be very interested.

In any case, even if this could work, I am still not sure this should be part of VRouter. I think this should be a Widget/State issue, not Navigation. Here is a small example using IndexedStack of what I mean. There is no animation since the IndexedStack should handle it here but there you go:

import 'package:flutter/material.dart';
import 'package:vrouter/vrouter.dart';

main() {
  runApp(SimpleRouterTest());
}

class SimpleRouterTest extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return VRouter(
      initialUrl: "/home",
      routes: [
        VNester(
          path: null,
          widgetBuilder: (child) => MyScaffold(child: child),
          nestedRoutes: [
            VWidget(
              path: '/home',
              widget: HomeScreen(),
            ),
            VWidget(
              path: '/settings',
              widget: SettingsScreen(),
            ),
          ],
        ),
      ],
    );
  }
}

class MyScaffold extends StatelessWidget {
  final Widget child;

  const MyScaffold({Key? key, required this.child}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    final currentIndex = (context.vRouter.url == '/home') ? 0 : 1;
    return Scaffold(
      body: IndexedStack(
        children: [
          HomeScreen(),
          SettingsScreen(),
        ],
        index: currentIndex,
      ),
      bottomNavigationBar: BottomNavigationBar(
        onTap: (index) => context.vRouter.push((index == 0) ? '/home' : '/settings'),
        currentIndex: currentIndex,
        items: [
          BottomNavigationBarItem(icon: Icon(Icons.home), label: 'Home'),
          BottomNavigationBarItem(icon: Icon(Icons.settings), label: 'Settings'),
        ],
      ),
    );
  }
}

class SettingsScreen extends StatelessWidget {
  @override
  Widget build(BuildContext context) {
    return Column(
      mainAxisAlignment: MainAxisAlignment.center,
      children: [
        Text('Type and navigate to see the state of the TextField be preserved'),
        TextField(),
      ],
    );
  }
}

class HomeScreen extends StatelessWidget {
  final colors = [
    Colors.transparent,
    Colors.redAccent,
    Colors.greenAccent,
    Colors.lightBlueAccent,
    Colors.amberAccent,
  ];

  @override
  Widget build(BuildContext context) {
    return Stack(
      children: [
        Positioned.fill(
          child: ListView.builder(
              itemBuilder: (_, index) =>
                  Container(
                    color: colors[index % colors.length],
                    height: 50,
                  )
          ),
        ),
        Align(
          alignment: Alignment.topCenter,
          child: Text('Scroll and navigate to see the state of the ListView be preserved'),
        ),
      ],
    );
  }
}
smkhalsa commented 3 years ago

I believe the MaterialPage maintains its state automatically, so long as the page is retained in memory.

lulupointu commented 3 years ago

And how can you make it retained in memory. Flutter disposes of its children as soon as they are not in the widget tree I think, same goes for MaterialPage

smkhalsa commented 3 years ago

And how can you make it retained in memory. Flutter disposes of its children as soon as they are not in the widget tree I think, same goes for MaterialPage

Pages are not Widgets. You could probably just create the page outside of a Widget build method (e.g. in the initState of a StatefulWidget).

lulupointu commented 3 years ago

In any case, even if MaterialPage retain its state, it does not retain its widget state does it ? If so could you provide a small snippet showcasing this ? (using vanilla Navigator, even Navigator 1.0 should work)

esDotDev commented 3 years ago

I understand what you want and why you think this might be included in VRouter. I disagree for now but I am still experimenting with what is possible, but so far I cannot even find if what you ask is possible.

I think it has to be the responsibility of the routers, who else could cache these routes for us but the Router? Otherwise we just end up re-writing our own router-like sub-tree, relagating the VRouter to just passing around paths and query strings.

That doesn't mean it's easy though, I know Flutter probably makes this way harder than it needs to be. This inability to save state unless something is on the Tree is such a frustrating limitation of the framework :/

I'll try and write some demo code today, but high level I think it needs to work something like:

Here is a small example using IndexedStack of what I mean. There is no animation since the IndexedStack should handle it here but there you go.

Unless I'm missing something, in this example, the children passed into your routes are never used? VRouter is basically just taking a round about way of passing us the current path args so we can essentially route the path internally. This would be quite confusing in practice as you're declaring HomeView() twice, one of them is never used, almost easier to just use onGenerateRoute with a single Page at this point.

lulupointu commented 3 years ago

I'll try and write some demo code today

I'm looking forward to that!

Unless I'm missing something, in this example, the children passed into your routes are never used? VRouter is basically just taking a round about way of passing us the current path args so we can essentially route the path internally. This would be quite confusing in practice as you're declaring HomeView() twice, one of them is never used, almost easier to just use onGenerateRoute with a single Page at this point.

True, this is useful for simple issues, for example if you only need the state to be persistent in a scaffold. I agree that there are many more use cases that are way to complex to use that. If it's too complex it means that you have to use a state management library I think.

theweiweiway commented 3 years ago

Having an offstage or indexed stack to render multiple navigators in parallel has been a widely-used solution for persisting state, all the way back to when flutter was just starting to grow. A lot of articles and discussion on this since it's a pretty common use-case for mobile apps:

https://medium.com/flutter/getting-to-the-bottom-of-navigation-in-flutter-b3e440b9386 https://medium.com/coding-with-flutter/flutter-case-study-multiple-navigators-with-bottomnavigationbar-90eb6caa6dbf

I feel like IndexedStack works great - why can't it scale for more complex cases? The router is the one preserving the state.

esDotDev commented 3 years ago

But again, it's not about the shared state, it's everything else. Hoisting the state above my views, and having each view grab the latest data is pretty trivial.

But things like remembering scroll position is a headache, and other small things like the contents of TextInputs or maybe an intro animation we don't want running again and again.

I do see what you mean though, that solving these low level problems might be the lesser of 2 evils.

I wonder if the new restoration API can be of any help here.

esDotDev commented 3 years ago

Ok, so here's a proof of concept "Persistent Router", I have not looked at your internal code, this is just to illustrate that the core concept is not really far fetched.

Full code below, but the core of it is:

Widget build(BuildContext context) {
    /// Try and find a known route for the current path
    RouteConfig matchingRoute = List<RouteConfig>.from(widget.routes)
        .firstWhere((element) => element.path == widget.currentPath, orElse: () => null);
    if (matchingRoute == null) matchingRoute = pageNotFoundRoute;

    // Remove any known routes that are not persistent
    knownRoutes.removeWhere((key, value) => value.maintainState == false);

    // Add the new route to our list of known routes 
    knownRoutes[matchingRoute.path] = matchingRoute;

    // Pass all known pages to IndexedStack, but only render the current one
    List<RouteConfig> allRoutes = knownRoutes.values.toList();
    int currentIndex = allRoutes.indexWhere((r) => r.path == matchingRoute.path);
    return IndexedStack(
      index: currentIndex,
      children: allRoutes.map((r) => r.page).toList(),
    );
  }

https://user-images.githubusercontent.com/736973/114152425-89144980-98db-11eb-8385-b4d10f698ff4.mp4

class PersistentRouterDemo extends StatefulWidget {
  static PageInfo info = PageInfo(title: "Persistent Router");
  @override
  _PersistentRouterDemoState createState() => _PersistentRouterDemoState();
}

class _PersistentRouterDemoState extends State<PersistentRouterDemo> {
  String url = "page1";
  void setUrl(String value) => setState(() => url = value);
  @override
  Widget build(BuildContext context) {
    return Column(
      children: [
        Row(
          children: [
            TextButton(onPressed: () => setUrl("page1"), child: Text("Page1")),
            TextButton(onPressed: () => setUrl("page2"), child: Text("Page2")),
            TextButton(onPressed: () => setUrl("page3"), child: Text("Page3")),
            TextButton(onPressed: () => setUrl("bad link"), child: Text("bad link")),
          ],
        ),
        Expanded(
            child: PersistentRouter(currentPath: url, routes: [
          RouteConfig(path: "page1", page: SomeStatefulPage("Page1"), maintainState: true),
          RouteConfig(path: "page2", page: SomeStatefulPage("Page2"), maintainState: true),
          RouteConfig(path: "page3", page: SomeStatefulPage("Page3")),
        ]))
      ],
    );
  }
}

class RouteConfig {
  RouteConfig({@required this.path, @required this.page, this.maintainState = false});
  final String path;
  final Widget page;
  final bool maintainState;
}

class PersistentRouter extends StatefulWidget {
  final String currentPath;
  final List<RouteConfig> routes;

  const PersistentRouter({Key key, @required this.currentPath, @required this.routes}) : super(key: key);
  @override
  _PersistentRouterState createState() => _PersistentRouterState();
}

class _PersistentRouterState extends State<PersistentRouter> {
  Map<String, RouteConfig> knownRoutes = {};
  RouteConfig pageNotFoundRoute = RouteConfig(path: "404", page: Center(child: Text("404")));

  @override
  Widget build(BuildContext context) {
    /// Try and find a known route for the current path
    RouteConfig matchingRoute = List<RouteConfig>.from(widget.routes)
        .firstWhere((element) => element.path == widget.currentPath, orElse: () => null);
    if (matchingRoute == null) matchingRoute = pageNotFoundRoute;

    // Remove any known routes that are not persistent
    knownRoutes.removeWhere((key, value) => value.maintainState == false);

    // Add the new route to our list of known routes 
    knownRoutes[matchingRoute.path] = matchingRoute;

    // Pass all known pages to IndexedStack, but only render the current one
    List<RouteConfig> allRoutes = knownRoutes.values.toList();
    int currentIndex = allRoutes.indexWhere((r) => r.path == matchingRoute.path);
    return IndexedStack(
      index: currentIndex,
      children: allRoutes.map((r) => r.page).toList(),
    );
  }
}

class SomeStatefulPage extends StatefulWidget {
  const SomeStatefulPage(this.title, {Key key}) : super(key: key);
  final String title;

  @override
  _SomeStatefulPageState createState() => _SomeStatefulPageState();
}

class _SomeStatefulPageState extends State<SomeStatefulPage> {
  List<int> items;
  @override
  void initState() {
    super.initState();
    Future.delayed(Duration(seconds: 1), () => setState(() => items = List.generate(100, (index) => index)));
  }

  @override
  Widget build(BuildContext context) {
    if (items == null) return Center(child: CircularProgressIndicator());
    return Column(
      children: [
        Text(widget.title, style: TextStyle(fontSize: 40)),
        TextField(),
        Expanded(child: ListView.builder(itemBuilder: (_, index) => Text("ITEM: $index"))),
      ],
    );
  }
}
esDotDev commented 3 years ago

Also I was wrong about indexed stack, it is a little smarter than just moving them offscreen, but at it's core it's extremely simple:

@override
  void paintStack(PaintingContext context, Offset offset) {
    if (firstChild == null || index == null)
      return;
    final RenderBox child = _childAtIndex();
    final StackParentData childParentData = child.parentData! as StackParentData;
    context.paintChild(child, childParentData.offset + offset);
  }

Probably wouldn't be too hard to implement an AnimatedIndexedStack that could show the previousIndex as it transitioned out.

esDotDev commented 3 years ago

Another proof of concept I've been playing with, it extends on the idea above, creating a "PathedStack", this is basically an indexed stack, that takes a path and a list of entries (builders), additionally it wraps a key around each entry, using ValueKey('path'), something like: https://gist.github.com/esDotDev/09b0cb9fe2604c44b1d5a642d5a9ac29

Using that, I can make nested stacks, that pretty much work as you'd expect with regards to nested routing and maintainState:

return MainScaffold( // Main scaffold has the first row of tab btns
  child: PathStack(
    currentPath: currentPath,
    entries: [
      PathStackEntry(path: "home", builder: (_) => SomeStatefulPage("HOME")),
      PathStackEntry(
        path: "settings/",
        builder: (_) => SettingsScaffold( // Setting scaffold has a nested set of tab menus
          child: PathStack(
            parentPath: "settings/",
            currentPath: currentPath,
            entries: [
              PathStackEntry(
                path: "page1",
                builder: (_) => SomeStatefulPage("page1 ${Random().nextInt(999)}", key: ValueKey(0)),
              ),
              PathStackEntry(
                path: "page2",
                builder: (_) => SomeStatefulPage("page2 ${Random().nextInt(999)}", key: ValueKey(1)),
              ),
            ],
          ),
        ),
      ),
    ],
  ),
);

This gives us the effective site map:

/home
/settings/page1
/settings/page2

Where all pages are persistent, and maintain state, but can also take new args (as the Random().nextInt) shows.

https://user-images.githubusercontent.com/736973/114322084-9566f500-9adb-11eb-9ff4-d45b027005dc.mp4

I realize this is heading in quite a different direction than VRouter currently, but might inspire some more thoughts.

lulupointu commented 3 years ago

So I have done a lot of research on the topic lately, and it appears to me that the new state restoration framework would only work for when the app in killed in the background then resumed. The flutter team is looking into extending it to browser back/forward state restoration. Hence I am really not sure this is something to be looked into when searching for a state restoration option.

The other option is, as I already demonstrated and that @esDotDev did as well to use something which keep the entire widget state into memory, by doing as IndexedStack does. However this does have its downsides:

  1. It can easily be very costly
  2. It is scoped to a 1-level state restoration
  3. It does not rely on Page but Widgets (which basically mean throwing Navigator to the bin)

Each of these issues is bad:

Therefore this is not an option for me.

I could also build something from the ground up, momentum and navigation_saver are two examples. Looking at them really make me say that this is a good idea for me. There are trying to bring something else rather than integrate themselves. Which can be good but is not my ideology, since this means a lot of learning and re-implementation for the developer.

In the end, what I would love would be to be able to use the new Restoration API from the flutter team to achieve state restoration. The flutter built state restoration into core widget directly (such as TextField or Scrollable) which means that without doing anything, a developer would magically see the text of its TextField and the scroll offset of its Scrollable be restored. If someone can find/craft an example of how to make this work I would be really interested, though I am not even sure this is possible. Again, from what I have gathered, this API is designed to be used to restore the state of an app which is killed in the background.

esDotDev commented 3 years ago

Interesting points, some thoughts:

  1. I don't think this is a major issue. Flutter suffers from memory leaks when making a lot of routes, and it's also slower to constantly be creating/destroying things. Here you are trading a higher memory footprint (using more ram), but in return you get stable memory and very fast performance when switching (low cpu cost). Flutter caches images in RAM already, which is the vast majority of your consumed RAM.

  2. Not sure what this means, but the stacks are nestable, so I don't think it's limited to 1 level of anything, As you can see in my example, I am fully nesting 2 scaffold'ed menus, which both maintain their state so they can be animated.

  3. Pop is kinda broken anyways, in all routing solutions, cause it's a bit intractable to have "pop" or "up" behavior with a pure declarative layout. Easy at first, doesn't scale well. It works for fullscreen views and dialogs, but that's about it in my experience.

  4. In all imagined solution, maintainState would be optional, so it is totally up to developer which routes they want stored in memory and which they do not.

esDotDev commented 3 years ago

I've logged an issue here for StateRestoration at runtime, as this would certainly simplify things at the router level: https://github.com/flutter/flutter/issues/80303

lulupointu commented 3 years ago

Thanks, I would certainly keep an eye on this issue since as I said this would be a viable thing to implement in VRouter. Thanks for taking the time to fill up the issue and warn me.

lulupointu commented 3 years ago

I'll close this since I think we can track the flutter issue.

Regarding state restoration using IndexedStack, I will be posting an example of this when I have the time. Which could offer an alternative.

Thanks again for the time taken to discuss this 😊

lulupointu commented 3 years ago

@esDotDev in particular, you might be interested by this !

I just found out that it has always been possible to use IndexedStack with VRouter to create the behavior we discussed.

Here is the code: ```dart import 'package:flutter/material.dart'; import 'package:vrouter/vrouter.dart'; void main() { runApp( VRouter( debugShowCheckedModeBanner: false, routes: [ VNester( path: '/', widgetBuilder: (child) => MyScaffold(child), // Child is the widget from nestedRoutes nestedRoutes: [ VWidget(path: null, widget: HomeScreen()), // null path matches parent ], ), VNester( path: '/', widgetBuilder: (child) => MyScaffold(child), // Child is the widget from nestedRoutes nestedRoutes: [ VWidget( path: 'profile', widget: ProfileScreen(), stackedRoutes: [VWidget(path: 'settings', widget: SettingsScreen())], ), ], ), ], ), ); } abstract class BaseWidget extends StatefulWidget { String get title; String get buttonText; String get to; @override _BaseWidgetState createState() => _BaseWidgetState(); } class _BaseWidgetState extends State { bool isChecked = false; @override Widget build(BuildContext context) { return Material( child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Text(widget.title), SizedBox(height: 50), ElevatedButton( onPressed: () => context.vRouter.to(widget.to), child: Text(widget.buttonText), ), SizedBox(height: 50), Checkbox( value: isChecked, onChanged: (value) => setState(() => isChecked = value ?? false), ), ], ), ), ); } } class MyScaffold extends StatefulWidget { final Widget child; const MyScaffold(this.child); @override _MyScaffoldState createState() => _MyScaffoldState(); } class _MyScaffoldState extends State { List tabs = [Container(), Container()]; List tabsLastVisitedUrls = ['/', '/profile']; @override Widget build(BuildContext context) { final currentIndex = context.vRouter.url.contains('profile') ? 1 : 0; // Populate the tabs when needed tabs[currentIndex] = widget.child; // Populate tabs last visited url tabsLastVisitedUrls[currentIndex] = context.vRouter.url; return Scaffold( body: IndexedStack( index: currentIndex, children: tabs, ), bottomNavigationBar: BottomNavigationBar( currentIndex: currentIndex, onTap: (value) => context.vRouter.to(tabsLastVisitedUrls[value]), items: [ BottomNavigationBarItem(icon: Icon(Icons.home), label: 'Home'), BottomNavigationBarItem(icon: Icon(Icons.settings), label: 'Profile'), ], ), ); } } class HomeScreen extends BaseWidget { @override String get title => 'Home'; @override String get buttonText => 'Go to Profile'; @override String get to => '/profile'; } class ProfileScreen extends BaseWidget { @override String get title => 'Profile'; @override String get buttonText => 'Go to Settings'; @override String get to => '/profile/settings'; } class SettingsScreen extends BaseWidget { @override String get title => 'Settings'; @override String get buttonText => 'Pop'; @override String get to => '/profile'; } ```

The "trickiest" part is the duplication of VNester: why would you need 2 of them? This is because each VNester creates a Navigator with a unique GlobalKey. Therefore if you use the same VNester the tabs will contains each a Navigator but both Navigators will have the same GlobalKey (And in flutter, having 2 widgets with the same GlobalKey in the widget tree is not allowed)

Apart from that it's just basic logic:

I don't know why I did not think of it before because it's been possible since v1.1.1 (if not before).

Let me know what you think !

esDotDev commented 3 years ago

Oh that's pretty neat! I think the whole index management thing is probably a headache to maintain, but maybe it could be made more dynamic?

This is essentially how my path_stack package works, which is a similar idea: https://github.com/gskinnerTeam/flutter_path_stack/blob/f07cf6a1d17bf8fd7ca5f383fcd9ce7c81c01db7/lib/src/path_stack.dart#L89

lulupointu commented 3 years ago

The index is pretty easy to maintain because you only care about the first route. Though It's even easier if you pass the currentIndex as an argument of MyScaffold.

See this updated example: ```dart import 'package:flutter/material.dart'; import 'package:vrouter/vrouter.dart'; void main() { runApp( VRouter( debugShowCheckedModeBanner: false, routes: [ VNester( path: '/', widgetBuilder: (child) => MyScaffold(child, currentIndex: 0), nestedRoutes: [ VWidget(path: null, widget: HomeScreen()), ], ), VNester( path: '/', widgetBuilder: (child) => MyScaffold(child, currentIndex: 1), nestedRoutes: [ VWidget( path: 'profile', widget: ProfileScreen(), stackedRoutes: [VWidget(path: 'settings', widget: SettingsScreen())], ), ], ), ], ), ); } abstract class BaseWidget extends StatefulWidget { String get title; String get buttonText; String get to; @override _BaseWidgetState createState() => _BaseWidgetState(); } class _BaseWidgetState extends State { bool isChecked = false; @override Widget build(BuildContext context) { return Material( child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Text(widget.title), SizedBox(height: 50), ElevatedButton( onPressed: () => context.vRouter.to(widget.to), child: Text(widget.buttonText), ), SizedBox(height: 50), Checkbox( value: isChecked, onChanged: (value) => setState(() => isChecked = value ?? false), ), ], ), ), ); } } class MyScaffold extends StatefulWidget { final Widget child; final int currentIndex; const MyScaffold(this.child, {required this.currentIndex}); @override _MyScaffoldState createState() => _MyScaffoldState(); } class _MyScaffoldState extends State { List tabs = [Container(), Container()]; List tabsLastVisitedUrls = ['/', '/profile']; @override Widget build(BuildContext context) { // Populate the tabs when needed tabs[widget.currentIndex] = widget.child; // Populate tabs last visited url tabsLastVisitedUrls[widget.currentIndex] = context.vRouter.url; return Scaffold( body: IndexedStack( index: widget.currentIndex, children: tabs, ), bottomNavigationBar: BottomNavigationBar( currentIndex: widget.currentIndex, onTap: (value) => context.vRouter.to(tabsLastVisitedUrls[value]), items: [ BottomNavigationBarItem(icon: Icon(Icons.home), label: 'Home'), BottomNavigationBarItem(icon: Icon(Icons.settings), label: 'Profile'), ], ), ); } } class HomeScreen extends BaseWidget { @override String get title => 'Home'; @override String get buttonText => 'Go to Profile'; @override String get to => '/profile'; } class ProfileScreen extends BaseWidget { @override String get title => 'Profile'; @override String get buttonText => 'Go to Settings'; @override String get to => '/profile/settings'; } class SettingsScreen extends BaseWidget { @override String get title => 'Settings'; @override String get buttonText => 'Pop'; @override String get to => '/profile'; } ```

What's great is that this really fits VRouter since no change to the package is needed.

lulupointu commented 3 years ago

Something even better is that this approach does not seem to have any restriction since it makes little assumption of what is needed.

Here is an "advanced" example ```dart import 'package:flutter/material.dart'; import 'package:vrouter/vrouter.dart'; void main() { runApp( VRouter( debugShowCheckedModeBanner: false, routes: [ VNester( path: '/', widgetBuilder: (child) => MyScaffold(child, currentIndex: 0), nestedRoutes: [ VWidget( path: null, key: ValueKey('Home'), widget: HomeScreen(), stackedRoutes: [ VNester( path: null, widgetBuilder: (child) => MyTabs(child, currentIndex: 0), nestedRoutes: [ VWidget(path: 'red', widget: ColorScreen(color: Colors.redAccent, title: 'Red')) ], ), VNester( path: null, widgetBuilder: (child) => MyTabs(child, currentIndex: 1), nestedRoutes: [ VWidget(path: 'green', widget: ColorScreen(color: Colors.greenAccent, title: 'Green')) ], ), ], ), ], ), VNester( path: '/', widgetBuilder: (child) => MyScaffold(child, currentIndex: 1), nestedRoutes: [ VWidget( path: 'profile', widget: ProfileScreen(), stackedRoutes: [VWidget(path: 'settings', widget: SettingsScreen())], ), ], ), ], ), ); } class BaseWidget extends StatefulWidget { final String title; final String buttonText; final String to; BaseWidget({required this.title, required this.buttonText, required this.to}); @override _BaseWidgetState createState() => _BaseWidgetState(); } class _BaseWidgetState extends State { bool isChecked = false; @override Widget build(BuildContext context) { return Material( child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Text(widget.title), SizedBox(height: 50), ElevatedButton( onPressed: () => context.vRouter.to(widget.to), child: Text(widget.buttonText), ), SizedBox(height: 50), Checkbox( value: isChecked, onChanged: (value) => setState(() => isChecked = value ?? false), ), ], ), ), ); } } class MyScaffold extends StatefulWidget { final Widget child; final int currentIndex; const MyScaffold(this.child, {required this.currentIndex}); @override _MyScaffoldState createState() => _MyScaffoldState(); } class _MyScaffoldState extends State { List tabs = [Container(), Container()]; List tabsLastVisitedUrls = ['/', '/profile']; @override Widget build(BuildContext context) { // Populate the tabs when needed tabs[widget.currentIndex] = widget.child; // Populate tabs last visited url tabsLastVisitedUrls[widget.currentIndex] = context.vRouter.url; return Scaffold( body: IndexedStack( index: widget.currentIndex, children: tabs, ), bottomNavigationBar: BottomNavigationBar( currentIndex: widget.currentIndex, onTap: (value) => context.vRouter.to(tabsLastVisitedUrls[value]), items: [ BottomNavigationBarItem(icon: Icon(Icons.home), label: 'Home'), BottomNavigationBarItem(icon: Icon(Icons.settings), label: 'Profile'), ], ), ); } } class HomeScreen extends StatelessWidget { @override Widget build(BuildContext context) { return BaseWidget(title: 'Home', buttonText: 'Go to Color Tabs', to: '/red'); } } class SettingsScreen extends StatelessWidget { @override Widget build(BuildContext context) { return BaseWidget(title: 'Settings', buttonText: 'Pop', to: '/profile'); } } class ProfileScreen extends StatelessWidget { @override Widget build(BuildContext context) { return BaseWidget(title: 'Profile', buttonText: 'Go to Settings', to: '/profile/settings'); } } class MyTabs extends StatefulWidget { final Widget child; final int currentIndex; const MyTabs(this.child, {required this.currentIndex}); @override _MyTabsState createState() => _MyTabsState(); } class _MyTabsState extends State with SingleTickerProviderStateMixin { late final tabController = TabController( initialIndex: widget.currentIndex, length: tabs.length, vsync: this, ); // We use this as the index to easily fetch the new widget when in comes into view int get tabControllerIndex => tabController.index + tabController.offset.sign.toInt(); List tabs = [Container(), Container()]; @override Widget build(BuildContext context) { // Sync the tabController with the url if (!tabController.indexIsChanging && tabControllerIndex != widget.currentIndex) tabController.animateTo(widget.currentIndex); // Populate the tabs when needed tabs[widget.currentIndex] = widget.child; tabs = List.from(tabs); // Needed so that TabBarView updates its children return NotificationListener( onNotification: (_) { // Syncs the url with the tabController if (tabControllerIndex != widget.currentIndex) context.vRouter.to(tabControllerIndex == 0 ? '/red' : '/green'); return false; }, child: TabBarView( controller: tabController, children: tabs, ), ); } } class ColorScreen extends StatefulWidget { final Color color; final String title; const ColorScreen({required this.color, required this.title}); @override _ColorScreenState createState() => _ColorScreenState(); } class _ColorScreenState extends State with AutomaticKeepAliveClientMixin { bool isChecked = false; @override Widget build(BuildContext context) { super.build(context); return Container( color: widget.color, child: Center( child: Column( mainAxisSize: MainAxisSize.min, children: [ Text(widget.title), SizedBox(height: 50), ElevatedButton( onPressed: () => context.vRouter.to('/'), child: Text('Pop'), ), SizedBox(height: 50), Checkbox( value: isChecked, onChanged: (value) => setState(() => isChecked = value ?? false), ), ], ), ), ); } @override bool get wantKeepAlive => true; } ```

You can see that on top of the previous example I added:

What's pretty neat is that once more this does not require anything new from VRouter, you can ask google "How to keep the state in a TabBarView" and you are likely to be able to come up with such a design.