Skip to content

Commit 185bc09

Browse files
authored
Fix RouterObserver didPop is not called when reverseTransitionDuratio… (#97171)
1 parent 2787eb0 commit 185bc09

File tree

2 files changed

+77
-14
lines changed

2 files changed

+77
-14
lines changed

packages/flutter/lib/src/widgets/navigator.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5023,11 +5023,15 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
50235023
bool? wasDebugLocked;
50245024
assert(() { wasDebugLocked = _debugLocked; _debugLocked = true; return true; }());
50255025
assert(_history.where(_RouteEntry.isRoutePredicate(route)).length == 1);
5026-
final _RouteEntry entry = _history.firstWhere(_RouteEntry.isRoutePredicate(route));
5027-
// For page-based route, the didPop can be called on any life cycle above
5028-
// pop.
5029-
assert(entry.currentState == _RouteLifecycle.popping ||
5030-
(entry.hasPage && entry.currentState.index < _RouteLifecycle.pop.index));
5026+
final int index = _history.indexWhere(_RouteEntry.isRoutePredicate(route));
5027+
final _RouteEntry entry = _history[index];
5028+
// For page-based route with zero transition, the finalizeRoute can be
5029+
// called on any life cycle above pop.
5030+
if (entry.hasPage && entry.currentState.index < _RouteLifecycle.pop.index) {
5031+
_observedRouteDeletions.add(_NavigatorPopObservation(route, _getRouteBefore(index - 1, _RouteEntry.willBePresentPredicate)?.route));
5032+
} else {
5033+
assert(entry.currentState == _RouteLifecycle.popping);
5034+
}
50315035
entry.finalize();
50325036
// finalizeRoute can be called during _flushHistoryUpdates if a pop
50335037
// finishes synchronously.

packages/flutter/test/widgets/navigator_test.dart

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,47 @@ void main() {
183183
expect('$exception', startsWith('Navigator operation requested with a context'));
184184
});
185185

186+
testWidgets('Zero transition page-based route correctly notifies observers when it is popped', (WidgetTester tester) async {
187+
final List<Page<void>> pages = <Page<void>>[
188+
const ZeroTransitionPage(name: 'Page 1'),
189+
const ZeroTransitionPage(name: 'Page 2'),
190+
];
191+
final List<NavigatorObservation> observations = <NavigatorObservation>[];
192+
193+
final TestObserver observer = TestObserver()
194+
..onPopped = (Route<dynamic>? route, Route<dynamic>? previousRoute) {
195+
observations.add(
196+
NavigatorObservation(
197+
current: route?.settings.name,
198+
previous: previousRoute?.settings.name,
199+
operation: 'pop',
200+
),
201+
);
202+
};
203+
final GlobalKey<NavigatorState> navigator = GlobalKey<NavigatorState>();
204+
await tester.pumpWidget(
205+
Directionality(
206+
textDirection: TextDirection.ltr,
207+
child: Navigator(
208+
key: navigator,
209+
pages: pages,
210+
observers: <NavigatorObserver>[observer],
211+
onPopPage: (Route<dynamic> route, dynamic result) {
212+
pages.removeLast();
213+
return route.didPop(result);
214+
},
215+
),
216+
),
217+
);
218+
219+
navigator.currentState!.pop();
220+
await tester.pump();
221+
222+
expect(observations.length, 1);
223+
expect(observations[0].current, 'Page 2');
224+
expect(observations[0].previous, 'Page 1');
225+
});
226+
186227
testWidgets('Navigator.of rootNavigator finds root Navigator', (WidgetTester tester) async {
187228
await tester.pumpWidget(MaterialApp(
188229
home: Material(
@@ -3941,6 +3982,22 @@ class AlwaysRemoveTransitionDelegate extends TransitionDelegate<void> {
39413982
}
39423983
}
39433984

3985+
class ZeroTransitionPage extends Page<void> {
3986+
const ZeroTransitionPage({
3987+
LocalKey? key,
3988+
Object? arguments,
3989+
required String name,
3990+
}) : super(key: key, name: name, arguments: arguments);
3991+
3992+
@override
3993+
Route<void> createRoute(BuildContext context) {
3994+
return NoAnimationPageRoute(
3995+
settings: this,
3996+
pageBuilder: (BuildContext context) => Text(name!),
3997+
);
3998+
}
3999+
}
4000+
39444001
class TestPage extends Page<void> {
39454002
const TestPage({
39464003
LocalKey? key,
@@ -3958,15 +4015,17 @@ class TestPage extends Page<void> {
39584015
}
39594016

39604017
class NoAnimationPageRoute extends PageRouteBuilder<void> {
3961-
NoAnimationPageRoute({required WidgetBuilder pageBuilder})
3962-
: super(pageBuilder: (BuildContext context, __, ___) {
3963-
return pageBuilder(context);
3964-
});
3965-
3966-
@override
3967-
AnimationController createAnimationController() {
3968-
return super.createAnimationController()..value = 1.0;
3969-
}
4018+
NoAnimationPageRoute({
4019+
RouteSettings? settings,
4020+
required WidgetBuilder pageBuilder
4021+
}) : super(
4022+
settings: settings,
4023+
transitionDuration: Duration.zero,
4024+
reverseTransitionDuration: Duration.zero,
4025+
pageBuilder: (BuildContext context, __, ___) {
4026+
return pageBuilder(context);
4027+
}
4028+
);
39704029
}
39714030

39724031
class StatefulTestWidget extends StatefulWidget {

0 commit comments

Comments
 (0)