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

Makes Router more accessible to developers #63429

Closed
4 of 7 tasks
chunhtai opened this issue Aug 10, 2020 · 105 comments
Closed
4 of 7 tasks

Makes Router more accessible to developers #63429

chunhtai opened this issue Aug 10, 2020 · 105 comments
Labels
c: proposal A detailed proposal for a change to Flutter customer: crowd Affects or could affect many people, though not necessarily a specific customer. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. P1 High-priority issues at the top of the work list team-framework Owned by Framework team

Comments

@chunhtai
Copy link
Contributor

chunhtai commented Aug 10, 2020

The router widget is pretty complex because it solved a whole range of issues. We should think about how do we make it easier for developers to use it.

Here are some ideas:

Documentation:

  • 1. Add deeplinking documentation to MaterialApp/WidgetsApp. https://flutter.dev/docs/development/ui/navigation/deep-linking
  • 2. Add more explanation around SystemNavigator.routeUpdate and SystemNavigator.routeInformationUpdate, we should mention how web browser react to these method.
  • 3. Add more explanation around how web browser interact with the flutter framework. For example, web browser calls didPushRouteInformation when user click backward and forward button.
@chunhtai chunhtai added framework flutter/packages/flutter repository. See also f: labels. P1 High-priority issues at the top of the work list c: proposal A detailed proposal for a change to Flutter f: routes Navigator, Router, and related APIs. labels Aug 10, 2020
@chunhtai chunhtai changed the title Makes Router more accessible Makes Router more accessible to developers Aug 10, 2020
@chunhtai chunhtai mentioned this issue Aug 10, 2020
3 tasks
@TahaTesser TahaTesser added d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos documentation labels Aug 13, 2020
@feinstein
Copy link
Contributor

Could you please create a document, maybe a Medium publication, explaining what were the problems with Navigator, why Router was created, how it solves the problems and how to use it? I think this will make the transition smoother once it's in the stable channel.

@chunhtai
Copy link
Contributor Author

@feinstein That sounds like a good idea.

cc @csells @johnpryan

@feinstein
Copy link
Contributor

Maybe add the Pages API as well, or any other modifications that came along.

@esDotDev
Copy link

esDotDev commented Aug 26, 2020

On this topic, I'm trying to create a simple example, but keep getting errors.

The following assertion was thrown building Builder:
Assertion failed: file:///A:/sdks/flutter/packages/flutter/lib/src/widgets/navigator.dart:575:12
route.settings == this
is not true
The relevant error-causing widget was:
  MaterialApp file:///.../lib/main.dart:33:26

Another exception was thrown: A GlobalKey was used multiple times inside one widget's child list.
Application finished.

I don't see what I'm doing wrong:
main.dart

void main() {
  runApp(MyApp());
}

