Skip to content

Commit fdab854

Browse files
authored
Minor doc, style, and perf updates to Navigator/Routes (flutter#71689)
* Minor doc, style, and perf updates to Navigator/Routes These are minor fixes I ended up making while working on a larger project that never went anywhere. - Used a ColoredBox instead of a DecoratedBox for ModalBarrier (probably a trivial memory/perf win). - A bunch of Navigator documentation fixes around when things rebuild. - Mark routes dirty when the Navigator has a dependency change. I cannot find a way to test this because as far as I can tell it makes no actual difference to when things rebuild because whenever the Navigator rebuilds the Overlay rebuilds and whenever that happens every OverlayEntry rebuilds, but in theory that's not guaranteed so this is sort of a correctness fix. It may even be a perf loss. We do something similar in didUpdateWidget already. I could be convinced to maybe remove these... - Make ModalRoute.filter public like everything else. - Made ModalRoute update its barrier when it gets an update, in case e.g. the modal barrier depends on inherited widgets via the navigator context. Again, not sure of any way to detect this, it might actually be moot, but it seems to be the technically correct solution? - Minor style fixes. All in all I couldn't figure out a way to test any of this (I wrote multiple large tests but it turns out they all already pass on master and are effectively redundant with existing tests). * Remove extraneous blank line
1 parent 50dfd13 commit fdab854

File tree

5 files changed

+73
-39
lines changed

5 files changed

+73
-39
lines changed

packages/flutter/lib/src/services/message_codecs.dart

-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
65
import 'dart:convert';
76
import 'dart:typed_data';
87

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

+2-5
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import 'package:flutter/rendering.dart';
88
import 'package:flutter/services.dart';
99

1010
import 'basic.dart';
11-
import 'container.dart';
1211
import 'debug.dart';
1312
import 'framework.dart';
1413
import 'gesture_detector.dart';
@@ -113,10 +112,8 @@ class ModalBarrier extends StatelessWidget {
113112
opaque: true,
114113
child: ConstrainedBox(
115114
constraints: const BoxConstraints.expand(),
116-
child: color == null ? null : DecoratedBox(
117-
decoration: BoxDecoration(
118-
color: color,
119-
),
115+
child: color == null ? null : ColoredBox(
116+
color: color!,
120117
),
121118
),
122119
),

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

+21-9
Original file line numberDiff line numberDiff line change
@@ -399,20 +399,30 @@ abstract class Route<T> {
399399
///
400400
/// See also:
401401
///
402-
/// * [changedExternalState], which is called when the [Navigator] rebuilds.
402+
/// * [changedExternalState], which is called when the [Navigator] has
403+
/// updated in some manner that might affect the routes.
403404
@protected
404405
@mustCallSuper
405406
void changedInternalState() { }
406407

407-
/// Called whenever the [Navigator] has its widget rebuilt, to indicate that
408-
/// the route may wish to rebuild as well.
408+
/// Called whenever the [Navigator] has updated in some manner that might
409+
/// affect routes, to indicate that the route may wish to rebuild as well.
409410
///
410-
/// This is called by the [Navigator] whenever the [NavigatorState]'s
411-
/// [State.widget] changes, for example because the [MaterialApp] has been rebuilt.
412-
/// This ensures that routes that directly refer to the state of the widget
413-
/// that built the [MaterialApp] will be notified when that widget rebuilds,
414-
/// since it would otherwise be difficult to notify the routes that state they
415-
/// depend on may have changed.
411+
/// This is called by the [Navigator] whenever the
412+
/// [NavigatorState]'s [State.widget] changes (as in [State.didUpdateWidget]),
413+
/// for example because the [MaterialApp] has been rebuilt. This
414+
/// ensures that routes that directly refer to the state of the
415+
/// widget that built the [MaterialApp] will be notified when that
416+
/// widget rebuilds, since it would otherwise be difficult to notify
417+
/// the routes that state they depend on may have changed.
418+
///
419+
/// It is also called whenever the [Navigator]'s dependencies change
420+
/// (as in [State.didChangeDependencies]). This allows routes to use the
421+
/// [Navigator]'s context ([NavigatorState.context]), for example in
422+
/// [ModalRoute.barrierColor], and update accordingly.
423+
///
424+
/// The [ModalRoute] subclass overrides this to force the barrier
425+
/// overlay to rebuild.
416426
///
417427
/// See also:
418428
///
@@ -3442,6 +3452,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
34423452
void didChangeDependencies() {
34433453
super.didChangeDependencies();
34443454
_updateHeroController(HeroControllerScope.of(context));
3455+
for (final _RouteEntry entry in _history)
3456+
entry.route.changedExternalState();
34453457
}
34463458

34473459
void _updateHeroController(HeroController? newHeroController) {

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

+49-23
Original file line numberDiff line numberDiff line change
@@ -871,15 +871,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
871871
/// Creates a route that blocks interaction with previous routes.
872872
ModalRoute({
873873
RouteSettings? settings,
874-
ui.ImageFilter? filter,
875-
}) : _filter = filter,
876-
super(settings: settings);
874+
this.filter,
875+
}) : super(settings: settings);
877876

878877
/// The filter to add to the barrier.
879878
///
880879
/// If given, this filter will be applied to the modal barrier using
881880
/// [BackdropFilter]. This allows blur effects, for example.
882-
final ui.ImageFilter? _filter;
881+
final ui.ImageFilter? filter;
883882

884883
// The API for general users of this class
885884

@@ -1130,9 +1129,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
11301129
///
11311130
/// If [barrierDismissible] is false, then tapping the barrier has no effect.
11321131
///
1133-
/// If this getter would ever start returning a different value, the
1134-
/// [Route.changedInternalState] should be invoked so that the change can take
1135-
/// effect.
1132+
/// If this getter would ever start returning a different value,
1133+
/// either [changedInternalState] or [changedExternalState] should
1134+
/// be invoked so that the change can take effect.
1135+
///
1136+
/// It is safe to use `navigator.context` to look up inherited
1137+
/// widgets here, because the [Navigator] calls
1138+
/// [changedExternalState] whenever its dependencies change, and
1139+
/// [changedExternalState] causes the modal barrier to rebuild.
11361140
///
11371141
/// See also:
11381142
///
@@ -1154,6 +1158,15 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
11541158
/// If [semanticsDismissible] is false, then modal barrier semantics are
11551159
/// excluded from the semantics tree and tapping on the modal barrier
11561160
/// has no effect.
1161+
///
1162+
/// If this getter would ever start returning a different value,
1163+
/// either [changedInternalState] or [changedExternalState] should
1164+
/// be invoked so that the change can take effect.
1165+
///
1166+
/// It is safe to use `navigator.context` to look up inherited
1167+
/// widgets here, because the [Navigator] calls
1168+
/// [changedExternalState] whenever its dependencies change, and
1169+
/// [changedExternalState] causes the modal barrier to rebuild.
11571170
bool get semanticsDismissible => true;
11581171

11591172
/// {@template flutter.widgets.ModalRoute.barrierColor}
@@ -1174,22 +1187,24 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
11741187
/// transparent to the specified color.
11751188
/// {@endtemplate}
11761189
///
1177-
/// If this getter would ever start returning a different color, the
1178-
/// [Route.changedInternalState] should be invoked so that the change can take
1179-
/// effect.
1190+
/// If this getter would ever start returning a different color, one
1191+
/// of the [changedInternalState] or [changedExternalState] methods
1192+
/// should be invoked so that the change can take effect.
1193+
///
1194+
/// It is safe to use `navigator.context` to look up inherited
1195+
/// widgets here, because the [Navigator] calls
1196+
/// [changedExternalState] whenever its dependencies change, and
1197+
/// [changedExternalState] causes the modal barrier to rebuild.
11801198
///
11811199
/// {@tool snippet}
11821200
///
1183-
/// It is safe to use `navigator.context` here. For example, to make
1184-
/// the barrier color use the theme's background color, one could say:
1201+
/// For example, to make the barrier color use the theme's
1202+
/// background color, one could say:
11851203
///
11861204
/// ```dart
11871205
/// Color get barrierColor => Theme.of(navigator.context).backgroundColor;
11881206
/// ```
11891207
///
1190-
/// The [Navigator] causes the [ModalRoute]'s modal barrier overlay entry
1191-
/// to rebuild any time its dependencies change.
1192-
///
11931208
/// {@end-tool}
11941209
///
11951210
/// See also:
@@ -1213,9 +1228,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
12131228
/// usually darkened by the modal barrier.
12141229
/// {@endtemplate}
12151230
///
1216-
/// If this getter would ever start returning a different label, the
1217-
/// [Route.changedInternalState] should be invoked so that the change can take
1218-
/// effect.
1231+
/// If this getter would ever start returning a different label,
1232+
/// either [changedInternalState] or [changedExternalState] should
1233+
/// be invoked so that the change can take effect.
1234+
///
1235+
/// It is safe to use `navigator.context` to look up inherited
1236+
/// widgets here, because the [Navigator] calls
1237+
/// [changedExternalState] whenever its dependencies change, and
1238+
/// [changedExternalState] causes the modal barrier to rebuild.
12191239
///
12201240
/// See also:
12211241
///
@@ -1236,9 +1256,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
12361256
/// While the route is animating into position, the color is animated from
12371257
/// transparent to the specified [barrierColor].
12381258
///
1239-
/// If this getter would ever start returning a different curve, the
1240-
/// [changedInternalState] should be invoked so that the change can take
1241-
/// effect.
1259+
/// If this getter would ever start returning a different curve,
1260+
/// either [changedInternalState] or [changedExternalState] should
1261+
/// be invoked so that the change can take effect.
1262+
///
1263+
/// It is safe to use `navigator.context` to look up inherited
1264+
/// widgets here, because the [Navigator] calls
1265+
/// [changedExternalState] whenever its dependencies change, and
1266+
/// [changedExternalState] causes the modal barrier to rebuild.
12421267
///
12431268
/// It defaults to [Curves.ease].
12441269
///
@@ -1451,6 +1476,7 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
14511476
@override
14521477
void changedExternalState() {
14531478
super.changedExternalState();
1479+
_modalBarrier.markNeedsBuild();
14541480
if (_scopeKey.currentState != null)
14551481
_scopeKey.currentState!._forceRebuildPage();
14561482
}
@@ -1493,9 +1519,9 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
14931519
barrierSemanticsDismissible: semanticsDismissible,
14941520
);
14951521
}
1496-
if (_filter != null) {
1522+
if (filter != null) {
14971523
barrier = BackdropFilter(
1498-
filter: _filter!,
1524+
filter: filter!,
14991525
child: barrier,
15001526
);
15011527
}

packages/flutter/test/widgets/overlay_test.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -1039,7 +1039,7 @@ void main() {
10391039
final dynamic renderObject = tester.renderObject(find.byType(Overlay));
10401040
expect(renderObject.clipBehavior, equals(Clip.hardEdge));
10411041

1042-
for(final Clip clip in Clip.values) {
1042+
for (final Clip clip in Clip.values) {
10431043
await tester.pumpWidget(
10441044
Directionality(
10451045
textDirection: TextDirection.ltr,

0 commit comments

Comments
 (0)