Scoped focus, so you can have a dialog with input controls and not lose your...

Scoped focus, so you can have a dialog with input controls and not lose your focus in the background.

This introduces a GlobalKey registry so that you can tell when a key
has gone away (so you can unfocus dead dialogs).

Also I added an assert that you're not calling setState() during a
build. It turns out that doing so means you have a bug, because since
you're dirty already (you're building), you won't get rebuilt.

The focus code itself is gnarly. It uses a Component and an internal
Inherited TagNode to manage the focus state, and does crazy things
like updating its state during build to pretend retroactively that it
was built with some other state, once someone asks for focus the first
time (i.e. the first time it's examined, so you can't tell that it was
in a different state before). It does this so that it can autofocus
controls which otherwise wouldn't be focused.

This moves all the focus management into Navigator and showDialog(),
so the complexity is all buried deep and not visible to apps,
hopefully.

To do something like two tabs that each have an Input widget that
needs to be focused when you switch panes, you'd need to have two
Focus objects, one in each tab, and you need to set their autofocus to
true (maybe that should be the default?).

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)
     );
   }
 

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';

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);
   }
 }

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;

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;
+
+  // 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 _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;
 
-  GlobalKey _currentlyFocusedKey;
-  GlobalKey get currentlyFocusedKey {
-    if (_currentlyFocusedKey != null)
-      return _currentlyFocusedKey;
-    return initialFocus;
+  void _setFocusedWidget(GlobalKey key) {
+    setState(() {
+      _focusedWidget = key;
+      if (_focusedScope == null)
+        _focusedScope = _noFocusedScope;
+    });
+    _updateWidgetRemovalListener(key);
   }
-  void set currentlyFocusedKey(GlobalKey value) {
-    if (value != _currentlyFocusedKey) {
-      _currentlyFocusedKey = value;
-      notifyDescendants();
+
+  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;
+    });
+  }
+
+  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);
+  }
 
-  static bool at(Component component) {
+  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)';
   }
 
 }

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));
   }
 }

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<GlobalKey, Widget> _registry = new Map<GlobalKey, Widget>();
+  static final Map<GlobalKey, int> _debugDuplicates = new Map<GlobalKey, int>();
+  static final Map<GlobalKey, Set<GlobalKeyRemovalListener>> _removalListeners = new Map<GlobalKey, Set<GlobalKeyRemovalListener>>();
+  static final Set<GlobalKey> _removedKeys = new Set<GlobalKey>();
+
+  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<GlobalKeyRemovalListener>();
+    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<GlobalKey, Widget> _globalKeys = new Map<GlobalKey, Widget>();
-  static final Map<GlobalKey, int> _debugDuplicateGlobalKeys = new Map<GlobalKey, int>();
-
   /// 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);