Update Semantics for SingleChildScrollViews (#12376) * Update Semantics for SingleChildScrollViews * refactor * review feedback * added assert and comments * doc
diff --git a/packages/flutter/lib/src/widgets/framework.dart b/packages/flutter/lib/src/widgets/framework.dart index 35d3039..bc01d99 100644 --- a/packages/flutter/lib/src/widgets/framework.dart +++ b/packages/flutter/lib/src/widgets/framework.dart
@@ -2145,6 +2145,11 @@ int _debugStateLockLevel = 0; bool get _debugStateLocked => _debugStateLockLevel > 0; + + /// Whether this widget tree is in the build phase. + /// + /// Only valid when asserts are enabled. + bool get debugBuilding => _debugBuilding; bool _debugBuilding = false; Element _debugCurrentBuildTarget;
diff --git a/packages/flutter/lib/src/widgets/gesture_detector.dart b/packages/flutter/lib/src/widgets/gesture_detector.dart index 7273be6..93c6372 100644 --- a/packages/flutter/lib/src/widgets/gesture_detector.dart +++ b/packages/flutter/lib/src/widgets/gesture_detector.dart
@@ -535,29 +535,31 @@ } } - /// This method can be called after the build phase, during the layout of the - /// nearest descendant [RenderObjectWidget] of the gesture detector, to filter - /// the list of available semantic actions. + /// This method can be called outside of the build phase to filter the list of + /// available semantic actions. + /// + /// The actual filtering is happening in the next frame and a frame will be + /// scheduled if non is pending. /// /// This is used by [Scrollable] to configure system accessibility tools so /// that they know in which direction a particular list can be scrolled. /// /// If this is never called, then the actions are not filtered. If the list of - /// actions to filter changes, it must be called again (during the layout of - /// the nearest descendant [RenderObjectWidget] of the gesture detector). + /// actions to filter changes, it must be called again. void replaceSemanticsActions(Set<SemanticsAction> actions) { assert(() { - if (!context.findRenderObject().owner.debugDoingLayout) { + final Element element = context; + if (element.owner.debugBuilding) { throw new FlutterError( 'Unexpected call to replaceSemanticsActions() method of RawGestureDetectorState.\n' - 'The replaceSemanticsActions() method can only be called during the layout phase.' + 'The replaceSemanticsActions() method can only be called outside of the build phase.' ); } return true; }()); if (!widget.excludeFromSemantics) { final RenderSemanticsGestureHandler semanticsGestureHandler = context.findRenderObject(); - semanticsGestureHandler.validActions = actions; + semanticsGestureHandler.validActions = actions; // will call _markNeedsSemanticsUpdate(), if required. } }
diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart index bf0b4d2..820e34e 100644 --- a/packages/flutter/lib/src/widgets/scroll_position.dart +++ b/packages/flutter/lib/src/widgets/scroll_position.dart
@@ -371,6 +371,18 @@ Set<SemanticsAction> _semanticActions; + /// Called whenever the scroll position or the dimensions of the scroll view + /// change to schedule an update of the available semantics actions. The + /// actual update will be performed in the nxt frame. If non is pending + /// a frame will be scheduled. + /// + /// For example: If the scroll view has been scrolled all the way to the top, + /// the action to scroll further up needs to be removed as the scroll view + /// cannot be scrolled in that direction anymore. + /// + /// This method is potentially called twice per frame (if scroll position and + /// scroll view dimensions both change) and therefore shouldn't do anything + /// expensive. void _updateSemanticActions() { SemanticsAction forward; SemanticsAction backward; @@ -409,7 +421,6 @@ applyNewDimensions(); _didChangeViewportDimension = false; } - _updateSemanticActions(); return true; } @@ -437,6 +448,7 @@ void applyNewDimensions() { assert(pixels != null); activity.applyNewDimensions(); + _updateSemanticActions(); // will potentially request a semantics update. } /// Animates the position such that the given object is as visible as possible @@ -614,6 +626,12 @@ } @override + void notifyListeners() { + _updateSemanticActions(); // will potentially request a semantics update. + super.notifyListeners(); + } + + @override void debugFillDescription(List<String> description) { if (debugLabel != null) description.add(debugLabel);
diff --git a/packages/flutter/lib/src/widgets/single_child_scroll_view.dart b/packages/flutter/lib/src/widgets/single_child_scroll_view.dart index a9c513e..c852fe6 100644 --- a/packages/flutter/lib/src/widgets/single_child_scroll_view.dart +++ b/packages/flutter/lib/src/widgets/single_child_scroll_view.dart
@@ -205,13 +205,18 @@ if (value == _offset) return; if (attached) - _offset.removeListener(markNeedsPaint); + _offset.removeListener(_hasScrolled); _offset = value; if (attached) - _offset.addListener(markNeedsPaint); + _offset.addListener(_hasScrolled); markNeedsLayout(); } + void _hasScrolled() { + markNeedsPaint(); + markNeedsSemanticsUpdate(); + } + @override void setupParentData(RenderObject child) { // We don't actually use the offset argument in BoxParentData, so let's @@ -223,12 +228,12 @@ @override void attach(PipelineOwner owner) { super.attach(owner); - _offset.addListener(markNeedsPaint); + _offset.addListener(_hasScrolled); } @override void detach() { - _offset.removeListener(markNeedsPaint); + _offset.removeListener(_hasScrolled); super.detach(); }
diff --git a/packages/flutter/test/material/tabs_test.dart b/packages/flutter/test/material/tabs_test.dart index cf679a6..03de12c 100644 --- a/packages/flutter/test/material/tabs_test.dart +++ b/packages/flutter/test/material/tabs_test.dart
@@ -1038,6 +1038,56 @@ semantics.dispose(); }); + testWidgets('correct scrolling semantics', (WidgetTester tester) async { + final SemanticsTester semantics = new SemanticsTester(tester); + + final List<Tab> tabs = new List<Tab>.generate(20, (int index) { + return new Tab(text: 'This is a very wide tab #$index'); + }); + + final TabController controller = new TabController( + vsync: const TestVSync(), + length: tabs.length, + initialIndex: 0, + ); + + await tester.pumpWidget( + boilerplate( + child: new Semantics( + container: true, + child: new TabBar( + isScrollable: true, + controller: controller, + tabs: tabs, + ), + ), + ), + ); + + expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollLeft])); + expect(semantics, isNot(includesNodeWith(label: 'This is a very wide tab #10'))); + + controller.index = 10; + await tester.pumpAndSettle(); + + expect(semantics, isNot(includesNodeWith(label: 'This is a very wide tab #0'))); + expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollLeft, SemanticsAction.scrollRight])); + expect(semantics, includesNodeWith(label: 'This is a very wide tab #10')); + + controller.index = 19; + await tester.pumpAndSettle(); + + expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollRight])); + + controller.index = 0; + await tester.pumpAndSettle(); + + expect(semantics, includesNodeWith(actions: <SemanticsAction>[SemanticsAction.scrollLeft])); + expect(semantics, includesNodeWith(label: 'This is a very wide tab #0')); + + semantics.dispose(); + }); + testWidgets('TabBar etc with zero tabs', (WidgetTester tester) async { final TabController controller = new TabController( vsync: const TestVSync(),