class MyApp extends StatefulWidget {
  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {

  MyAppRouteInformationParser _routeParser = MyAppRouteInformationParser();
  MyAppRouterDelegate _routerDelegate;
  AppModel _appModel = AppModel();

  @override
  void initState() {
    _routerDelegate = MyAppRouterDelegate(_appModel);
    super.initState();
  }

  @override
  Widget build(BuildContext context) {
    return ChangeNotifierProvider<AppModel>.value(
      value: _appModel,
      child: MaterialApp.router(
        // The parser will check .location, and return an object that represents the Page config
        routeInformationParser: _routeParser,
        routerDelegate: _routerDelegate,
      ),
    );
  }
}

routing.dart

// Create the base class for all the route paths in this app
abstract class MyAbstractPageConfig {
  MyAbstractPageConfig();
}

/// Delegate checks some external object (of your choosing) that represents the current route info, and returns a Navigator with a list of Pages
class MyAppRouterDelegate extends RouterDelegate<MyAbstractPageConfig>
    with ChangeNotifier, PopNavigatorRouterDelegateMixin<MyAbstractPageConfig> {

  @override
  final GlobalObjectKey<NavigatorState> navigatorKey;

  AppModel appModel;

  // Listen to a changeNotifier (AppModel) to notify us when the current page has changed
  MyAppRouterDelegate(this.appModel) : navigatorKey = GlobalObjectKey<NavigatorState>(appModel) {
    appModel.addListener(notifyListeners);
  }

  @override
  MyAbstractPageConfig get currentConfiguration => appModel.currentPageConfig;

  @override
  Widget build(BuildContext context) {
    Page buildNavPage(String name, Widget Function() builder) => CustomBuilderPage(
        key: ValueKey(name), routeBuilder: (_, __) => PageRouteBuilder(pageBuilder: (_, __, ___) => builder?.call()));

    // Fetch the current config from the AppModel
    MyAbstractPageConfig config = appModel.currentPageConfig;
    return Navigator(
      key: navigatorKey,
      pages: [
        // Always add home view, this would allow us to go back to home, even when deeplinking
        buildNavPage("home", () => HomePage()),
        // Add ListView if the current path is a ListPagePath
        if (appModel.currentPageConfig is ListPageConfig) ...{
          buildNavPage("list", () => ListPage()),
        },
        // Add DetailsPage if the current path is a DetailsPagePath. Pass the payload from the Path, into the actual widget.
        if (config is DetailsPageConfig) ...{
          // Dart implicitly casts `AbstractPagePath` to `DetailsPathPath` here, so we can access path.itemId with compile-safety
          buildNavPage("details", () => DetailsPage(pageConfig: config))
        },
      ],
      onPopPage: _handlePopPage,
    );
  }

  @override
  /// When the RouteParser returns a new page type, this will be called.
  Future<void> setNewRoutePath(MyAbstractPageConfig path) async {
    assert(path != null);
    /// Update the model with the new path, this will trigger the build fxn to fire and a new Navigator.pages stack is generated.
    appModel.currentPageConfig = path;
  }

  // We can explicitly handle back btns here, forcing a consistent up-to-parent navigation style
  bool _handlePopPage(Route<dynamic> route, dynamic result) {
    final success = route.didPop(result);
    if (success) {
      /// Update the model with the new path, this will trigger the build fxn to fire and a new Navigator.pages stack is generated.
      if (appModel.currentPageConfig is DetailsPageConfig) {
        appModel.currentPageConfig = ListPageConfig();
      }
      if (appModel.currentPageConfig is ListPageConfig) {
        appModel.currentPageConfig = HomePageConfig();
      }
    }
    return success;
  }
}

class MyAppRouteInformationParser extends RouteInformationParser<MyAbstractPageConfig> {
  @override
  Future<MyAbstractPageConfig> parseRouteInformation(RouteInformation routeInformation) async {
    // Use the .location string, to construct a Path object which will be based to the RouterDelegate to build the actual page
    final uri = Uri.parse(routeInformation.location);
    if (uri.pathSegments.length >= 2) {
      String itemId = uri.pathSegments[1];
      return DetailsPageConfig(itemId);
    } else if (uri.pathSegments.length == 1) {
      return ListPageConfig();
    } else {
      return HomePageConfig();
    }
  }

  @override
  RouteInformation restoreRouteInformation(MyAbstractPageConfig configuration) {
    if (configuration is HomePageConfig) {
      return RouteInformation(location: '/');
    }
    if (configuration is ListPageConfig) {
      return RouteInformation(location: '/items');
    }
    if (configuration is DetailsPageConfig) {
      return RouteInformation(location: '/items/${configuration.itemId}');
    }
    return null;
  }
}

@chunhtai
Copy link
Contributor Author

chunhtai commented Aug 26, 2020

Hi @esDotDev
The error is in this line

Page buildNavPage(String name, Widget Function() builder) => CustomBuilderPage(
        key: ValueKey(name), routeBuilder: (_, __) => PageRouteBuilder(pageBuilder: (_, __, ___) => builder?.call()));

it is complaining the route it return does not set its setting to the page.
changing it to

Page buildNavPage(String name, Widget Function() builder) => CustomBuilderPage(
        key: ValueKey(name), routeBuilder: (_, settings) => PageRouteBuilder(settings: settings, pageBuilder: (_, __, ___) => builder?.call()));

or just use TransitionBuilderPage

We should probably throw a more meaningful message than just assertion error

@esDotDev
Copy link

esDotDev commented Aug 27, 2020

Thanks, that works! I can't seem to figure out how to keep the browser url and history in sync though.

I've implemented restoreRouteInformation and currentConfiguration, and assigned a PlatformRouteInformationProvider:

MaterialApp.router(
        routeInformationProvider: PlatformRouteInformationProvider(),
        routeInformationParser: _routeParser,
        routerDelegate: _routerDelegate,
      )
  RouteInformation restoreRouteInformation(MyAbstractPageConfig configuration) {
    if (configuration is HomePageConfig) {
      return RouteInformation(location: '/');
    }
    if (configuration is ListPageConfig) {
      return RouteInformation(location: '/items');
    }
    if (configuration is DetailsPageConfig) {
      return RouteInformation(location: '/items/${configuration.itemId}');
    }
    return null;
  }

Browser url never seems to change. I also tried calling Router.navigate() inside the build method of the Delegate, didn't seem to work.

I also seem to be getting a null RouteInformation on startup, forcing me to add this to the top of parseRouteInformation

if(routeInformation == null) return HomePageConfig();

@chunhtai
Copy link
Contributor Author

hi @esDotDev, I am currently working on the engine side work #64494. Once this is done, the url should update accordingly

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

Am I correct in my understanding that, other than the initial call, setNewRoutePath will generally only fire on Web, and is a result of pasting an address into the address bar? Or are there other situations where the "operating system" pushes routes?

Additionally, _handlePopPage will only be called on platforms with an explicit back btn (Android & Web), is t hat also correct?

Finally, I don't quite see how we would emulate the previous Navigator.push() behavior, where we could endlessly add to the NavStack and then pop back. This would be analagous to a web user clicking the same 2 links back and forth creating many entries in the back stack.

I tried to implement something similar where I add the new pages to a stack, but it's throwing various errors, or just not loading the new pages:

MyAbstractPageConfig config = appModel.currentPageConfig;
    if(config is HomePageConfig || config == null){
      _pageStack.add(buildNavPage("HomePage", () => HomePage()));
    }
    if(config is ListPageConfig){
      _pageStack.add(buildNavPage("ListPage", () => ListPage()));
    }
    if(config is DetailsPageConfig){
      _pageStack.add(buildNavPage("Details", () => DetailsPage(pageConfig: config)));
    }
    return Navigator(
      key: navigatorKey,
      pages: [..._pageStack],
      onPopPage: _handlePopPage,
    );
  }

