Unverified Commit fdab8546 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Minor doc, style, and perf updates to Navigator/Routes (#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
parent 50dfd137
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:convert'; import 'dart:convert';
import 'dart:typed_data'; import 'dart:typed_data';
......
...@@ -8,7 +8,6 @@ import 'package:flutter/rendering.dart'; ...@@ -8,7 +8,6 @@ import 'package:flutter/rendering.dart';
import 'package:flutter/services.dart'; import 'package:flutter/services.dart';
import 'basic.dart'; import 'basic.dart';
import 'container.dart';
import 'debug.dart'; import 'debug.dart';
import 'framework.dart'; import 'framework.dart';
import 'gesture_detector.dart'; import 'gesture_detector.dart';
...@@ -113,10 +112,8 @@ class ModalBarrier extends StatelessWidget { ...@@ -113,10 +112,8 @@ class ModalBarrier extends StatelessWidget {
opaque: true, opaque: true,
child: ConstrainedBox( child: ConstrainedBox(
constraints: const BoxConstraints.expand(), constraints: const BoxConstraints.expand(),
child: color == null ? null : DecoratedBox( child: color == null ? null : ColoredBox(
decoration: BoxDecoration( color: color!,
color: color,
),
), ),
), ),
), ),
......
...@@ -399,20 +399,30 @@ abstract class Route<T> { ...@@ -399,20 +399,30 @@ abstract class Route<T> {
/// ///
/// See also: /// See also:
/// ///
/// * [changedExternalState], which is called when the [Navigator] rebuilds. /// * [changedExternalState], which is called when the [Navigator] has
/// updated in some manner that might affect the routes.
@protected @protected
@mustCallSuper @mustCallSuper
void changedInternalState() { } void changedInternalState() { }
/// Called whenever the [Navigator] has its widget rebuilt, to indicate that /// Called whenever the [Navigator] has updated in some manner that might
/// the route may wish to rebuild as well. /// affect routes, to indicate that the route may wish to rebuild as well.
/// ///
/// This is called by the [Navigator] whenever the [NavigatorState]'s /// This is called by the [Navigator] whenever the
/// [State.widget] changes, for example because the [MaterialApp] has been rebuilt. /// [NavigatorState]'s [State.widget] changes (as in [State.didUpdateWidget]),
/// This ensures that routes that directly refer to the state of the widget /// for example because the [MaterialApp] has been rebuilt. This
/// that built the [MaterialApp] will be notified when that widget rebuilds, /// ensures that routes that directly refer to the state of the
/// since it would otherwise be difficult to notify the routes that state they /// widget that built the [MaterialApp] will be notified when that
/// depend on may have changed. /// widget rebuilds, since it would otherwise be difficult to notify
/// the routes that state they depend on may have changed.
///
/// It is also called whenever the [Navigator]'s dependencies change
/// (as in [State.didChangeDependencies]). This allows routes to use the
/// [Navigator]'s context ([NavigatorState.context]), for example in
/// [ModalRoute.barrierColor], and update accordingly.
///
/// The [ModalRoute] subclass overrides this to force the barrier
/// overlay to rebuild.
/// ///
/// See also: /// See also:
/// ///
...@@ -3442,6 +3452,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res ...@@ -3442,6 +3452,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin, Res
void didChangeDependencies() { void didChangeDependencies() {
super.didChangeDependencies(); super.didChangeDependencies();
_updateHeroController(HeroControllerScope.of(context)); _updateHeroController(HeroControllerScope.of(context));
for (final _RouteEntry entry in _history)
entry.route.changedExternalState();
} }
void _updateHeroController(HeroController? newHeroController) { void _updateHeroController(HeroController? newHeroController) {
......
...@@ -871,15 +871,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -871,15 +871,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// Creates a route that blocks interaction with previous routes. /// Creates a route that blocks interaction with previous routes.
ModalRoute({ ModalRoute({
RouteSettings? settings, RouteSettings? settings,
ui.ImageFilter? filter, this.filter,
}) : _filter = filter, }) : super(settings: settings);
super(settings: settings);
/// The filter to add to the barrier. /// The filter to add to the barrier.
/// ///
/// If given, this filter will be applied to the modal barrier using /// If given, this filter will be applied to the modal barrier using
/// [BackdropFilter]. This allows blur effects, for example. /// [BackdropFilter]. This allows blur effects, for example.
final ui.ImageFilter? _filter; final ui.ImageFilter? filter;
// The API for general users of this class // The API for general users of this class
...@@ -1130,9 +1129,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1130,9 +1129,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// ///
/// If [barrierDismissible] is false, then tapping the barrier has no effect. /// If [barrierDismissible] is false, then tapping the barrier has no effect.
/// ///
/// If this getter would ever start returning a different value, the /// If this getter would ever start returning a different value,
/// [Route.changedInternalState] should be invoked so that the change can take /// either [changedInternalState] or [changedExternalState] should
/// effect. /// be invoked so that the change can take effect.
///
/// It is safe to use `navigator.context` to look up inherited
/// widgets here, because the [Navigator] calls
/// [changedExternalState] whenever its dependencies change, and
/// [changedExternalState] causes the modal barrier to rebuild.
/// ///
/// See also: /// See also:
/// ///
...@@ -1154,6 +1158,15 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1154,6 +1158,15 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// If [semanticsDismissible] is false, then modal barrier semantics are /// If [semanticsDismissible] is false, then modal barrier semantics are
/// excluded from the semantics tree and tapping on the modal barrier /// excluded from the semantics tree and tapping on the modal barrier
/// has no effect. /// has no effect.
///
/// If this getter would ever start returning a different value,
/// either [changedInternalState] or [changedExternalState] should
/// be invoked so that the change can take effect.
///
/// It is safe to use `navigator.context` to look up inherited
/// widgets here, because the [Navigator] calls
/// [changedExternalState] whenever its dependencies change, and
/// [changedExternalState] causes the modal barrier to rebuild.
bool get semanticsDismissible => true; bool get semanticsDismissible => true;
/// {@template flutter.widgets.ModalRoute.barrierColor} /// {@template flutter.widgets.ModalRoute.barrierColor}
...@@ -1174,22 +1187,24 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1174,22 +1187,24 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// transparent to the specified color. /// transparent to the specified color.
/// {@endtemplate} /// {@endtemplate}
/// ///
/// If this getter would ever start returning a different color, the /// If this getter would ever start returning a different color, one
/// [Route.changedInternalState] should be invoked so that the change can take /// of the [changedInternalState] or [changedExternalState] methods
/// effect. /// should be invoked so that the change can take effect.
///
/// It is safe to use `navigator.context` to look up inherited
/// widgets here, because the [Navigator] calls
/// [changedExternalState] whenever its dependencies change, and
/// [changedExternalState] causes the modal barrier to rebuild.
/// ///
/// {@tool snippet} /// {@tool snippet}
/// ///
/// It is safe to use `navigator.context` here. For example, to make /// For example, to make the barrier color use the theme's
/// the barrier color use the theme's background color, one could say: /// background color, one could say:
/// ///
/// ```dart /// ```dart
/// Color get barrierColor => Theme.of(navigator.context).backgroundColor; /// Color get barrierColor => Theme.of(navigator.context).backgroundColor;
/// ``` /// ```
/// ///
/// The [Navigator] causes the [ModalRoute]'s modal barrier overlay entry
/// to rebuild any time its dependencies change.
///
/// {@end-tool} /// {@end-tool}
/// ///
/// See also: /// See also:
...@@ -1213,9 +1228,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1213,9 +1228,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// usually darkened by the modal barrier. /// usually darkened by the modal barrier.
/// {@endtemplate} /// {@endtemplate}
/// ///
/// If this getter would ever start returning a different label, the /// If this getter would ever start returning a different label,
/// [Route.changedInternalState] should be invoked so that the change can take /// either [changedInternalState] or [changedExternalState] should
/// effect. /// be invoked so that the change can take effect.
///
/// It is safe to use `navigator.context` to look up inherited
/// widgets here, because the [Navigator] calls
/// [changedExternalState] whenever its dependencies change, and
/// [changedExternalState] causes the modal barrier to rebuild.
/// ///
/// See also: /// See also:
/// ///
...@@ -1236,9 +1256,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1236,9 +1256,14 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
/// While the route is animating into position, the color is animated from /// While the route is animating into position, the color is animated from
/// transparent to the specified [barrierColor]. /// transparent to the specified [barrierColor].
/// ///
/// If this getter would ever start returning a different curve, the /// If this getter would ever start returning a different curve,
/// [changedInternalState] should be invoked so that the change can take /// either [changedInternalState] or [changedExternalState] should
/// effect. /// be invoked so that the change can take effect.
///
/// It is safe to use `navigator.context` to look up inherited
/// widgets here, because the [Navigator] calls
/// [changedExternalState] whenever its dependencies change, and
/// [changedExternalState] causes the modal barrier to rebuild.
/// ///
/// It defaults to [Curves.ease]. /// It defaults to [Curves.ease].
/// ///
...@@ -1451,6 +1476,7 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1451,6 +1476,7 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
@override @override
void changedExternalState() { void changedExternalState() {
super.changedExternalState(); super.changedExternalState();
_modalBarrier.markNeedsBuild();
if (_scopeKey.currentState != null) if (_scopeKey.currentState != null)
_scopeKey.currentState!._forceRebuildPage(); _scopeKey.currentState!._forceRebuildPage();
} }
...@@ -1493,9 +1519,9 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T ...@@ -1493,9 +1519,9 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
barrierSemanticsDismissible: semanticsDismissible, barrierSemanticsDismissible: semanticsDismissible,
); );
} }
if (_filter != null) { if (filter != null) {
barrier = BackdropFilter( barrier = BackdropFilter(
filter: _filter!, filter: filter!,
child: barrier, child: barrier,
); );
} }
......
...@@ -1039,7 +1039,7 @@ void main() { ...@@ -1039,7 +1039,7 @@ void main() {
final dynamic renderObject = tester.renderObject(find.byType(Overlay)); final dynamic renderObject = tester.renderObject(find.byType(Overlay));
expect(renderObject.clipBehavior, equals(Clip.hardEdge)); expect(renderObject.clipBehavior, equals(Clip.hardEdge));
for(final Clip clip in Clip.values) { for (final Clip clip in Clip.values) {
await tester.pumpWidget( await tester.pumpWidget(
Directionality( Directionality(
textDirection: TextDirection.ltr, textDirection: TextDirection.ltr,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment