From 8059aea365f7527f2a8ac3d639f8af375060457d Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Mon, 22 Jan 2018 18:12:39 -0800 Subject: [PATCH] TextFields should only have one SemanticsNode (#14219) * scrolling node eleminated * remove second node * fix ids --- .../lib/src/widgets/editable_text.dart | 1 + .../flutter/lib/src/widgets/scrollable.dart | 55 +++++--- .../test/material/text_field_test.dart | 120 ++++++++++++++++++ 3 files changed, 159 insertions(+), 17 deletions(-) diff --git a/packages/flutter/lib/src/widgets/editable_text.dart b/packages/flutter/lib/src/widgets/editable_text.dart index 19c78403a6..22c2824859 100644 --- a/packages/flutter/lib/src/widgets/editable_text.dart +++ b/packages/flutter/lib/src/widgets/editable_text.dart @@ -648,6 +648,7 @@ class EditableTextState extends State with AutomaticKeepAliveClien FocusScope.of(context).reparentIfNeeded(widget.focusNode); super.build(context); // See AutomaticKeepAliveClientMixin. return new Scrollable( + excludeFromSemantics: true, axisDirection: _isMultiline ? AxisDirection.down : AxisDirection.right, controller: _scrollController, physics: const ClampingScrollPhysics(), diff --git a/packages/flutter/lib/src/widgets/scrollable.dart b/packages/flutter/lib/src/widgets/scrollable.dart index a7fb4dff51..e7047d3a8d 100644 --- a/packages/flutter/lib/src/widgets/scrollable.dart +++ b/packages/flutter/lib/src/widgets/scrollable.dart @@ -79,8 +79,10 @@ class Scrollable extends StatefulWidget { this.controller, this.physics, @required this.viewportBuilder, + this.excludeFromSemantics: false, }) : assert(axisDirection != null), assert(viewportBuilder != null), + assert(excludeFromSemantics != null), super (key: key); /// The direction in which this widget scrolls. @@ -147,6 +149,19 @@ class Scrollable extends StatefulWidget { /// slivers and sizes itself based on the size of the slivers. final ViewportBuilder viewportBuilder; + /// Whether the scroll actions introduced by this [Scrollable] are exposed + /// in the semantics tree. + /// + /// Text fields with an overflow are usually scrollable to make sure that the + /// user can get to the beginning/end of the entered text. However, these + /// scrolling actions are generally not exposed to the semantics layer. + /// + /// See also: + /// + /// * [GestureDetector.excludeFromSemantics], which is used to accomplish the + /// exclusion. + final bool excludeFromSemantics; + /// The axis along which the scroll view scrolls. /// /// Determined by the [axisDirection]. @@ -490,27 +505,33 @@ class ScrollableState extends State with TickerProviderStateMixin Widget build(BuildContext context) { assert(position != null); // TODO(ianh): Having all these global keys is sad. - final Widget result = new _ExcludableScrollSemantics( - key: _excludableScrollSemanticsKey, - child: new RawGestureDetector( - key: _gestureDetectorKey, - gestures: _gestureRecognizers, - behavior: HitTestBehavior.opaque, - child: new Semantics( - explicitChildNodes: true, - child: new IgnorePointer( - key: _ignorePointerKey, - ignoring: _shouldIgnorePointer, - ignoringSemantics: false, - child: new _ScrollableScope( - scrollable: this, - position: position, - child: widget.viewportBuilder(context, position), - ), + Widget result = new RawGestureDetector( + key: _gestureDetectorKey, + gestures: _gestureRecognizers, + behavior: HitTestBehavior.opaque, + excludeFromSemantics: widget.excludeFromSemantics, + child: new Semantics( + explicitChildNodes: !widget.excludeFromSemantics, + child: new IgnorePointer( + key: _ignorePointerKey, + ignoring: _shouldIgnorePointer, + ignoringSemantics: false, + child: new _ScrollableScope( + scrollable: this, + position: position, + child: widget.viewportBuilder(context, position), ), ), ), ); + + if (!widget.excludeFromSemantics) { + result = new _ExcludableScrollSemantics( + key: _excludableScrollSemanticsKey, + child: result, + ); + } + return _configuration.buildViewportChrome(context, result, widget.axisDirection); } diff --git a/packages/flutter/test/material/text_field_test.dart b/packages/flutter/test/material/text_field_test.dart index c070b0e9e0..2cd1afcbee 100644 --- a/packages/flutter/test/material/text_field_test.dart +++ b/packages/flutter/test/material/text_field_test.dart @@ -164,6 +164,10 @@ void main() { return endpoints[0].point + const Offset(0.0, -2.0); } + setUp(() { + debugResetSemanticsIdCounter(); + }); + testWidgets('TextField has consistent size', (WidgetTester tester) async { final Key textFieldKey = new UniqueKey(); String textFieldValue; @@ -1747,4 +1751,120 @@ void main() { expect(tester.getBottomLeft(find.byKey(keyB)).dy, rowBottomY); }); + testWidgets('TextField semantics', (WidgetTester tester) async { + final SemanticsTester semantics = new SemanticsTester(tester); + final TextEditingController controller = new TextEditingController(); + final Key key = new UniqueKey(); + + await tester.pumpWidget( + overlay( + child: new TextField( + key: key, + controller: controller, + ) + ), + ); + + expect(semantics, hasSemantics(new TestSemantics.root( + children: [ + new TestSemantics.rootChild( + id: 2, + textDirection: TextDirection.ltr, + actions: [ + SemanticsAction.tap, + ], + flags: [ + SemanticsFlag.isTextField, + ], + ), + ], + ), ignoreTransform: true, ignoreRect: true)); + + controller.text = 'Guten Tag'; + await tester.pump(); + + expect(semantics, hasSemantics(new TestSemantics.root( + children: [ + new TestSemantics.rootChild( + id: 2, + textDirection: TextDirection.ltr, + value: 'Guten Tag', + actions: [ + SemanticsAction.tap, + ], + flags: [ + SemanticsFlag.isTextField, + ], + ), + ], + ), ignoreTransform: true, ignoreRect: true)); + + await tester.tap(find.byKey(key)); + await tester.pump(); + + expect(semantics, hasSemantics(new TestSemantics.root( + children: [ + new TestSemantics.rootChild( + id: 2, + textDirection: TextDirection.ltr, + value: 'Guten Tag', + actions: [ + SemanticsAction.tap, + SemanticsAction.moveCursorBackwardByCharacter, + ], + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.isFocused, + ], + ), + ], + ), ignoreTransform: true, ignoreRect: true)); + + controller.selection = const TextSelection.collapsed(offset: 4); + await tester.pump(); + + expect(semantics, hasSemantics(new TestSemantics.root( + children: [ + new TestSemantics.rootChild( + id: 2, + textDirection: TextDirection.ltr, + value: 'Guten Tag', + actions: [ + SemanticsAction.tap, + SemanticsAction.moveCursorBackwardByCharacter, + SemanticsAction.moveCursorForwardByCharacter, + ], + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.isFocused, + ], + ), + ], + ), ignoreTransform: true, ignoreRect: true)); + + controller.text = 'Schönen Feierabend'; + controller.selection = const TextSelection.collapsed(offset: 0); + await tester.pump(); + + expect(semantics, hasSemantics(new TestSemantics.root( + children: [ + new TestSemantics.rootChild( + id: 2, + textDirection: TextDirection.ltr, + value: 'Schönen Feierabend', + actions: [ + SemanticsAction.tap, + SemanticsAction.moveCursorForwardByCharacter, + ], + flags: [ + SemanticsFlag.isTextField, + SemanticsFlag.isFocused, + ], + ), + ], + ), ignoreTransform: true, ignoreRect: true)); + + semantics.dispose(); + }); + }