@chunhtai
Copy link
Contributor Author

Am I correct in my understanding that, other than the initial call, setNewRoutePath will generally only fire on Web, and is a result of pasting an address into the address bar? Or are there other situations where the "operating system" pushes routes?

Yes that is true for all the platform embeddings that we owned (third party embeddings or plug-ins can do whatever they want)

Additionally, _handlePopPage will only be called on platforms with an explicit back btn (Android & Web), is t hat also correct?

Yes and no, it will be called in Android. For Web, the backward button currently calls the _handlePopPage. The engine change i am working will change that behavior. After the change if app uses router, the Web will pass the new url and state after the backward/forward buttons to the route information parser, which passes the parsed result to setNewRoutePath. If you are not using router, the behavior will stay the same for backward compatibility.

Finally, I don't quite see how we would emulate the previous Navigator.push() behavior, where we could endlessly add to the NavStack and then pop back. This would be analagous to a web user clicking the same 2 links back and forth creating many entries in the back stack.

As I mentioned in previous question, We will separate our the browser history and the router nav stack. Browsers keep tracks of all the urls and states the app navigated to, and just tell the app go to specific url and state when user click backward and forward button. We wouldn't run into this situation.

The code you provide is a bit contradicting with our design, your app state _pageStack and the MyAbstractPageConfig should be one to one mapping, that means a MyAbstractPageConfig should always create the _pageStack with the same pages and should not keep adding page on top of it.

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

Oh, that is interesting that browser history will become just a list of intents. But how does that work on Android? Previously we had similar behavior to what you're describing, where we would just push the same route many times and continually add to the stack (think like viewing many productDetails?id=foo in sequence, on both android or web).

I don't understand the one to one mapping thing you are describing. I'm just trying to maintain a history of routes, and let that grow as new routes are added, and then allow them to be popped off. I don't know the routes before hand, for example user might do something like:

.currentPage = DetailsPage(id: 1)
.currentPage = DetailsPage(id: 2)
.currentPage = DetailsPage(id: 3)
.currentPage = DetailsPage(id: 4)
pop()  <!-- goes to 3
pop() <!-- goes to 2
pop() <!-- goes to 1
.currentPage = DetailsPage(id: 2)
pop() <!-- goes to 1

My assumption was that I could just manage this one-dimensional page stack myself, provide the pages to the navigator as needed, and then rely on the Navigator to pop the stack. That's not correct I guess?

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

Looking at my example, I realize I shouldn't be doing this in build, but instead in the setNewRoutePath. But is there an issue other than that? Like is this not an expected use case:

List<Page> _pages;

Future<void> setNewRoutePath(MyAbstractPageConfig path) async {
    _pages.add(_buildPageFromPath(path))
   state.currentPath = path;
}


Widget build(){
    return Navigator(
        pages: _pages
    )
}

[Edit] I realize now that the example doesn't really make sense either, since setNewRoutePath() is only called from the OS. So I guess build is the only place this could go?

I guess I'm just confused how you would emulate a classic viewstack with the Router, working on both Android and Web with the same behavior... are we forced to use Navigator.Push as opposed to just setting the new Path config?

My understanding from what you're describing, is that if I had an app that could generate a long history of routes, on Web, using Router, when I take that same app to Android, I will get different behavior than web in terms of history? ie, Android app would have no history, unless I refactor to use the classic Push API.

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

I think I understand what you are saying with the 1:1 mapping. I should have something more like this??

class AppState {
    List<MyPageInfo> pageStack; 

    void AddPage(MyPageInfo p){
        pageStack.add(p);
        notifyListeners();
    }

   void PopPage(){
        pageStack.removeLast();
        notifyListeners();
    }
}

/// Delegate builds a list of Pages, based on the list of MyPageInfo's in the AppState? 

