chore: go router proposal #216
Conversation
f7deleon
force-pushed
the
feat/propose-go-router
branch
3 times, most recently
from
93d8e29
to
857c85d
Compare
| ], | ||
| redirect: (BuildContext context, GoRouterState state) { | ||
| final authState = AuthenticationScope.of(context); | ||
| if (authState == AuthenticationStatus.unauthenticated) { |
There was a problem hiding this comment.
We may need to be careful, if this is evaluated at the top of the navigation tree, a new route /details it will be redirected to / because it returns a modified value for the path.
Maybe we should return null if the user is authenticated or check the current path and redirect based on that.
We had this problem in another project that used go_router
| ), | ||
| ]; | ||
| } | ||
| final GoRouter appRouter = GoRouter( |
There was a problem hiding this comment.
It can be good to add the initial route as a parameter for testing and deep linking, what do you think?
| routes: <RouteBase>[ | ||
| GoRoute( | ||
| path: '/', | ||
| builder: (BuildContext context, GoRouterState state) => |
There was a problem hiding this comment.
Maybe we can add a helper method for creating the page widget
| GoRoute( | ||
| path: '/welcome', | ||
| builder: (BuildContext context, GoRouterState state) => | ||
| const WelcomeScreen(), |
There was a problem hiding this comment.
What would happen if we want to send parameters to the page without going to a repository, for example a details page. Do we have an example on how that would look?
f7deleon
force-pushed
the
feat/propose-go-router
branch
from
857c85d
to
53c7988
Compare
| @@ -69,7 +68,7 @@ class _ProjectWidget extends StatelessWidget { | |||
|
|
|||
| @override | |||
| Widget build(BuildContext context) => GestureDetector( | |||
| onTap: () => FlutterWebBrowser.openWebPage(url: project.url), | |||
| onTap: () => context.push('/details'), | |||
| final SessionRepository _sessionRepository; | ||
| final AuthenticationStatus _requiredAppStatus; | ||
| final PageRouteInfo _redirectPage; | ||
| class AuthenticationScope extends InheritedNotifier<StreamAuthNotifier> { |
There was a problem hiding this comment.
cannot we avoid it? I'ts strange to use a InheritedNotifier. We should use a provider or something like that, we shouldn't mix how we get data
♻️ Refactor
✏️ Description:
Proposal to remove auto router since it uses the build runner package and makes you regenerate the code each time you change the navigation tree.