From ef81bbe4b6e8abe9b1e74995ad0fe2e1dd42c620 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 18 Oct 2019 15:24:45 -0700 Subject: [PATCH 1/2] [web] Support input action --- .../src/engine/text_editing/text_editing.dart | 82 +++++-- lib/web_ui/test/text_editing_test.dart | 217 +++++++++++++++--- 2 files changed, 243 insertions(+), 56 deletions(-) diff --git a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart index 29bdcc5276115..5e95b1fed77dd 100644 --- a/lib/web_ui/lib/src/engine/text_editing/text_editing.dart +++ b/lib/web_ui/lib/src/engine/text_editing/text_editing.dart @@ -171,23 +171,29 @@ class EditingState { /// This corresponds to Flutter's [TextInputConfiguration]. class InputConfiguration { InputConfiguration({ - this.inputType, - this.obscureText = false, + @required this.inputType, + @required this.inputAction, + @required this.obscureText, }); InputConfiguration.fromFlutter(Map flutterInputConfiguration) : inputType = EngineInputType.fromName( flutterInputConfiguration['inputType']['name']), + inputAction = flutterInputConfiguration['inputAction'], obscureText = flutterInputConfiguration['obscureText']; /// The type of information being edited in the input control. final EngineInputType inputType; + /// The default action for the input field. + final String inputAction; + /// Whether to hide the text being edited. final bool obscureText; } typedef _OnChangeCallback = void Function(EditingState editingState); +typedef _OnActionCallback = void Function(String inputAction); /// Wraps the DOM element used to provide text editing capabilities. /// @@ -219,11 +225,16 @@ class TextEditingElement { const Duration(milliseconds: 100); final HybridTextEditing owner; - bool _enabled = false; + + @visibleForTesting + bool isEnabled = false; html.HtmlElement domElement; + InputConfiguration _inputConfiguration; EditingState _lastEditingState; + _OnChangeCallback _onChange; + _OnActionCallback _onAction; final List> _subscriptions = >[]; @@ -261,12 +272,15 @@ class TextEditingElement { void enable( InputConfiguration inputConfig, { @required _OnChangeCallback onChange, + @required _OnActionCallback onAction, }) { - assert(!_enabled); + assert(!isEnabled); _initDomElement(inputConfig); - _enabled = true; + isEnabled = true; + _inputConfiguration = inputConfig; _onChange = onChange; + _onAction = onAction; // Chrome on Android will hide the onscreen keyboard when you tap outside // the text box. Instead, we want the framework to tell us to hide the @@ -279,7 +293,7 @@ class TextEditingElement { if (browserEngine == BrowserEngine.blink || browserEngine == BrowserEngine.unknown) { _subscriptions.add(domElement.onBlur.listen((_) { - if (_enabled) { + if (isEnabled) { _refocus(); } })); @@ -297,6 +311,8 @@ class TextEditingElement { // Subscribe to text and selection changes. _subscriptions.add(domElement.onInput.listen(_handleChange)); + _subscriptions.add(domElement.onKeyDown.listen(_maybeSendAction)); + /// Detects changes in text selection. /// /// Currently only used in Firefox. @@ -330,9 +346,9 @@ class TextEditingElement { /// /// Calling [disable] also removes any registered event listeners. void disable() { - assert(_enabled); + assert(isEnabled); - _enabled = false; + isEnabled = false; _lastEditingState = null; for (int i = 0; i < _subscriptions.length; i++) { @@ -390,7 +406,7 @@ class TextEditingElement { void setEditingState(EditingState editingState) { _lastEditingState = editingState; - if (!_enabled || !editingState.isValid) { + if (!isEnabled || !editingState.isValid) { return; } @@ -416,6 +432,13 @@ class TextEditingElement { _onChange(_lastEditingState); } } + + void _maybeSendAction(html.KeyboardEvent event) { + if (event.keyCode == 13) { + event.preventDefault(); + _onAction(_inputConfiguration.inputAction); + } + } } /// The implementation of a persistent mode for [TextEditingElement]. @@ -512,7 +535,7 @@ class HybridTextEditing { /// /// Use [stopUsingCustomEditableElement] to switch back to default element. void useCustomEditableElement(TextEditingElement customEditingElement) { - if (_isEditing && customEditingElement != _customEditingElement) { + if (isEditing && customEditingElement != _customEditingElement) { stopEditing(); } _customEditingElement = customEditingElement; @@ -529,20 +552,21 @@ class HybridTextEditing { /// Flag which shows if there is an ongoing editing. /// /// Also used to define if a keyboard is needed. - bool _isEditing = false; + @visibleForTesting + bool isEditing = false; /// Indicates whether the input element needs to be positioned. /// /// See [TextEditingElement._delayBeforePositioning]. bool get inputElementNeedsToBePositioned => - !inputPositioned && _isEditing && doesKeyboardShiftInput; + !inputPositioned && isEditing && doesKeyboardShiftInput; /// Flag indicating whether the input element's position is set. /// /// See [inputElementNeedsToBePositioned]. bool inputPositioned = false; - Map _configuration; + InputConfiguration _configuration; /// All "flutter/textinput" platform messages should be sent to this method. void handleTextInput(ByteData data) { @@ -551,11 +575,11 @@ class HybridTextEditing { case 'TextInput.setClient': final bool clientIdChanged = _clientId != null && _clientId != call.arguments[0]; - if (clientIdChanged && _isEditing) { + if (clientIdChanged && isEditing) { stopEditing(); } _clientId = call.arguments[0]; - _configuration = call.arguments[1]; + _configuration = InputConfiguration.fromFlutter(call.arguments[1]); break; case 'TextInput.setEditingState': @@ -564,7 +588,7 @@ class HybridTextEditing { break; case 'TextInput.show': - if (!_isEditing) { + if (!isEditing) { _startEditing(); } break; @@ -579,7 +603,7 @@ class HybridTextEditing { case 'TextInput.clearClient': case 'TextInput.hide': - if (_isEditing) { + if (isEditing) { stopEditing(); } break; @@ -587,17 +611,18 @@ class HybridTextEditing { } void _startEditing() { - assert(!_isEditing); - _isEditing = true; + assert(!isEditing); + isEditing = true; editingElement.enable( - InputConfiguration.fromFlutter(_configuration), + _configuration, onChange: _syncEditingStateToFlutter, + onAction: _sendInputActionToFlutter, ); } void stopEditing() { - assert(_isEditing); - _isEditing = false; + assert(isEditing); + isEditing = false; editingElement.disable(); } @@ -666,6 +691,19 @@ class HybridTextEditing { ); } + void _sendInputActionToFlutter(String inputAction) { + ui.window.onPlatformMessage( + 'flutter/textinput', + const JSONMethodCodec().encodeMethodCall( + MethodCall( + 'TextInputClient.performAction', + [_clientId, inputAction], + ), + ), + _emptyCallback, + ); + } + /// Positioning of input element is only done if we are not expecting input /// to be shifted by a virtual keyboard or if the input is already positioned. /// diff --git a/lib/web_ui/test/text_editing_test.dart b/lib/web_ui/test/text_editing_test.dart index 09f1fc1d2fb87..f9fc6330c8bc1 100644 --- a/lib/web_ui/test/text_editing_test.dart +++ b/lib/web_ui/test/text_editing_test.dart @@ -3,10 +3,11 @@ // found in the LICENSE file. import 'dart:html'; +import 'dart:js_util' as js_util; import 'dart:typed_data'; import 'package:ui/ui.dart' as ui; -import 'package:ui/src/engine.dart'; +import 'package:ui/src/engine.dart' hide window; import 'package:test/test.dart'; @@ -16,14 +17,21 @@ const MethodCodec codec = JSONMethodCodec(); TextEditingElement editingElement; EditingState lastEditingState; +String lastInputAction; -final InputConfiguration singlelineConfig = - InputConfiguration(inputType: EngineInputType.text); +final InputConfiguration singlelineConfig = InputConfiguration( + inputType: EngineInputType.text, + obscureText: false, + inputAction: 'TextInputAction.done', +); final Map flutterSinglelineConfig = createFlutterConfig('text'); -final InputConfiguration multilineConfig = - InputConfiguration(inputType: EngineInputType.multiline); +final InputConfiguration multilineConfig = InputConfiguration( + inputType: EngineInputType.multiline, + obscureText: false, + inputAction: 'TextInputAction.newline', +); final Map flutterMultilineConfig = createFlutterConfig('multiline'); @@ -31,21 +39,24 @@ void trackEditingState(EditingState editingState) { lastEditingState = editingState; } +void trackInputAction(String inputAction) { + lastInputAction = inputAction; +} + void main() { + tearDown(() { + lastEditingState = null; + lastInputAction = null; + }); + group('$TextEditingElement', () { setUp(() { editingElement = TextEditingElement(HybridTextEditing()); }); tearDown(() { - try { + if (editingElement.isEnabled) { editingElement.disable(); - } catch (e) { - if (e is AssertionError) { - // This is fine. It just means the test itself disabled the editing element. - } else { - rethrow; - } } }); @@ -57,7 +68,11 @@ void main() { // The focus initially is on the body. expect(document.activeElement, document.body); - editingElement.enable(singlelineConfig, onChange: trackEditingState); + editingElement.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); expect( document.getElementsByTagName('input'), hasLength(1), @@ -81,7 +96,11 @@ void main() { }); test('Can read editing state correctly', () { - editingElement.enable(singlelineConfig, onChange: trackEditingState); + editingElement.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); final InputElement input = editingElement.domElement; input.value = 'foo bar'; @@ -97,29 +116,50 @@ void main() { lastEditingState, EditingState(text: 'foo bar', baseOffset: 4, extentOffset: 6), ); + + // There should be no input action. + expect(lastInputAction, isNull); }); test('Can set editing state correctly', () { - editingElement.enable(singlelineConfig, onChange: trackEditingState); + editingElement.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); editingElement.setEditingState( EditingState(text: 'foo bar baz', baseOffset: 2, extentOffset: 7)); checkInputEditingState(editingElement.domElement, 'foo bar baz', 2, 7); + + // There should be no input action. + expect(lastInputAction, isNull); }); test('Re-acquires focus', () async { - editingElement.enable(singlelineConfig, onChange: trackEditingState); + editingElement.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); expect(document.activeElement, editingElement.domElement); editingElement.domElement.blur(); // The focus remains on [editingElement.domElement]. expect(document.activeElement, editingElement.domElement); + + // There should be no input action. + expect(lastInputAction, isNull); }); test('Multi-line mode also works', () { // The textarea element is created lazily. expect(document.getElementsByTagName('textarea'), hasLength(0)); - editingElement.enable(multilineConfig, onChange: trackEditingState); + editingElement.enable( + multilineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); expect(document.getElementsByTagName('textarea'), hasLength(1)); final TextAreaElement textarea = @@ -152,6 +192,9 @@ void main() { expect(document.getElementsByTagName('textarea'), hasLength(0)); // The focus is back to the body. expect(document.activeElement, document.body); + + // There should be no input action. + expect(lastInputAction, isNull); }); test('Same instance can be re-enabled with different config', () { @@ -160,7 +203,11 @@ void main() { expect(document.getElementsByTagName('textarea'), hasLength(0)); // Use single-line config and expect an `` to be created. - editingElement.enable(singlelineConfig, onChange: trackEditingState); + editingElement.enable( + singlelineConfig, + onChange: trackEditingState, + onAction: trackInputAction, + ); expect(document.getElementsByTagName('input'), hasLength(1)); expect(document.getElementsByTagName('textarea'), hasLength(0)); @@ -170,7 +217,11 @@ void main() { expect(document.getElementsByTagName('textarea'), hasLength(0)); // Use multi-line config and expect an `