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

[go_router] fix Popping state and re-rendering scaffold at the same time doesn't update the URL on web [new] #8352

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
314 changes: 314 additions & 0 deletions packages/go_router/example/lib/stream_listener_router.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,314 @@
import 'dart:async';
import 'dart:developer';

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

void main() {
GoRouter.optionURLReflectsImperativeAPIs = true;

WidgetsFlutterBinding.ensureInitialized();

runApp(const MyApp());
}

/// A counter stream that emits a new value when the counter is incremented.
class CounterStream {
int _counter = 0;

final StreamController<int> _streamController =
StreamController<int>.broadcast();

/// The stream that emits a new value when the counter is incremented.
Stream<int> get stateStream => _streamController.stream.asBroadcastStream();

/// Increments the counter and emits a new value.
void increment() {
_streamController.sink.add(++_counter);
}
}

/// A counter stream that emits a new value when the counter is incremented.
final CounterStream counterStream = CounterStream();

/// A listener that listens to a stream and refreshes the router when the stream emits a new value.
class StreamListener extends ChangeNotifier {
/// Creates a stream listener.
StreamListener(Stream<dynamic> stream) {
notifyListeners();

_subscription = stream.asBroadcastStream().listen((_) {
notifyListeners();
});
}

late final StreamSubscription<dynamic> _subscription;

@override
void notifyListeners() {
super.notifyListeners();
log('refreshing the router');
}

@override
void dispose() {
_subscription.cancel();
super.dispose();
}
}

/// The main application widget.
class MyApp extends StatefulWidget {
/// Creates the main application widget.
const MyApp({super.key});

@override
State<MyApp> createState() => _MyAppState();
}

final GlobalKey<NavigatorState> _rootNavigatorKey = GlobalKey<NavigatorState>();

final GoRouter _router = GoRouter(
initialLocation: '/',
navigatorKey: _rootNavigatorKey,
refreshListenable: StreamListener(counterStream.stateStream),
routes: <RouteBase>[
ShellRoute(
builder: (BuildContext context, GoRouterState state, Widget child) {
return GenericPage(child: child);
},
routes: <RouteBase>[
GoRoute(
path: '/',
builder: (BuildContext context, GoRouterState state) =>
const GenericPage(showPushButton: true, path: 'a'),
routes: <RouteBase>[
GoRoute(
path: 'a',
name: 'a',
builder: (BuildContext context, GoRouterState state) =>
const GenericPage(showPushButton: true, path: 'b'),
routes: <RouteBase>[
GoRoute(
path: 'b',
name: 'b',
builder: (BuildContext context, GoRouterState state) =>
const GenericPage(showBackButton: true),
),
],
),
],
),
],
),
],
);

class _MyAppState extends State<MyApp> {
late StreamSubscription<int> _stateSubscription;

/// The current state of the counter.
int _currentState = 0;

@override
void initState() {
super.initState();
_stateSubscription = counterStream.stateStream.listen((int state) {
setState(() {
_currentState = state;
log('$_currentState:: "try double place to listen"');
});
});
}

@override
void dispose() {
_stateSubscription.cancel();
super.dispose();
}

@override
Widget build(BuildContext context) {
return MaterialApp.router(
title: 'Flutter Demo',
theme: ThemeData(
primarySwatch: Colors.blue,
),
routerConfig: _router,
);
}
}

/// A dialog test widget.
class DialogTest extends StatelessWidget {
/// Creates a dialog test widget.
const DialogTest({super.key});

@override
Widget build(BuildContext context) {
return Center(
child: Container(
width: 300,
height: 300,
alignment: Alignment.center,
child: Material(
color: Colors.white,
child: Column(
children:
<String>['Navigator::pop', 'GoRouter::pop'].map((String e) {
return InkWell(
child: SizedBox(
height: 60,
width: 300,
child: Row(
mainAxisAlignment: MainAxisAlignment.center,
children: <Widget>[
Text(e),
const Icon(Icons.close),
],
),
),
onTap: () {
if (e == 'GoRouter::pop') {
// WHEN THE USER PRESSES THIS BUTTON, THE URL
// DOESN'T CHANGE, BUT THE SCREEN DOES
counterStream
.increment(); // <- when removing this line the issue is gone
GoRouter.of(context).pop();
} else {
Navigator.of(context).pop();
}
},
);
}).toList(),
),
),
),
);
}
}