Seems like this would ideally be part of the concrete delegate, so we could just have a delegate with that behavior and re-use it. But I guess we can do the same with a specialized StackableRouteState or something.

If this is correct I don't really understand why my example was not working. The seem technically to do the same thing, except mine might create duplicate pages if build() is called repeatedly.

@chunhtai
Copy link
Contributor Author

chunhtai commented Aug 28, 2020

yes Android and web will act differently in terms of route history management, they have very different ways on dealing with history. For web, every history entry is just a page, it does not really stack on each other so that it can go forward and backward. where the android page is pages stack on each other so it can only go backward.

On web, you don't worry about route stack, you just need to focus on which page you build in setNewRoutePath when giving a url, and you should not stack another page on top in the method (unless you have a in page button that pop that page then you should probably build multiple page with that url, for example, Gmail). The 1 to 1 mapping I mentioned earlier is that one url should always produce same list of pages. for example /setting produce [HomePage, SettingPage], the next time setNewRoutePath calls with /setting should still produce the same list [HomePage, SettingPage], it should not become [HomePage, SettingPage, HomePage, SettingPage]. There is really no reason to do that because you don't rely on the route stack to navigate between pages on Web.

You may think then I do I support android in this case? the android route stack is managed some where else, the setNewRoutePath won't be call in android.

On android, it is a whole different story. for example open a page on button click, in this case, you WILL want to add that page on top of the existing page.

@chunhtai
Copy link
Contributor Author

chunhtai commented Aug 28, 2020

for 1:1 mapping you should have something like this

Future<void> setNewRoutePath(MyAbstractPageConfig path) async {
   state.currentPath = path;
   if (state.currentPath is HomePage)
       _pages = <Page>[HomePage()];
   else if (state.currentPath is HomePage)
       _pages = <Page>[HomePage(), SettingPage()];// I build two page because SettingPage may have in page back button or HomePage have state that I want to preserve
}

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

Ok, I will try and digest this. My inital thought is that I don't know why we wouldn't abstract these into the same behavior. From the app developers viewpoint, the only difference to us is that one can go forward + back, and the other only back. (And we actually don't care about this either, a page is a page, we will just load it)

So in both (all) cases, we really just need a list of history state, and then the ability to re-construct any given route. I as the app developer don't really care how the OS manages these history states( back button, verbal command, direct url link, sign language ), or what it tells me to jump to. I do want some control over what history states are added to this history as I browse, but I don't see why I should be concerned with which sandbox/environment the runtime is in. Back btn is a back btn, app state is app state, routes are routes... seems like an unnecessary distinction.

Would we then need an alternate implementation for iOS, and maybe another implementation for Fuschia or some other future OS, where each app needs to handle (and worse, understand) all these os-specific mechanisms for history?

On android, it is a whole different story. for example open a page on button click, in this case, you WILL want to add that page on top of the existing page.

I still don't understand how the classic viewstack would be done on Android/iOS/Desktop if we can not maintain our own dynamic stack of pages, and do not have access to whatever mechanism the Web build is using to maintain this list of router entries.

@chunhtai
Copy link
Contributor Author

chunhtai commented Aug 28, 2020

So in both (all) cases, we really just need a list of history state, and then the ability to re-construct any given route.

I think this is true in the current design, the re-construct any given route. is setNewRoutePath. As long as you have that, browser navigation will work just fine. we really just need a list of history state We were already doing that by using Navigator.push and pop and we provide a better way to do that with Navigator.pages in the Navigator 2.0. I just want to point out the fact setNewRoutePath should not stack route on top each other because it is not necessary and can potentially OOM if users keep pressing forward and backward buttons.

Back btn is a back btn

I agree with most part, but browser backward button is not the same as android back button. the android back button is more of dismissed the current activity. the browser backward button is to go to previous page that you were viewing, so the backward button can mean pushing a new page and can be confused with flutter's route stack. That is why we want to separate them out to a different concept. Now the android back button is still dismiss the current activity, the browser backward button is treated as go to this new url

I think the design mindset should be make sure setNewRoutePath create a list of pages that make sense for that route, and just design your app by changing the pages in appstate when user interaction with the widget. That way the app should work all platforms.

You also mentioned your app does not work, Can you be more specific where it doesn't work or provide a runnable example?

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

Might work better if I show you what was trying to create, and then you can advise the correct approach.

So lets say a developer has an app like this, they want consistent behavior on Desktop, Android, iOS and Web:
2020-08-28_13-42-21 (1)

  • Note: The only change the Dev wants to make here, is hide the BackBtn on platforms where the OS provides one.
  • Note2: App would likely have a home btn and/or breadcrumbs to let the user directly jump to some other section. (Think like any e-commerce app)

Is this possible currently with Router? My example was simply trying to emulate this, by adding a new Page into a list each time we get one, and then providing that whole list inside of Navigator.pages.

If I understand you correctly, you're saying that only on web would this behavior work, but on all other platforms, we are relegated to a hard-coded back history of "../" up to parent, but no ability to easily construct a dynamic history of pages visited and easily pop back through them.

Currently this is very simple with the existing Navigator, but it misses the ability to have a history stack on the web AFAIK:

class MyApp extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MaterialApp(home: MyHome());
}

