-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[go_router_builder] Allow users to specify onExit as optional #11151
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,3 +1,8 @@ | ||||||||
| ## 4.3.0 | ||||||||
|
|
||||||||
| - Adds support for `overrideOnExit` parameter in `TypedGoRoute` and `TypedRelativeGoRoute` annotations. | ||||||||
| When set to `true`, the generated route will include `overrideOnExit: true` in the GoRoute constructor, enabling custom `onExit` callback implementation for the route. Defaults to `false`. | ||||||||
|
Comment on lines
+3
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changelog entry is a bit verbose. For better conciseness, which is encouraged by the repository style guide, you could shorten it to be more in line with other entries in this file.
Suggested change
References
|
||||||||
|
|
||||||||
| ## 4.2.0 | ||||||||
|
|
||||||||
| - Adds supports for `TypedQueryParameter` annotation. | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| // ignore_for_file: public_member_api_docs, unreachable_from_main | ||
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:go_router/go_router.dart'; | ||
|
|
||
| part 'not_override_on_exit_example.g.dart'; | ||
|
|
||
| void main() => runApp(App()); | ||
|
|
||
| class App extends StatelessWidget { | ||
| App({super.key}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) => | ||
| MaterialApp.router(routerConfig: _router, title: _appTitle); | ||
|
|
||
| final GoRouter _router = GoRouter(routes: $appRoutes); | ||
| } | ||
|
|
||
| @TypedGoRoute<HomeRoute>(path: '/') | ||
| class HomeRoute extends GoRouteData with $HomeRoute { | ||
| const HomeRoute(); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context, GoRouterState state) => const HomeScreen(); | ||
| } | ||
|
|
||
| @TypedGoRoute<Sub1Route>(path: '/sub-1-route') | ||
| class Sub1Route extends GoRouteData with $Sub1Route { | ||
| const Sub1Route(); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context, GoRouterState state) => const Sub1Screen(); | ||
| } | ||
|
|
||
| @TypedGoRoute<Sub2Route>(path: '/sub-2-route') | ||
| class Sub2Route extends GoRouteData with $Sub2Route { | ||
| const Sub2Route(); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context, GoRouterState state) => const Sub2Screen(); | ||
| } | ||
|
|
||
| class HomeScreen extends StatefulWidget { | ||
| const HomeScreen({super.key}); | ||
|
|
||
| @override | ||
| State<HomeScreen> createState() => _HomeScreenState(); | ||
| } | ||
|
|
||
| class _HomeScreenState extends State<HomeScreen> { | ||
| String? _result; | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) => Scaffold( | ||
| appBar: AppBar(title: const Text(_appTitle)), | ||
| body: Center( | ||
| child: ElevatedButton( | ||
| onPressed: () async { | ||
| final String? result = await const Sub1Route().push<String?>(context); | ||
| if (!context.mounted) { | ||
| return; | ||
| } | ||
| setState(() => _result = result); | ||
| }, | ||
| child: Text(_result ?? 'Go to sub 1 screen'), | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| class Sub1Screen extends StatelessWidget { | ||
| const Sub1Screen({super.key}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) => Scaffold( | ||
| appBar: AppBar(title: const Text('$_appTitle Sub 1 screen')), | ||
| body: Center( | ||
| child: ElevatedButton( | ||
| onPressed: () async { | ||
| final String? result = await const Sub2Route().push<String?>(context); | ||
| if (!context.mounted) { | ||
| return; | ||
| } | ||
| context.pop(result); | ||
| }, | ||
| child: const Text('Go to sub 2 screen'), | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| class Sub2Screen extends StatelessWidget { | ||
| const Sub2Screen({super.key}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) => Scaffold( | ||
| appBar: AppBar(title: const Text('$_appTitle Sub 2 screen')), | ||
| body: Center( | ||
| child: ElevatedButton( | ||
| onPressed: () => context.pop('Sub2Screen'), | ||
| child: const Text('Go back to sub 1 screen'), | ||
| ), | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| const String _appTitle = 'GoRouter Example: builder'; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ class App extends StatelessWidget { | |
| @TypedGoRoute<HomeRoute>( | ||
| path: '/', | ||
| routes: <TypedGoRoute<GoRouteData>>[ | ||
| TypedGoRoute<SubRoute>(path: 'sub-route'), | ||
| TypedGoRoute<SubRoute>(path: 'sub-route', overrideOnExit: true), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This api is not ideal since the customer may not know why the flag is here if they don't have the context. I have several questions
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chunhtai Thank you for your response.
In my example here: If I do it like this, I can return to HomePage: final result = await Details2RouteData(
$extra: Details2PageExtra(data: 'Test data 2'),
).push<String?>(context);
if (!context.mounted) return;
await WidgetsBinding.instance.endOfFrame;
if (!context.mounted) return;
context.pop(result);However, the user will briefly see Details1Page before returning to Home. Therefore, I think a better solution would be to allow the default onExit parameter to be null, instead of always returning true.
What do you think if I handle it like this? final bool hasOverriddenOnExit = classElement.methods.any(
(method) => method.name == 'onExit',
); |
||
| ], | ||
| ) | ||
| class HomeRoute extends GoRouteData with $HomeRoute { | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:go_router_builder_example/not_override_on_exit_example.dart'; | ||
|
|
||
| void main() { | ||
| testWidgets('HomeScreen should return result from Sub2Screen', ( | ||
| WidgetTester tester, | ||
| ) async { | ||
| await tester.pumpWidget(App()); | ||
| expect(find.byType(HomeScreen), findsOne); | ||
|
|
||
| await tester.tap(find.widgetWithText(ElevatedButton, 'Go to sub 1 screen')); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(find.byType(Sub1Screen), findsOne); | ||
|
|
||
| await tester.tap(find.widgetWithText(ElevatedButton, 'Go to sub 2 screen')); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(find.byType(Sub2Screen), findsOne); | ||
|
|
||
| await tester.tap(find.widgetWithText(ElevatedButton, 'Go back to sub 1 screen')); | ||
| await tester.pumpAndSettle(); | ||
|
|
||
| expect(find.byType(HomeScreen), findsOne); | ||
|
|
||
| await tester.tap(find.widgetWithText(ElevatedButton, 'Sub2Screen')); | ||
|
|
||
| expect(find.byType(Sub1Screen), findsNothing); | ||
| expect(find.byType(Sub2Screen), findsNothing); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changelog entry is a bit verbose. For better readability, it's good practice for changelog entries to be concise. Consider removing implementation details like what happens when the flag is true and its default value.