diff --git a/packages/flutter/example/address_book/lib/main.dart b/packages/flutter/example/address_book/lib/main.dart index 1f24c3e296d..d5eb926d739 100644 --- a/packages/flutter/example/address_book/lib/main.dart +++ b/packages/flutter/example/address_book/lib/main.dart @@ -12,7 +12,6 @@ import 'package:sky/widgets/default_text_style.dart'; import 'package:sky/widgets/dialog.dart'; import 'package:sky/widgets/floating_action_button.dart'; import 'package:sky/widgets/flat_button.dart'; -import 'package:sky/widgets/focus.dart'; import 'package:sky/widgets/icon.dart'; import 'package:sky/widgets/icon_button.dart'; import 'package:sky/widgets/material.dart'; @@ -123,13 +122,10 @@ class AddressBookApp extends App { } Widget buildMain(Navigator navigator) { - return new Focus( - initialFocus: nameKey, - child: new Scaffold( - toolbar: buildToolBar(navigator), - body: buildBody(navigator), - floatingActionButton: buildFloatingActionButton(navigator) - ) + return new Scaffold( + toolbar: buildToolBar(navigator), + body: buildBody(navigator), + floatingActionButton: buildFloatingActionButton(navigator) ); } diff --git a/packages/flutter/example/stocks/lib/main.dart b/packages/flutter/example/stocks/lib/main.dart index d8c653baa6f..e119b61e9ab 100644 --- a/packages/flutter/example/stocks/lib/main.dart +++ b/packages/flutter/example/stocks/lib/main.dart @@ -22,7 +22,6 @@ import 'package:sky/widgets/drawer_divider.dart'; import 'package:sky/widgets/drawer_header.dart'; import 'package:sky/widgets/drawer_item.dart'; import 'package:sky/widgets/floating_action_button.dart'; -import 'package:sky/widgets/focus.dart'; import 'package:sky/widgets/icon.dart'; import 'package:sky/widgets/icon_button.dart'; import 'package:sky/widgets/modal_overlay.dart'; diff --git a/packages/flutter/example/stocks/lib/stock_home.dart b/packages/flutter/example/stocks/lib/stock_home.dart index 5ed43238049..4d0bbaed312 100644 --- a/packages/flutter/example/stocks/lib/stock_home.dart +++ b/packages/flutter/example/stocks/lib/stock_home.dart @@ -296,9 +296,6 @@ class StockHome extends StatefulComponent { ), ]; addMenuToOverlays(overlays); - return new Focus( - initialFocus: searchFieldKey, - child: new Stack(overlays) - ); + return new Stack(overlays); } } diff --git a/packages/flutter/lib/widgets/dialog.dart b/packages/flutter/lib/widgets/dialog.dart index f41b421b9e1..781137d6e1d 100644 --- a/packages/flutter/lib/widgets/dialog.dart +++ b/packages/flutter/lib/widgets/dialog.dart @@ -7,6 +7,7 @@ import 'dart:async'; import 'package:sky/theme/colors.dart' as colors; import 'package:sky/widgets/basic.dart'; import 'package:sky/widgets/default_text_style.dart'; +import 'package:sky/widgets/focus.dart'; import 'package:sky/widgets/material.dart'; import 'package:sky/widgets/navigator.dart'; import 'package:sky/widgets/scrollable_viewport.dart'; @@ -24,7 +25,7 @@ class Dialog extends Component { this.content, this.actions, this.onDismiss - }) : super(key: key); + }): super(key: key); /// The (optional) title of the dialog is displayed in a large font at the top /// of the dialog. @@ -111,7 +112,11 @@ Future showDialog(Navigator navigator, DialogBuilder builder) { navigator.push(new DialogRoute( completer: completer, builder: (navigator, route) { - return builder(navigator); + return new Focus( + key: new GlobalKey.fromObjectIdentity(route), + autofocus: true, + child: builder(navigator) + ); } )); return completer.future; diff --git a/packages/flutter/lib/widgets/focus.dart b/packages/flutter/lib/widgets/focus.dart index c50d434d622..513fc16c4aa 100644 --- a/packages/flutter/lib/widgets/focus.dart +++ b/packages/flutter/lib/widgets/focus.dart @@ -4,53 +4,239 @@ import 'package:sky/widgets/widget.dart'; -class Focus extends Inherited { +typedef void FocusChanged(GlobalKey key); - // TODO(ianh): This doesn't yet support nested scopes. We should not - // be telling our _currentlyFocusedKey that they are focused if we - // ourselves are not focused. Otherwise if you have a dialog with a - // text field over the top of a pane with a text field, they'll - // fight over control of the keyboard. +// _noFocusedScope is used by Focus to track the case where none of the Focus +// component's subscopes (e.g. dialogs) are focused. This is distinct from the +// focused scope being null, which means that we haven't yet decided which scope +// is focused and whichever is the first scope to ask for focus will get it. +final GlobalKey _noFocusedScope = new GlobalKey(); - Focus({ - GlobalKey key, - this.initialFocus, +class _FocusScope extends Inherited { + + _FocusScope({ + Key key, + this.scopeFocused: true, // are we focused in our ancestor scope? + this.focusedScope, // which of our descendant scopes is focused, if any? + this.focusedWidget, Widget child }) : super(key: key, child: child); - final GlobalKey initialFocus; + final bool scopeFocused; - GlobalKey _currentlyFocusedKey; - GlobalKey get currentlyFocusedKey { - if (_currentlyFocusedKey != null) - return _currentlyFocusedKey; - return initialFocus; + // These are mutable because we implicitly changed them when they're null in + // certain cases, basically pretending retroactively that we were constructed + // with the right keys. + GlobalKey focusedScope; + GlobalKey focusedWidget; + + // The ...IfUnset() methods don't need to notify descendants because by + // definition they are only going to make a change the very first time that + // our state is checked. + + void _setFocusedWidgetIfUnset(GlobalKey key) { + assert(parent is Focus); + (parent as Focus)._setFocusedWidgetIfUnset(key); // TODO(ianh): remove cast once analyzer is cleverer + focusedWidget = (parent as Focus)._focusedWidget; + focusedScope = (parent as Focus)._focusedScope == _noFocusedScope ? null : (parent as Focus)._focusedScope; } - void set currentlyFocusedKey(GlobalKey value) { - if (value != _currentlyFocusedKey) { - _currentlyFocusedKey = value; - notifyDescendants(); + + void _setFocusedScopeIfUnset(GlobalKey key) { + assert(parent is Focus); + (parent as Focus)._setFocusedScopeIfUnset(key); // TODO(ianh): remove cast once analyzer is cleverer + assert(focusedWidget == (parent as Focus)._focusedWidget); + focusedScope = (parent as Focus)._focusedScope == _noFocusedScope ? null : (parent as Focus)._focusedScope; + } + + bool syncShouldNotify(_FocusScope old) { + assert(parent is Focus); + if (scopeFocused != old.scopeFocused) + return true; + if (!scopeFocused) + return false; + if (focusedScope != old.focusedScope) + return true; + if (focusedScope != null) + return false; + if (focusedWidget != old.focusedWidget) + return true; + return false; + } + +} + +class Focus extends StatefulComponent { + + Focus({ + GlobalKey key, // key is required if this is a nested Focus scope + this.autofocus: false, + this.child + }) : super(key: key) { + assert(!autofocus || key != null); + } + + bool autofocus; + Widget child; + + void syncFields(Focus source) { + autofocus = source.autofocus; + child = source.child; + } + + + GlobalKey _focusedWidget; // when null, the first component to ask if it's focused will get the focus + GlobalKey _currentlyRegisteredWidgetRemovalListenerKey; + + void _setFocusedWidget(GlobalKey key) { + setState(() { + _focusedWidget = key; + if (_focusedScope == null) + _focusedScope = _noFocusedScope; + }); + _updateWidgetRemovalListener(key); + } + + void _setFocusedWidgetIfUnset(GlobalKey key) { + if (_focusedWidget == null && (_focusedScope == null || _focusedScope == _noFocusedScope)) { + _focusedWidget = key; + _focusedScope = _noFocusedScope; + _updateWidgetRemovalListener(key); } } - void syncState(Focus old) { - _currentlyFocusedKey = old._currentlyFocusedKey; - super.syncState(old); - } + void _widgetRemoved(GlobalKey key) { + assert(_focusedWidget == key); + _currentlyRegisteredWidgetRemovalListenerKey = null; + setState(() { + _focusedWidget = null; + }); + } - static bool at(Component component) { + void _updateWidgetRemovalListener(GlobalKey key) { + if (_currentlyRegisteredWidgetRemovalListenerKey != key) { + if (_currentlyRegisteredWidgetRemovalListenerKey != null) + GlobalKey.unregisterRemovalListener(_currentlyRegisteredWidgetRemovalListenerKey, _widgetRemoved); + if (key != null) + GlobalKey.registerRemovalListener(key, _widgetRemoved); + _currentlyRegisteredWidgetRemovalListenerKey = key; + } + } + + + GlobalKey _focusedScope; // when null, the first scope to ask if it's focused will get the focus + GlobalKey _currentlyRegisteredScopeRemovalListenerKey; + + void _setFocusedScope(GlobalKey key) { + setState(() { + _focusedScope = key; + }); + _updateScopeRemovalListener(key); + } + + void _setFocusedScopeIfUnset(GlobalKey key) { + if (_focusedScope == null) { + _focusedScope = key; + _updateScopeRemovalListener(key); + } + } + + void _scopeRemoved(GlobalKey key) { + assert(_focusedScope == key); + _currentlyRegisteredScopeRemovalListenerKey = null; + setState(() { + _focusedScope = null; + }); + } + + void _updateScopeRemovalListener(GlobalKey key) { + if (_currentlyRegisteredScopeRemovalListenerKey != key) { + if (_currentlyRegisteredScopeRemovalListenerKey != null) + GlobalKey.unregisterRemovalListener(_currentlyRegisteredScopeRemovalListenerKey, _scopeRemoved); + if (key != null) + GlobalKey.registerRemovalListener(key, _scopeRemoved); + _currentlyRegisteredScopeRemovalListenerKey = key; + } + } + + + bool _didAutoFocus = false; + void didMount() { + if (autofocus && !_didAutoFocus) { + _didAutoFocus = true; + Focus._moveScopeTo(this); + } + _updateWidgetRemovalListener(_focusedWidget); + _updateScopeRemovalListener(_focusedScope); + super.didMount(); + } + + void didUnmount() { + _updateWidgetRemovalListener(null); + _updateScopeRemovalListener(null); + super.didUnmount(); + } + + Widget build() { + return new _FocusScope( + scopeFocused: Focus._atScope(this), + focusedScope: _focusedScope == _noFocusedScope ? null : _focusedScope, + focusedWidget: _focusedWidget, + child: child + ); + } + + static bool at(Component component, { bool autofocus: true }) { assert(component != null); assert(component.key is GlobalKey); - Focus focus = component.inheritedOfType(Focus); - return focus == null || focus.currentlyFocusedKey == component.key; + _FocusScope focusScope = component.inheritedOfType(_FocusScope); + if (focusScope != null) { + if (autofocus) + focusScope._setFocusedWidgetIfUnset(component.key); + return focusScope.scopeFocused && + focusScope.focusedScope == null && + focusScope.focusedWidget == component.key; + } + return true; } + static bool _atScope(Focus component, { bool autofocus: true }) { + assert(component != null); + _FocusScope focusScope = component.inheritedOfType(_FocusScope); + if (focusScope != null) { + if (autofocus) + focusScope._setFocusedScopeIfUnset(component.key); + assert(component.key != null); + return focusScope.scopeFocused && + focusScope.focusedScope == component.key; + } + return true; + } + + // Don't call moveTo() from your build() function, it's intended to be called + // from event listeners, e.g. in response to a finger tap or tab key. + static void moveTo(Component component) { assert(component != null); assert(component.key is GlobalKey); - Focus focus = component.inheritedOfType(Focus); - if (focus != null) - focus.currentlyFocusedKey = component.key; + _FocusScope focusScope = component.inheritedOfType(_FocusScope); + if (focusScope != null) { + assert(focusScope.parent is Focus); + (focusScope.parent as Focus)._setFocusedWidget(component.key); // TODO(ianh): remove cast once analyzer is cleverer + } + } + + static void _moveScopeTo(Focus component) { + assert(component != null); + assert(component.key != null); + _FocusScope focusScope = component.inheritedOfType(_FocusScope); + if (focusScope != null) { + assert(focusScope.parent is Focus); + (focusScope.parent as Focus)._setFocusedScope(component.key); // TODO(ianh): remove cast once analyzer is cleverer + } + } + + String toStringName() { + return '${super.toStringName()}(focusedScope=$_focusedScope; focusedWidget=$_focusedWidget)'; } } diff --git a/packages/flutter/lib/widgets/navigator.dart b/packages/flutter/lib/widgets/navigator.dart index 327ec278fa4..969bab168e3 100644 --- a/packages/flutter/lib/widgets/navigator.dart +++ b/packages/flutter/lib/widgets/navigator.dart @@ -9,6 +9,7 @@ import 'package:sky/animation/animation_performance.dart'; import 'package:sky/animation/curves.dart'; import 'package:sky/widgets/animated_component.dart'; import 'package:sky/widgets/basic.dart'; +import 'package:sky/widgets/focus.dart'; import 'package:vector_math/vector_math.dart'; typedef Widget RouteBuilder(Navigator navigator, RouteBase route); @@ -280,6 +281,6 @@ class Navigator extends StatefulComponent { ); visibleRoutes.add(transition); } - return new Stack(visibleRoutes); + return new Focus(child: new Stack(visibleRoutes)); } } diff --git a/packages/flutter/lib/widgets/widget.dart b/packages/flutter/lib/widgets/widget.dart index cbafe4a77b0..16ad4eaa8ba 100644 --- a/packages/flutter/lib/widgets/widget.dart +++ b/packages/flutter/lib/widgets/widget.dart @@ -31,7 +31,7 @@ abstract class Key { class StringKey extends Key { StringKey(this.value) : super.constructor(); final String value; - String toString() => value; + String toString() => '[\'${value}\']'; bool operator==(other) => other is StringKey && other.value == value; int get hashCode => value.hashCode; } @@ -44,10 +44,86 @@ class ObjectKey extends Key { int get hashCode => identityHashCode(value); } +typedef void GlobalKeyRemovalListener(GlobalKey key); + abstract class GlobalKey extends Key { GlobalKey.constructor() : super.constructor(); // so that subclasses can call us, since the Key() factory constructor shadows the implicit constructor factory GlobalKey({ String label }) => new LabeledGlobalKey(label); factory GlobalKey.fromObjectIdentity(Object value) => new GlobalObjectKey(value); + + static final Map _registry = new Map(); + static final Map _debugDuplicates = new Map(); + static final Map> _removalListeners = new Map>(); + static final Set _removedKeys = new Set(); + + void _register(Widget widget) { + assert(() { + if (_registry.containsKey(this)) { + int oldCount = _debugDuplicates.putIfAbsent(this, () => 1); + assert(oldCount >= 1); + _debugDuplicates[this] = oldCount + 1; + } + return true; + }); + _registry[this] = widget; + } + + void _unregister(Widget widget) { + assert(() { + if (_registry.containsKey(this) && _debugDuplicates.containsKey(this)) { + int oldCount = _debugDuplicates[this]; + assert(oldCount >= 2); + if (oldCount == 2) { + _debugDuplicates.remove(this); + } else { + _debugDuplicates[this] = oldCount - 1; + } + } + return true; + }); + if (_registry[this] == widget) { + _registry.remove(this); + _removedKeys.add(this); + } + } + + static bool _notifyingListeners = false; + + static void registerRemovalListener(GlobalKey key, GlobalKeyRemovalListener listener) { + assert(!_notifyingListeners); + assert(key != null); + if (!_removalListeners.containsKey(key)) + _removalListeners[key] = new Set(); + bool added = _removalListeners[key].add(listener); + assert(added); + } + + static void unregisterRemovalListener(GlobalKey key, GlobalKeyRemovalListener listener) { + assert(!_notifyingListeners); + assert(key != null); + assert(_removalListeners.containsKey(key)); + bool removed = _removalListeners[key].remove(listener); + if (_removalListeners[key].isEmpty) + _removalListeners.remove(key); + assert(removed); + } + + static void _notifyListeners() { + assert(!_inRenderDirtyComponents); + assert(!Widget._notifyingMountStatus); + assert(_debugDuplicates.isEmpty); + _notifyingListeners = true; + for (GlobalKey key in _removedKeys) { + if (!_registry.containsKey(key) && _removalListeners.containsKey(key)) { + for (GlobalKeyRemovalListener listener in _removalListeners[key]) + listener(key); + _removalListeners.remove(key); + } + } + _removedKeys.clear(); + _notifyingListeners = false; + } + } class LabeledGlobalKey extends GlobalKey { @@ -150,45 +226,19 @@ abstract class Widget { _notifyingMountStatus = false; sky.tracing.end("Widget._notifyMountStatusChanged"); } - assert(_debugDuplicateGlobalKeys.isEmpty); + GlobalKey._notifyListeners(); } - static final Map _globalKeys = new Map(); - static final Map _debugDuplicateGlobalKeys = new Map(); - /// Override this function to learn when this [Widget] enters the widget tree. void didMount() { - if (key is GlobalKey) { - assert(() { - if (_globalKeys.containsKey(key)) { - int oldCount = _debugDuplicateGlobalKeys.putIfAbsent(key, () => 1); - assert(oldCount >= 1); - _debugDuplicateGlobalKeys[key] = oldCount + 1; - } - return true; - }); - _globalKeys[key] = this; - } + if (key is GlobalKey) + (key as GlobalKey)._register(this); // TODO(ianh): remove cast when analyzer is cleverer } /// Override this function to learn when this [Widget] leaves the widget tree. void didUnmount() { - if (key is GlobalKey) { - assert(() { - if (_globalKeys.containsKey(key) && _debugDuplicateGlobalKeys.containsKey(key)) { - int oldCount = _debugDuplicateGlobalKeys[key]; - assert(oldCount >= 2); - if (oldCount == 2) { - _debugDuplicateGlobalKeys.remove(key); - } else { - _debugDuplicateGlobalKeys[key] = oldCount - 1; - } - } - return true; - }); - if (_globalKeys[key] == this) - _globalKeys.remove(key); - } + if (key is GlobalKey) + (key as GlobalKey)._unregister(this); // TODO(ianh): remove cast when analyzer is cleverer } RenderObject _root; @@ -309,7 +359,7 @@ abstract class Widget { String toStringName() { if (key == null) return '$runtimeType(unkeyed; hashCode=$hashCode)'; - return '$runtimeType("$key"; hashCode=$hashCode)'; + return '$runtimeType($key; hashCode=$hashCode)'; } } @@ -372,7 +422,6 @@ abstract class Inherited extends TagNode { void _sync(Widget old, dynamic slot) { if (old != null) { - syncState(old); if (syncShouldNotify(old)) notifyDescendants(); } @@ -392,8 +441,7 @@ abstract class Inherited extends TagNode { walkChildren(notifyChildren); } - void syncState(Inherited old) { } - bool syncShouldNotify(Inherited old) => false; + bool syncShouldNotify(Inherited old); } @@ -495,8 +543,7 @@ abstract class Component extends Widget { : _order = _currentOrder + 1, super._withKey(key); - static Component _currentlyBuilding; - bool get _isBuilding => _currentlyBuilding == this; + bool _isBuilding = false; bool _dirty = true; @@ -571,17 +618,18 @@ abstract class Component extends Widget { oldBuilt = old._built; } + _isBuilding = true; + int lastOrder = _currentOrder; _currentOrder = _order; - _currentlyBuilding = this; _built = build(); - assert(_built != null); - _currentlyBuilding = null; _currentOrder = lastOrder; - + assert(_built != null); _built = syncChild(_built, oldBuilt, slot); assert(_built != null); assert(_built.parent == this); + _isBuilding = false; + _dirty = false; _root = _built.root; assert(_root == root); // in case a subclass reintroduces it @@ -596,7 +644,8 @@ abstract class Component extends Widget { } void _scheduleBuild() { - if (_isBuilding || _dirty || !_mounted) + assert(!_isBuilding); + if (_dirty || !_mounted) return; _dirty = true; _scheduleComponentForRender(this);