class MyHome extends StatelessWidget {
  @override
  Widget build(BuildContext context) => MyScaffold(Center(child: Text("HOME")), showBack: false);
}

class DetailsPage extends StatelessWidget {
  final int itemId;

  const DetailsPage(this.itemId, {Key key}) : super(key: key);

  @override
  Widget build(BuildContext context) => MyScaffold(Container(
        alignment: Alignment.center,
        color: Colors.red.withOpacity(.2 + itemId * .2),
        child: Text("DETAILS $itemId"),
      ));
}

class MyScaffold extends StatelessWidget {
  final Widget page;
  final bool showBack;

  const MyScaffold(this.page, {Key key, this.showBack = true}) : super(key: key);

  void _showDetailsPage(BuildContext c, int i) {
    Navigator.push(c, PageRouteBuilder(pageBuilder: (_, __, ___) => DetailsPage(i)));
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Stack(children: [
        page,
        if (showBack) ...{FlatButton(onPressed: () => Navigator.pop(context), child: Text("BACK"))},
      ]),
      bottomNavigationBar: Row(
        children: [
          FlatButton(onPressed: () => _showDetailsPage(context, 1), child: Text("Product 1)")),
          FlatButton(onPressed: () => _showDetailsPage(context, 2), child: Text("Product 2)")),
          FlatButton(onPressed: () => _showDetailsPage(context, 3), child: Text("Product 3)"))
        ],
      ),
    );
  }
}

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

the browser backward button is to go to previous page that you were viewing, so the backward button can mean pushing a new page and can be confused with flutter's route stack.

Just feels like it's an os level implementation detail... like from the Developer perspective, both end in a Widget, the only thing we really care about is that, to the extent there are transitions, they play in the correct direction. Which basically comes down to: is this 'new' view behind me in the history stack.

I think the design mindset should be make sure setNewRoutePath create a list of pages that make sense for that route, and just design your app by changing the pages in appstate when user interaction with the widget. That way the app should work all platforms.

This works for the use case I'm describing, if Pages can be a List of pages rather than a single item... I'll give it a go?

@chunhtai
Copy link
Contributor Author

the browser backward button is to go to previous page that you were viewing, so the backward button can mean pushing a new page and can be confused with flutter's route stack.

Just feels like it's an os level implementation detail... like from the Developer perspective, both end in a Widget, the only thing we really care about is that, to the extent there are transitions, they play in the correct direction. Which basically comes down to: is this 'new' view behind me in the history stack.

I think the design mindset should be make sure setNewRoutePath create a list of pages that make sense for that route, and just design your app by changing the pages in appstate when user interaction with the widget. That way the app should work all platforms.

This works for my use case, if Pages can be a List of pages rather than a single item... I'll give it a go?

It looks like you are trying to mix between the page back button with the browser backward button. That means the same url may present different state, you will need to leverage the RouteInformation.state so that you know which state the page is in when the browser tells it to go to a certain url. Let me try to come up with an example code

@esDotDev
Copy link

esDotDev commented Aug 28, 2020

Well in this example, when on Web or Android we would hide our internal back btn, and rely on the OS btn to drive the pop. We wouldn't show both at once.

When on Desktop or iOS or PlatformWithoutABackButtonX, we would provide a back btn, this way we get consistent behavior on all platforms, and we don't have two code paths.

@chunhtai
Copy link
Contributor Author

@esDotDev then it will be much simpler, something like this should work. You can't really test web until this is merged flutter/engine#20794. if you can build custom engine, you should be able to test it with the code in that pr.

abstract class RoutePath {
  const RoutePath();
}

class HomePath extends RoutePath{}

class Page0Path extends RoutePath{}
class Page1Path extends RoutePath{}
class Page2Path extends RoutePath{}

class MyRouteInformationParse extends RouteInformationParser<RoutePath> {
  @override
  Future<RoutePath> parseRouteInformation(RouteInformation routeInformation) {
    if (routeInformation.location == '/')
      return SynchronousFuture<RoutePath>(HomePath());
    if (routeInformation.location == '/0')
      return SynchronousFuture<RoutePath>(Page0Path());
    if (routeInformation.location == '/1')
      return SynchronousFuture<RoutePath>(Page1Path());
    if (routeInformation.location == '/2')
      return SynchronousFuture<RoutePath>(Page2Path());
    throw '404 ?';
  }

