From db577577c7cbe832248342339b25cabada300f6a Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Tue, 21 Jul 2015 10:55:24 -0700 Subject: [PATCH 1/2] Focus support. This patch provides a class to manage focus, Focus, and a class to manage ownership of the keyboard, KeyboardHandle. Inherited (in widgets.dart) is adjusted to support subclasses that need to copy state from instance to instance. A GlobalKey key type is introduced that is basically the same as UniqueKey. Component classes that need a globally-unique key can specify that their 'key' constructor argument is a GlobalKey. Focus ----- You can use `Focus.at(this)` to determine if you, a Component, are currently focused. You can use `Focus.moveTo(this)` to take the focus or give it to a particular component. For this to work, there has to be a Focus class in the widget hierarchy. Currently, there can only be one Focus class, because nested scopes aren't supported. We should add support for that in a future patch. See issue #229. KeyboardHandle -------------- Instead of directly interacting with the keyboard service, you now ask for a KeyboardHandle using `_keyboard.show(client)`. This returns a KeyboardHandle class. On that class, you can call `handle.release()` when you want to hide the keyboard. If `handle.attached` is still true, and you need to reshow the keyboard after the user hid it, then you can can `handle.showByRequest()`. The point of this is that the `keyboard.show()` method maintains the invariant that only one KeyboardHandle is live at a time. There are some issues with the keyboard service that I filed as a result of doing this patch: #226 #227 --- sky/sdk/BUILD.gn | 1 + sky/sdk/example/address_book/lib/main.dart | 72 ++++++++++++++-------- sky/sdk/example/stocks/lib/stock_home.dart | 10 ++- sky/sdk/lib/editing/input.dart | 58 +++++++++-------- sky/sdk/lib/mojo/keyboard.dart | 67 ++++++++++++++++++-- sky/sdk/lib/widgets/focus.dart | 56 +++++++++++++++++ sky/sdk/lib/widgets/widget.dart | 43 ++++++++----- 7 files changed, 231 insertions(+), 76 deletions(-) create mode 100644 sky/sdk/lib/widgets/focus.dart diff --git a/sky/sdk/BUILD.gn b/sky/sdk/BUILD.gn index 020af51ead4..5bff99bca1a 100644 --- a/sky/sdk/BUILD.gn +++ b/sky/sdk/BUILD.gn @@ -67,6 +67,7 @@ dart_pkg("sky") { "lib/widgets/fixed_height_scrollable.dart", "lib/widgets/flat_button.dart", "lib/widgets/floating_action_button.dart", + "lib/widgets/focus.dart", "lib/widgets/icon.dart", "lib/widgets/icon_button.dart", "lib/widgets/ink_well.dart", diff --git a/sky/sdk/example/address_book/lib/main.dart b/sky/sdk/example/address_book/lib/main.dart index d16a29ef47f..df87257bc0d 100644 --- a/sky/sdk/example/address_book/lib/main.dart +++ b/sky/sdk/example/address_book/lib/main.dart @@ -2,35 +2,47 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:sky/editing/input.dart'; import 'package:sky/theme/colors.dart' as colors; import 'package:sky/theme/typography.dart' as typography; import 'package:sky/widgets/basic.dart'; import 'package:sky/widgets/default_text_style.dart'; -import 'package:sky/widgets/theme.dart'; -import 'package:sky/widgets/widget.dart'; -import 'package:sky/widgets/task_description.dart'; -import 'package:sky/widgets/scaffold.dart'; -import 'package:sky/widgets/tool_bar.dart'; -import 'package:sky/widgets/icon_button.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/material.dart'; -import 'package:sky/editing/input.dart'; +import 'package:sky/widgets/scaffold.dart'; import 'package:sky/widgets/scrollable_viewport.dart'; +import 'package:sky/widgets/task_description.dart'; +import 'package:sky/widgets/theme.dart'; +import 'package:sky/widgets/tool_bar.dart'; +import 'package:sky/widgets/widget.dart'; class Field extends Component { - Field({this.icon: null, this.placeholder: null}); + Field({ + Key key, + this.inputKey, + this.icon, + this.placeholder + }): super(key: key); - String icon; - String placeholder; + final GlobalKey inputKey; + final String icon; + final String placeholder; Widget build() { return new Flex([ new Padding( padding: const EdgeDims.symmetric(horizontal: 16.0), - child: new Icon(type:icon, size:24) + child: new Icon(type: icon, size: 24) ), - new Flexible(child:new Input(placeholder:placeholder, focused:false)) + new Flexible( + child: new Input( + key: inputKey, + placeholder: placeholder + ) + ) ], direction: FlexDirection.horizontal ); @@ -53,6 +65,13 @@ class AddressBookApp extends App { ); } + static final GlobalKey nameKey = new GlobalKey(); + static final GlobalKey phoneKey = new GlobalKey(); + static final GlobalKey emailKey = new GlobalKey(); + static final GlobalKey addressKey = new GlobalKey(); + static final GlobalKey ringtoneKey = new GlobalKey(); + static final GlobalKey noteKey = new GlobalKey(); + Widget buildBody() { return new Material( child: new ScrollableBlock([ @@ -62,12 +81,12 @@ class AddressBookApp extends App { decoration: new BoxDecoration(backgroundColor: colors.Purple[300]) ) ), - new Field(icon:"social/person", placeholder:"Name"), - new Field(icon: "communication/phone", placeholder:"Phone"), - new Field(icon: "communication/email", placeholder:"Email"), - new Field(icon: "maps/place", placeholder:"Address"), - new Field(icon: "av/volume_up", placeholder:"Ringtone"), - new Field(icon: "content/add", placeholder:"Add note"), + new Field(inputKey: nameKey, icon: "social/person", placeholder: "Name"), + new Field(inputKey: phoneKey, icon: "communication/phone", placeholder: "Phone"), + new Field(inputKey: emailKey, icon: "communication/email", placeholder: "Email"), + new Field(inputKey: addressKey, icon: "maps/place", placeholder: "Address"), + new Field(inputKey: ringtoneKey, icon: "av/volume_up", placeholder: "Ringtone"), + new Field(inputKey: noteKey, icon: "content/add", placeholder: "Add note"), ]) ); } @@ -81,24 +100,25 @@ class AddressBookApp extends App { } Widget build() { - ThemeData theme = new ThemeData( brightness: ThemeBrightness.light, primarySwatch: colors.Teal, accentColor: colors.PinkAccent[100] ); - return new Theme( data: theme, - child: new DefaultTextStyle( - style: typography.error, // if you see this, you've forgotten to correctly configure the text style! - child: new TaskDescription( - label: 'Address Book', + child: new DefaultTextStyle( + style: typography.error, // if you see this, you've forgotten to correctly configure the text style! + child: new TaskDescription( + label: 'Address Book', + child: new Focus( + defaultFocus: nameKey, child: buildMain() ) ) - ); - } + ) + ); + } } void main() { diff --git a/sky/sdk/example/stocks/lib/stock_home.dart b/sky/sdk/example/stocks/lib/stock_home.dart index 224ec81fff5..c0d83d46e01 100644 --- a/sky/sdk/example/stocks/lib/stock_home.dart +++ b/sky/sdk/example/stocks/lib/stock_home.dart @@ -10,6 +10,7 @@ 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'; @@ -244,6 +245,8 @@ class StockHome extends StatefulComponent { ); } + static GlobalKey searchFieldKey = new GlobalKey(); + // TODO(abarth): Should we factor this into a SearchBar in the framework? Widget buildSearchBar() { return new ToolBar( @@ -253,7 +256,7 @@ class StockHome extends StatefulComponent { onPressed: _handleSearchEnd ), center: new Input( - focused: true, + key: searchFieldKey, placeholder: 'Search stocks', onChanged: _handleSearchQueryChanged ), @@ -318,6 +321,9 @@ class StockHome extends StatefulComponent { ), ]; addMenuToOverlays(overlays); - return new Stack(overlays); + return new Focus( + defaultFocus: searchFieldKey, + child: new Stack(overlays) + ); } } diff --git a/sky/sdk/lib/editing/input.dart b/sky/sdk/lib/editing/input.dart index 2ca7e07dace..91eb5e316b7 100644 --- a/sky/sdk/lib/editing/input.dart +++ b/sky/sdk/lib/editing/input.dart @@ -7,6 +7,7 @@ import 'package:sky/editing/editable_text.dart'; import 'package:sky/mojo/keyboard.dart'; import 'package:sky/painting/text_style.dart'; import 'package:sky/widgets/basic.dart'; +import 'package:sky/widgets/focus.dart'; import 'package:sky/widgets/theme.dart'; typedef void ValueChanged(value); @@ -17,23 +18,18 @@ const EdgeDims _kTextfieldPadding = const EdgeDims.symmetric(vertical: 8.0); class Input extends StatefulComponent { - // Current thinking is that Widget will have an optional globalKey - // or heroKey and it will ask Focus.from(this).isFocused which will - // check using its globalKey. - // Only one element can use a globalKey at a time and its' up to - // Widget.sync to maintain the mapping. - // Never makes sense to have both a localKey and a globalKey. - // Possibly a class HeroKey who functions as a UUID. - - Input({Key key, - this.placeholder, - this.onChanged, - this.focused}) - : super(key: key); + Input({ + Key key, + this.placeholder, + this.onChanged + }): super(key: key); String placeholder; ValueChanged onChanged; - bool focused = false; + + String _value = ''; + EditableString _editableValue; + KeyboardHandle _keyboardHandle = KeyboardHandle.unattached; void initState() { _editableValue = new EditableString( @@ -46,17 +42,13 @@ class Input extends StatefulComponent { void syncFields(Input source) { placeholder = source.placeholder; onChanged = source.onChanged; - focused = source.focused; } - String _value = ''; - bool _isAttachedToKeyboard = false; - EditableString _editableValue; - void _handleTextUpdated() { - scheduleBuild(); if (_value != _editableValue.text) { - _value = _editableValue.text; + setState(() { + _value = _editableValue.text; + }); if (onChanged != null) onChanged(_value); } @@ -64,10 +56,12 @@ class Input extends StatefulComponent { Widget build() { ThemeData themeData = Theme.of(this); + bool focused = Focus.at(this); - if (focused && !_isAttachedToKeyboard) { - keyboard.show(_editableValue.stub); - _isAttachedToKeyboard = true; + if (focused && !_keyboardHandle.attached) { + _keyboardHandle = keyboard.show(_editableValue.stub); + } else if (!focused && _keyboardHandle.attached) { + _keyboardHandle.release(); } TextStyle textStyle = themeData.text.subhead; @@ -109,13 +103,23 @@ class Input extends StatefulComponent { return new Listener( child: input, - onPointerDown: (_) => keyboard.showByRequest() + onPointerDown: focus ); } + void focus(_) { + if (Focus.at(this)) { + assert(_keyboardHandle.attached); + _keyboardHandle.showByRequest(); + } else { + Focus.moveTo(this); + // we'll get told to rebuild and we'll take care of the keyboard then + } + } + void didUnmount() { - if (_isAttachedToKeyboard) - keyboard.hide(); + if (_keyboardHandle.attached) + _keyboardHandle.release(); super.didUnmount(); } } diff --git a/sky/sdk/lib/mojo/keyboard.dart b/sky/sdk/lib/mojo/keyboard.dart index c0b699baa77..3b640223673 100644 --- a/sky/sdk/lib/mojo/keyboard.dart +++ b/sky/sdk/lib/mojo/keyboard.dart @@ -6,15 +6,74 @@ import 'package:mojom/keyboard/keyboard.mojom.dart'; import 'package:sky/mojo/shell.dart' as shell; class _KeyboardConnection { - KeyboardServiceProxy proxy; _KeyboardConnection() { proxy = new KeyboardServiceProxy.unbound(); shell.requestService("mojo:keyboard", proxy); } - KeyboardService get keyboard => proxy.ptr; + KeyboardServiceProxy proxy; + KeyboardService get keyboardService => proxy.ptr; + + static final _KeyboardConnection instance = new _KeyboardConnection(); } -final _KeyboardConnection _connection = new _KeyboardConnection(); -final KeyboardService keyboard = _connection.keyboard; +class Keyboard { + + Keyboard(this.service); + + // The service is exposed in case you need direct access. + // However, as a general rule, you should be able to do + // most of what you need using only this class. + final KeyboardService service; + + KeyboardHandle _currentHandle; + + KeyboardHandle show(KeyboardClientStub stub) { + assert(stub != null); + if (_currentHandle != null) { + if (_currentHandle.stub == stub) + return _currentHandle; + _currentHandle.release(); + } + _currentHandle = new KeyboardHandle._show(this, stub); + return _currentHandle; + } + +} + +class KeyboardHandle { + + KeyboardHandle._show(Keyboard keyboard, this.stub) : _keyboard = keyboard { + _keyboard.service.show(stub); + _attached = true; + } + + KeyboardHandle._unattached(Keyboard keyboard) : _keyboard = keyboard, stub = null, _attached = false; + static final unattached = new KeyboardHandle._unattached(keyboard); + + final Keyboard _keyboard; + final KeyboardClientStub stub; + + bool _attached; + bool get attached => _attached; + + void showByRequest() { + assert(_attached); + assert(_keyboard._currentHandle == this); + _keyboard.service.showByRequest(); + } + + void release() { + if (_attached) { + assert(_keyboard._currentHandle == this); + _keyboard.service.hide(); + _attached = false; + _keyboard._currentHandle = null; + } + assert(_keyboard._currentHandle != this); + } + +} + +final Keyboard keyboard = new Keyboard(_KeyboardConnection.instance.keyboardService); diff --git a/sky/sdk/lib/widgets/focus.dart b/sky/sdk/lib/widgets/focus.dart new file mode 100644 index 00000000000..2ad94f7a597 --- /dev/null +++ b/sky/sdk/lib/widgets/focus.dart @@ -0,0 +1,56 @@ +// Copyright 2015 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:sky/widgets/widget.dart'; + +class Focus extends Inherited { + + // 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. + + Focus({ + GlobalKey key, + GlobalKey defaultFocus, + Widget child + }) : super(key: key, child: child); + + GlobalKey defaultFocus; + + GlobalKey _currentlyFocusedKey; + GlobalKey get currentlyFocusedKey { + if (_currentlyFocusedKey != null) + return _currentlyFocusedKey; + return defaultFocus; + } + void set currentlyFocusedKey(GlobalKey value) { + if (value != _currentlyFocusedKey) { + _currentlyFocusedKey = value; + notifyDescendants(); + } + } + + void syncState(Focus old) { + _currentlyFocusedKey = old._currentlyFocusedKey; + super.syncState(old); + } + + static bool at(Component component) { + assert(component != null); + assert(component.key is GlobalKey); + Focus focus = component.inheritedOfType(Focus); + return focus == null || focus.currentlyFocusedKey == component.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; + } + +} diff --git a/sky/sdk/lib/widgets/widget.dart b/sky/sdk/lib/widgets/widget.dart index 1108755b924..83b04d002a4 100644 --- a/sky/sdk/lib/widgets/widget.dart +++ b/sky/sdk/lib/widgets/widget.dart @@ -21,10 +21,7 @@ final bool _shouldLogRenderDuration = false; typedef Widget Builder(); typedef void WidgetTreeWalker(Widget); -abstract class KeyBase { -} - -abstract class Key extends KeyBase { +abstract class Key { Key.constructor(); // so that subclasses can call us, since the Key() factory constructor shadows the implicit constructor factory Key(String value) => new StringKey(value); factory Key.stringify(Object value) => new StringKey(value.toString()); @@ -43,7 +40,7 @@ class StringKey extends Key { class ObjectKey extends Key { ObjectKey(this.value) : super.constructor(); final Object value; - String toString() => '[Instance of ${value.runtimeType}]'; + String toString() => '[${value.runtimeType}(${value.hashCode})]'; bool operator==(other) => other is ObjectKey && identical(other.value, value); int get hashCode => identityHashCode(value); } @@ -53,6 +50,11 @@ class UniqueKey extends Key { String toString() => '[$hashCode]'; } +class GlobalKey extends Key { + GlobalKey() : super.constructor(); + String toString() => '[Global Key $hashCode]'; +} + /// A base class for elements of the widget tree abstract class Widget { @@ -326,22 +328,29 @@ abstract class Inherited extends TagNode { Inherited({ Key key, Widget child }) : super._withKey(child, key); void _sync(Widget old, dynamic slot) { - if (old != null && syncShouldNotify(old)) { - final Type ourRuntimeType = runtimeType; - void notifyChildren(Widget child) { - if (child is Component && - child._dependencies != null && - child._dependencies.contains(ourRuntimeType)) - child._dependenciesChanged(); - if (child.runtimeType != ourRuntimeType) - child.walkChildren(notifyChildren); - } - walkChildren(notifyChildren); + if (old != null) { + syncState(old); + if (syncShouldNotify(old)) + notifyDescendants(); } super._sync(old, slot); } - bool syncShouldNotify(Inherited old); + void notifyDescendants() { + final Type ourRuntimeType = runtimeType; + void notifyChildren(Widget child) { + if (child is Component && + child._dependencies != null && + child._dependencies.contains(ourRuntimeType)) + child._dependenciesChanged(); + if (child.runtimeType != ourRuntimeType) + child.walkChildren(notifyChildren); + } + walkChildren(notifyChildren); + } + + void syncState(Inherited old) { } + bool syncShouldNotify(Inherited old) => false; } From 86fd1e4840b862f1c4aa6e4c5c3dd4f409cb6959 Mon Sep 17 00:00:00 2001 From: Hixie Date: Thu, 23 Jul 2015 08:43:29 -0700 Subject: [PATCH 2/2] Rename defaultFocus to initialFocus. --- sky/sdk/example/address_book/lib/main.dart | 2 +- sky/sdk/example/stocks/lib/stock_home.dart | 2 +- sky/sdk/lib/widgets/focus.dart | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sky/sdk/example/address_book/lib/main.dart b/sky/sdk/example/address_book/lib/main.dart index df87257bc0d..7dee745cdee 100644 --- a/sky/sdk/example/address_book/lib/main.dart +++ b/sky/sdk/example/address_book/lib/main.dart @@ -112,7 +112,7 @@ class AddressBookApp extends App { child: new TaskDescription( label: 'Address Book', child: new Focus( - defaultFocus: nameKey, + initialFocus: nameKey, child: buildMain() ) ) diff --git a/sky/sdk/example/stocks/lib/stock_home.dart b/sky/sdk/example/stocks/lib/stock_home.dart index c0d83d46e01..851912975b8 100644 --- a/sky/sdk/example/stocks/lib/stock_home.dart +++ b/sky/sdk/example/stocks/lib/stock_home.dart @@ -322,7 +322,7 @@ class StockHome extends StatefulComponent { ]; addMenuToOverlays(overlays); return new Focus( - defaultFocus: searchFieldKey, + initialFocus: searchFieldKey, child: new Stack(overlays) ); } diff --git a/sky/sdk/lib/widgets/focus.dart b/sky/sdk/lib/widgets/focus.dart index 2ad94f7a597..5abb6fd1b43 100644 --- a/sky/sdk/lib/widgets/focus.dart +++ b/sky/sdk/lib/widgets/focus.dart @@ -14,17 +14,17 @@ class Focus extends Inherited { Focus({ GlobalKey key, - GlobalKey defaultFocus, + GlobalKey initialFocus, Widget child }) : super(key: key, child: child); - GlobalKey defaultFocus; + GlobalKey initialFocus; GlobalKey _currentlyFocusedKey; GlobalKey get currentlyFocusedKey { if (_currentlyFocusedKey != null) return _currentlyFocusedKey; - return defaultFocus; + return initialFocus; } void set currentlyFocusedKey(GlobalKey value) { if (value != _currentlyFocusedKey) {