Commit e071f0ba authored by Adam Barth's avatar Adam Barth Committed by GitHub

Nesting MaterialApps should not assert (#4636)

Turns out we weren't managing focus correct between navigator routes because we
were missing a Focus widget above the routes. However, adding this widget
caused us to explode at startup because the initial route was trying to move
focus during the build phase.

This patch teaches Focus to have an initiallyFocusedScope, which can be use to
initialize the child focus scope.

Fixes #4065
parent b3780ebc
......@@ -21,19 +21,30 @@ class _FocusScope extends InheritedWidget {
_FocusScope({
Key key,
this.focusState,
this.scopeFocused: true, // are we focused in our ancestor scope?
this.focusedScope, // which of our descendant scopes is focused, if any?
this.scopeFocused,
this.focusedScope,
this.focusedWidget,
Widget child
}) : super(key: key, child: child);
}) : super(key: key, child: child) {
assert(scopeFocused != null);
}
/// The state for this focus scope.
///
/// This widget is always our direct parent widget.
final _FocusState focusState;
/// Whether this scope is focused in our ancestor focus scope.
final bool scopeFocused;
// These are mutable because we implicitly change them when they're null in
// certain cases, basically pretending retroactively that we were constructed
// with the right keys.
/// Which of our descendant scopes is focused, if any.
GlobalKey focusedScope;
/// Which of our descendant widgets is focused, if any.
GlobalKey focusedWidget;
// The _setFocusedWidgetIfUnset() methodsdon't need to notify descendants
......@@ -101,11 +112,20 @@ class Focus extends StatefulWidget {
/// The [key] argument must not be null.
Focus({
@required GlobalKey key,
this.initiallyFocusedScope,
this.child
}) : super(key: key) {
assert(key != null);
}
/// The global key of the [Focus] widget below this widget in the tree that
/// will be focused initially.
///
/// If non-null, a [Focus] widget with this key must be added to the tree
/// before the end of the current microtask in which the [Focus] widget was
/// initially constructed.
final GlobalKey initiallyFocusedScope;
/// The widget below this widget in the tree.
final Widget child;
......@@ -210,8 +230,15 @@ class _FocusState extends State<Focus> {
@override
void initState() {
super.initState();
_focusedScope = config.initiallyFocusedScope;
_updateWidgetRemovalListener(_focusedWidget);
_updateScopeRemovalListener(_focusedScope);
assert(() {
if (_focusedScope != null)
scheduleMicrotask(_debugCheckInitiallyFocusedScope);
return true;
});
}
@override
......@@ -221,6 +248,34 @@ class _FocusState extends State<Focus> {
super.dispose();
}
void _debugCheckInitiallyFocusedScope() {
assert(config.initiallyFocusedScope != null);
assert(() {
if (!mounted)
return true;
Widget widget = config.initiallyFocusedScope.currentWidget;
if (widget == null) {
throw new FlutterError(
'The initially focused scope is not in the tree.\n'
'When a Focus widget is given an initially focused scope, that focus '
'scope must be added to the tree before the end of the microtask in '
'which the Focus widget was first built. However, it is the end of '
'the microtask and ${config.initiallyFocusedScope} is not in the '
'tree.'
);
}
if (widget is! Focus) {
throw new FlutterError(
'The initially focused scope was not a Focus widget.\n'
'The initially focused scope for a Focus widget must be another '
'Focus widget. Instead, the initially focused scope was a '
'${widget.runtimeType} widget.'
);
}
return true;
});
}
GlobalKey _focusedWidget; // when null, the first widget to ask if it's focused will get the focus
GlobalKey _currentlyRegisteredWidgetRemovalListenerKey;
......
......@@ -4,6 +4,7 @@
import 'package:meta/meta.dart';
import 'focus.dart';
import 'framework.dart';
import 'overlay.dart';
......@@ -21,6 +22,12 @@ abstract class Route<T> {
/// The overlay entries for this route.
List<OverlayEntry> get overlayEntries => const <OverlayEntry>[];
/// The key this route will use for its root [Focus] widget, if any.
///
/// If this route is the first route shown by the navigator, the navigator
/// will initialize its [Focus] to this key.
GlobalKey get focusKey => null;
/// Called when the route is inserted into the navigator.
///
/// Use this to populate overlayEntries and add them to the overlay
......@@ -515,14 +522,23 @@ class NavigatorState extends State<Navigator> {
return true;
}
// TODO(abarth): We should be able to take a focusScopeKey as configuration
// information in case our parent wants to control whether we are focused.
final GlobalKey _focusScopeKey = new GlobalKey();
@override
Widget build(BuildContext context) {
assert(!_debugLocked);
assert(_history.isNotEmpty);
_hadTransaction = false;
return new Overlay(
key: _overlayKey,
initialEntries: _history.first.overlayEntries
final Route<dynamic> initialRoute = _history.first;
return new Focus(
key: _focusScopeKey,
initiallyFocusedScope: initialRoute.focusKey,
child: new Overlay(
key: _overlayKey,
initialEntries: initialRoute.overlayEntries
)
);
}
}
......
......@@ -431,7 +431,7 @@ class _ModalScopeState extends State<_ModalScope> {
);
}
contents = new Focus(
key: new GlobalObjectKey(config.route),
key: config.route.focusKey,
child: new RepaintBoundary(child: contents)
);
return contents;
......@@ -519,12 +519,31 @@ abstract class ModalRoute<T> extends TransitionRoute<T> with LocalHistoryRoute<T
return child;
}
@override
GlobalKey get focusKey => new GlobalObjectKey(this);
@override
void didPush() {
Focus.moveScopeTo(new GlobalObjectKey(this), context: navigator.context);
if (!settings.isInitialRoute) {
BuildContext overlayContext = navigator.overlay.context;
if (overlayContext == null) {
throw new FlutterError(
'Unable to find the BuildContext for the Navigator\'s overlay.\n'
'Did you remember to pass the settings object to the route\'s '
'constructor in your onGenerateRoute callback?'
);
}
Focus.moveScopeTo(focusKey, context: overlayContext);
}
super.didPush();
}
@override
void didPopNext(Route<dynamic> nextRoute) {
Focus.moveScopeTo(focusKey, context: navigator.overlay.context);
super.didPopNext(nextRoute);
}
// The API for subclasses to override - used by this class
/// Whether you can dismiss this route by tapping the modal barrier.
......
// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/material.dart';
void main() {
testWidgets('Can nest apps', (WidgetTester tester) async {
await tester.pumpWidget(
new MaterialApp(
home: new MaterialApp(
home: new Text('Home sweet home')
)
)
);
expect(find.text('Home sweet home'), findsOneWidget);
});
testWidgets('Focus handling', (WidgetTester tester) async {
GlobalKey inputKey = new GlobalKey();
await tester.pumpWidget(new MaterialApp(
home: new Material(
child: new Center(
child: new Input(key: inputKey, autofocus: true)
)
)
));
expect(Focus.at(inputKey.currentContext), isTrue);
});
}
......@@ -98,8 +98,8 @@ void main() {
)
);
case '/2': return new TestRoute<Null>(settings: settings, child: new Text('E'));
case '/3': return new TestRoute<Null> (settings: settings, child: new Text('F'));
case '/4': return new TestRoute<Null> (settings: settings, child: new Text('G'));
case '/3': return new TestRoute<Null>(settings: settings, child: new Text('F'));
case '/4': return new TestRoute<Null>(settings: settings, child: new Text('G'));
}
}
)
......
......@@ -26,10 +26,17 @@ void main() {
await tester.pumpWidget(new Navigator(
key: navigatorKey,
onGenerateRoute: (RouteSettings settings) {
if (settings.name == '/')
return new MaterialPageRoute<Null>(builder: (_) => new Container(child: new ThePositiveNumbers()));
else if (settings.name == '/second')
return new MaterialPageRoute<Null>(builder: (_) => new Container(child: new ThePositiveNumbers()));
if (settings.name == '/') {
return new MaterialPageRoute<Null>(
settings: settings,
builder: (_) => new Container(child: new ThePositiveNumbers())
);
} else if (settings.name == '/second') {
return new MaterialPageRoute<Null>(
settings: settings,
builder: (_) => new Container(child: new ThePositiveNumbers())
);
}
return null;
}
));
......
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