  @override
  RouteInformation restoreRouteInformation(RoutePath configuration) {
    if (configuration is HomePath)
      return RouteInformation(location: '/');
    if (configuration is Page0Path)
      return RouteInformation(location: '/0');
    if (configuration is Page1Path)
      return RouteInformation(location: '/1');
    if (configuration is Page2Path)
      return RouteInformation(location: '/2');
    throw 'unknown route?';
  }
}

class MyRouterDelegate extends RouterDelegate<RoutePath> with ChangeNotifier, PopNavigatorRouterDelegateMixin<RoutePath>{
  @override
  GlobalKey<NavigatorState> navigatorKey = GlobalKey<NavigatorState>();

  List<Page> get pages => _pages;
  List<Page> _pages;
  set pages(List<Page> pages) {
    _pages = pages;
    notifyListeners();
  } 


  @override
  Future<void> setNewRoutePath(RoutePath configuration) {
    _pages = <Page>[HomePage()];
    if (configuration is Page0Path)
      _pages.add(Page0());
    if (configuration is Page1Path)
      _pages.add(Page1());
    if (configuration is Page2Path)
      _pages.add(Page2());
    return SynchronousFuture<void>(null);
  }

  @override
  RoutePath get currentConfiguration {
    if (_pages.last is HomePage)
      return HomePath();
    if (_pages.last is Page0)
      return Page0Path();
    if (_pages.last is Page1)
      return Page1Path();
    if (_pages.last is Page2)
      return Page2Path();
    throw 'unknown route?';
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Navigator(
        key: navigatorKey,
        pages: _pages,
        onPopPage: _handlePopPage,
      ),
      bottomNavigationBar: Row(
        children: [
          FlatButton(onPressed: () => pages = List<Page>.from(pages..add(Page0())), child: Text("Product 1)")),
          FlatButton(onPressed: () => pages = List<Page>.from(pages..add(Page1())), child: Text("Product 2)")),
          FlatButton(onPressed: () => pages = List<Page>.from(pages..add(Page2())), child: Text("Product 3)"))
        ],
      ),
    );
  }
}

class Page0 extends TransitionBuilderPage{
  Page0(): super(
    pageBuilder: (_, __, ___) => Text('Product 1')
  );
}

class Page1 extends TransitionBuilderPage{
  Page1(): super(
    pageBuilder: (_, __, ___) => Text('Product 2')
  );
}

class Page2 extends TransitionBuilderPage{
  Page2(): super(
    pageBuilder: (_, __, ___) => Text('Product 3')
  );
}

class HomePage extends TransitionBuilderPage{
  HomePage(): super(
    pageBuilder: (_, __, ___) => Text('HomePage')
  );
}

@feinstein
Copy link
Contributor

@Jonas-Sander:
#63429 (comment)

@Jonas-Sander
Copy link

Ah, thanks I missed that.

We are aware of the difficulty that the developers are facing with the current navigator 2.0 API, and have being exploring different ideas on how to make things easier. Your feedbacks have been very helpful to determine our next step.

Well would be nice to hear what different ideas they have been exploring and what the next steps are.
Kind of a bummer with this response but I guess there will be a reason for it :)

@chunhtai
Copy link
Contributor Author

@Jonas-Sander
We definitely need a lot of more documentations.

The API is not likely to change, it is the complexity vs functionality. This is the simplest form we can come up with while still enables all use cases.

Therefore, we have being explore different ideas around building a routing library on top of the API that targets specific use case. This should theoretically has simpler API, but we do not have a concrete idea yet.

@Jonas-Sander
Copy link

@chunhtai Great, thanks for your answer.

I have some follow-up questions.
I'm asking these as I think the answers may help to get the community involved and more quickly iterate on new solutions.
Feel free to ignore or say that you don't have an answer. These are just some questions out of my gut.

  • First off: Do you have a specific wish on how the community should help in this regard?
    Should the community try to build their own packages and solutions on top of the Router?
    Should the community wait for experiments from the flutter team and try them out?
  • Is there a plan on how and by whom the next steps of "Navigation 2.1" should be achieved?
  • Is there documentation or code what ideas you have already explored and what the results were?
  • You mentioned a specific use cases. What are these use cases? What is the target goal? Just make it simpler? What part?
  • If the community should get involved: Where and how should ideas be explored?
    Is there a ticket (this one?) or specific place where created solutions should be discussed?

@esDotDev
Copy link

esDotDev commented Dec 16, 2020

I guess I don't see why there needs to be any forward/backward support at all.

Ya I don't totally get this either. Deeplink is obviously a huge priority, and the browser back btn needs to work, but "forward" seems much less important to your avg developer. If "forward" specifically is not important to you, then you really don't need Router... It's very easy to get the browser url updating and to support deep-linking, and also awesome libs like yours that can make that even easier. Not sure, but I think with a little bit of work, you could get those history states working directly with the Imperative API and just sidestep Router all-together.