/// A generic page that can be used to display a page in the app.
class GenericPage extends StatefulWidget {
/// Creates a generic page.
const GenericPage({
this.child,
Key? key,
this.showPushButton = false,
this.showBackButton = false,
this.path,
}) : super(key: key ?? const ValueKey<String>('ShellWidget'));

/// The child widget to be displayed in the page.
final Widget? child;

/// Whether to show the push button.
final bool showPushButton;

/// Whether to show the back button.
final bool showBackButton;

/// The path of the page.
final String? path;

@override
State<GenericPage> createState() => _GenericPageState();
}

class _GenericPageState extends State<GenericPage> {
late StreamSubscription<int> _stateSubscription;
int _currentState = 0;
final GlobalKey<ScaffoldState> _scaffoldKey = GlobalKey<ScaffoldState>();
@override
void initState() {
super.initState();
_stateSubscription = counterStream.stateStream.listen((int state) {
setState(() {
_currentState = state;
});
});
}

@override
void dispose() {
_stateSubscription.cancel();
super.dispose();
}

@override
Widget build(BuildContext context) {
return Scaffold(
key: _scaffoldKey,
appBar: widget.child != null
? AppBar(
title: Text('Count: $_currentState'),
actions: <Widget>[
TextButton(
onPressed: () {
showDialog<void>(
context: context,
builder: (BuildContext context) {
return const DialogTest();
},
);
},
child: const Text('dialog1'),
),
TextButton(
onPressed: () {
showModalBottomSheet<void>(
context: context,
builder: (BuildContext context) {
return const DialogTest();
},
);
},
child: const Text('dialog2'),
),
TextButton(
onPressed: () {
_scaffoldKey.currentState?.openEndDrawer();
},
child: const Text('EndDrawer'),
),
],
)
: null,
endDrawer: const Drawer(
width: 200,
child: DialogTest(),
),
body: _buildWidget(context),
);
}

Widget _buildWidget(BuildContext context) {
if (widget.child != null) {
return widget.child!;
}

if (widget.showBackButton) {
return TextButton(
onPressed: () {
// WHEN THE USER PRESSES THIS BUTTON, THE URL
// DOESN'T CHANGE, BUT THE SCREEN DOES
counterStream
.increment(); // <- when removing this line the issue is gone
GoRouter.of(context).pop();
},
child: const Text('<- Go Back'),
);
}

if (widget.showPushButton) {
return TextButton(
onPressed: () {
GoRouter.of(context).goNamed(widget.path!);
},
child: const Text('Push ->'),
);
}

return Text('Current state: $_currentState');
}
}
18 changes: 17 additions & 1 deletion packages/go_router/lib/src/information_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ enum NavigatingType {
/// Restore the current match list with
/// [RouteInformationState.baseRouteMatchList].
restore,

/// Pop Current route.
pop,
ahyangnb marked this conversation as resolved.
Show resolved Hide resolved
}

/// The data class to be stored in [RouteInformation.state] to be used by
Expand All @@ -49,7 +52,9 @@ class RouteInformationState<T> {
this.completer,
this.baseRouteMatchList,
required this.type,
}) : assert((type == NavigatingType.go || type == NavigatingType.restore) ==
}) : assert((type == NavigatingType.go ||
type == NavigatingType.restore ||
type == NavigatingType.pop) ==
(completer == null)),
assert((type != NavigatingType.go) == (baseRouteMatchList != null));

Expand Down Expand Up @@ -227,6 +232,17 @@ class GoRouteInformationProvider extends RouteInformationProvider
return completer.future;
}

/// Save the pop state to value when remove top-most route.
void popSave<T>(String location, {required RouteMatchList base}) {
_value = RouteInformation(
uri: Uri.parse(location),
state: RouteInformationState<T>(
baseRouteMatchList: base,
type: NavigatingType.pop,
),
);
}

RouteInformation _valueInEngine;

void _platformReportsNewRouteInformation(RouteInformation routeInformation) {
Expand Down
3 changes: 3 additions & 0 deletions packages/go_router/lib/src/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ class GoRouteInformationParser extends RouteInformationParser<RouteMatchList> {
return baseRouteMatchList!.uri.toString() != newMatchList.uri.toString()
? newMatchList
: baseRouteMatchList;
case NavigatingType.pop:
// Will do nothing.
ahyangnb marked this conversation as resolved.
Show resolved Hide resolved
return baseRouteMatchList!;
}
}

Expand Down
4 changes: 4 additions & 0 deletions packages/go_router/lib/src/router.dart
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,10 @@ class GoRouter implements RouterConfig<RouteMatchList> {
return true;
}());
routerDelegate.pop<T>(result);
routeInformationProvider.popSave<T>(
routerDelegate.currentConfiguration.uri.toString(),
base: routerDelegate.currentConfiguration,
);
}

/// Refresh the route.
Expand Down
Loading