Also, you can still use the state-first paradigm, without using Router. I have a running example here, with code here. I wasn't able to implement forward, but I didn't spend much time trying.

I think this could be a fairly key part of the messaging from Google to Developers to make things easier. Explain Router is the "all-in-one" but takes a bit of wiring, but there are other ways to get there as well if you want the "single page app" approach that many will be fine with: No fwd btn, browser-back works as pop(), and deeplinks work).

@ghost
Copy link

ghost commented Jan 31, 2021

Hi, I'm experimenting with my router package on top of Navigator 2.0, which provides concise, cohesive, type-safe API, (hopefully) without losing the functionality and flexibility of Navigator 2.0.

https://github.com/polyflection/flit_router

@esDotDev
Copy link

esDotDev commented Mar 1, 2021

@chunhtai Curious what is the best way to implement a system where we need an Overlay above the Navigator?

For example, DesktopTitleBar needs to call Overlay.of here:

return Column(children: [
   DesktopTitleBar().
   Expanded(child: Navigator(pages: ...))
]);

Wrapping with a 2nd Navigator is not great as we have to wrap our content in a MaterialPage, and provide a handler for onPop, neither of which is our intent and it expands our bug surface. Using a nested MaterialApp is problematic, as seems to interfere with Router if we use the onGenerateRoute handler like we would want.

Has this use case been considered? Is there some elegant way I'm missing? The Overlay widget itself seems like it would be ideal, but I can find zero guidance on how to implement Overlay in a standalone fashion. I've posted a question on that here: https://stackoverflow.com/questions/66425271/how-to-use-an-overlay-without-the-navigator-app

@esDotDev
Copy link

esDotDev commented Jan 8, 2022

The answer to the above, is to return something like this:

_overlayEntry ??= OverlayEntry(builder: (_){
      MyAppScaffold(child: Navigator(...))
});
_overlayEntry!.markNeedsBuild();

return Overlay(initialEntries: [_overlayEntry!]);

Now any persistent UI that wraps the navigator/router, can interact with an Overlay, and things like Tooltips should work,

@esDotDev
Copy link

esDotDev commented Jan 9, 2022

@chunhtai One thing I really can't seem to determine from the docs, is how I can simply bind to the current location from within the widget tree.

EDIT - I figured it out I think? This seems to work well, but is pretty verbose. Please lmk if there is an easier way than this.

One downside to this is it needs to be a child of Router and use Router.of(context), I would prefer to be able to bind directly to the global path value if possible.

class _LocationTextState extends State<LocationText> {
  RouterDelegate get delegate => Router.of(context).routerDelegate;
  RouteInformationProvider? get provider => Router.of(context).routeInformationProvider;
  @override
  void initState() {
    super.initState();
    scheduleMicrotask(() {
      delegate.addListener(_handleRouteChanged);
      provider?.addListener(_handleRouteChanged);
    });
  }

  @override
  void dispose() {
    delegate.removeListener(_handleRouteChanged);
    provider?.removeListener(_handleRouteChanged);
    super.dispose();
  }

  void _handleRouteChanged() => setState(() {});

  String getLocation(BuildContext context) {
    final r = Router.of(context);
    final parser = r.routeInformationParser;
    final config = r.routerDelegate.currentConfiguration;
    return parser?.restoreRouteInformation(config)?.location ?? '/';
  }

  @override
  Widget build(BuildContext context) => Text(getLocation(context));
}

@xuanswe
Copy link

xuanswe commented Jan 13, 2022

@chunhtai Does Flutter team have (or plan to have) an API to delete an entry (not just overwrite the previous one) in the browser history?

@chunhtai
Copy link
Contributor Author

@nguyenxndaidev No, there is no such API, and I think you can't do that in native web too, at least not with other side affect.

@xuanswe
Copy link

xuanswe commented Jan 13, 2022

@chunhtai yeah, I think you are right.
I was confused with the browser extension history API, which is not available for normal websites.

@esDotDev
Copy link

One section of the docs that is quite interesting, but not well explained is the secion on nested routers:
A particularly elaborate application might have multiple Router widgets, in a tree configuration, with the first handling the entire route parsing and making the result available for routers in the subtree. The routers in the subtree do not participate in route information parsing but merely take the result from the first router to build their sub routes.

I'm struggling to understand how the root router would "making the result available for routers in the subtree".

@csells
Copy link
Contributor

csells commented Jan 14, 2022

Oh yes! I want to know how that works, too. Such a feature would be great for nested navigation scenarios, e.g. if each tab in a TabBarView could have it's own Router and it's own set of routes.

@esDotDev
Copy link

esDotDev commented Jan 14, 2022

I've been thinking about this all night, I guess the Router at the top could do something like:

class TopRouter extends RouterDelegate {
  
  List<Page> buildPageStack() => ....
  
  // Top router does not return a navigator
  Widget build(context) {
    return MainApp();
  }
}

With routers below doing:

class SubRouter extends RouterDelegate {

Widget build(context) {
  return Navigator(
    pages: (Router.of(context).routerDelegate as TopRouter).buildPageStack();
  )
}

I'm not sure if that technique is what the docs are alluding to, or something else. My main question with this approach, is why use a child Router at all when we could just drop the Navigator in directly.

@chunhtai
Copy link
Contributor Author

@esDotDev there can be many way to make the result available, you can store the result in app state and let the sub router grabs the result from it.

I'm not sure if that technique is what the docs are alluding to, or something else. My main question with this approach, is why use a child Router at all when we could just drop the Navigator in directly.

You can just drop a navigator, but you won't be able to use back button dispatcher.

@esDotDev
Copy link

Ah, that didn't occur to me, so Router is responsible for listening for OS level back buttons, and without some control over the navigator, the navigator can't respond to those events properly. So we should still use a Router to wrap our child Navigators, so those navigators can response to OS level back events.

Looks like some more details here: https://api.flutter.dev/flutter/widgets/BackButtonDispatcher-class.html

@rayliverified
Copy link

rayliverified commented Jan 14, 2022

Oh yes! I want to know how that works, too. Such a feature would be great for nested navigation scenarios, e.g. if each tab in a TabBarView could have it's own Router and it's own set of routes.

Would love to see the community share their solutions here as well.
I've genuinely struggled with nested tab navigation so much. If there's an Flutter idiomatic way of building nested navigation, I haven't seen it work.

My solution is to pass the tabs as query arguments. DefaultRoute.home(tab: tab)
We need to update the URL whenever a nested tab changes right? So I set the route stack directly and do an in place replacement.

RouteController.replace( context!, DefaultRoute.home(), DefaultRoute.home(tab: tab));

What really bothers me is I haven't been able to get nested tabs with different URLs instead of arguments (i.e. /home/1 , /home/2) working. If the URL changes, the Navigator treats it as a different route and forces a rebuild, which recreates the entire tab parent.

/home/1 changes to /home/2. The URL is now different so the Navigator thinks they are two different pages. The entire nested tab gets rebuilt.

Simply put, the Flutter solution of nesting a Navigator inside of a Navigator (multiplying the complexity) is insanity and doesn't work in practice. Creating some dedicated Navigation coordinators and data communication widgets would be much more usable and sane. Perhaps a TabNavigator Widget that allows the user to set URLs for each tab and then updates the parent Navigator with URLs correctly.

Edit: Managing back navigation within nested tabs is another issue on top. Sometimes back should navigate within the current Navigator. Other times, it should be discarded and pressing back should navigate on the parent Navigator.

@esDotDev
Copy link

esDotDev commented Jan 14, 2022

@esDotDev there can be many way to make the result available, you can store the result in app state and let the sub router grabs the result from it.

Specifically the use case I'm interested in, is how to have multiple routers, inside an indexed stack, and switch between them, with some root ancestor widget handling the route parsing. Finding it hard to wrap my head around who is responsible for creating pages, or where the state should live. Add to that the back button dispatcher reference passing and I'm pretty lost.

The primary goal is that the page stacks in each Navigator would be stateful, and persist when the other tab is in view. So, imagine an app with only 2 routes (/a and /b), each route has a TextField and maintains it's contents as you change routes because each route is actually rendered by it's own Navigator/Router.

A demo of this just this would be amazingly helpful.

@darshankawar darshankawar added the customer: crowd Affects or could affect many people, though not necessarily a specific customer. label Mar 29, 2022
@flutter-triage-bot flutter-triage-bot bot added team-framework Owned by Framework team triaged-framework Triaged by Framework team labels Jul 8, 2023
@flutter-triage-bot flutter-triage-bot bot removed the triaged-framework Triaged by Framework team label Dec 1, 2023
@flutter-triage-bot
Copy link

This issue is marked P1 but has had no recent status updates.

The P1 label indicates high-priority issues that are at the top of the work list. This is the highest priority level a bug can have if it isn't affecting a top-tier customer or breaking the build. Bugs marked P1 are generally actively being worked on unless the assignee is dealing with a P0 bug (or another P1 bug). Issues at this level should be resolved in a matter of months and should have monthly updates on GitHub.

Please consider where this bug really falls in our current priorities, and label it or assign it accordingly. This allows people to have a clearer picture of what work is actually planned. Thanks!

Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: proposal A detailed proposal for a change to Flutter customer: crowd Affects or could affect many people, though not necessarily a specific customer. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. P1 High-priority issues at the top of the work list team-framework Owned by Framework team
Projects
None yet
Development

No branches or pull requests