From b5c461a9172fc09277c76daa8a5d480d95744f0a Mon Sep 17 00:00:00 2001 From: Michael Goderbauer Date: Wed, 19 Jul 2017 16:40:24 -0700 Subject: [PATCH] a11y: implement new SemanticsAction "showOnScreen" (v2) (#11156) * a11y: implement new SemanticsAction "showOnScreen" (v2) This action is triggered when the user swipes (in accessibility mode) to the last visible item of a scrollable list to bring that item fully on screen. This requires engine rolled to flutter/engine#3856. I am in the process of adding tests, but I'd like to get early feedback to see if this approach is OK. * fix null check * review comments * review comments * Add test * fix analyzer warning --- .../flutter/lib/src/rendering/object.dart | 20 +++++++++++-- .../flutter/lib/src/rendering/semantics.dart | 18 +++++++++-- .../flutter/lib/src/rendering/viewport.dart | 18 +++++++++++ .../lib/src/rendering/viewport_offset.dart | 9 ++++++ .../lib/src/widgets/scroll_position.dart | 1 + .../src/widgets/single_child_scroll_view.dart | 19 ++++++++++++ .../widgets/scrollable_semantics_test.dart | 30 +++++++++++++++++++ 7 files changed, 109 insertions(+), 6 deletions(-) diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 67d0d47a64..1b08db4845 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -739,7 +739,8 @@ class _RootSemanticsFragment extends _InterestingSemanticsFragment { assert(parentSemantics == null); renderObjectOwner._semantics ??= new SemanticsNode.root( handler: renderObjectOwner is SemanticsActionHandler ? renderObjectOwner as dynamic : null, - owner: renderObjectOwner.owner.semanticsOwner + owner: renderObjectOwner.owner.semanticsOwner, + showOnScreen: renderObjectOwner.showOnScreen, ); final SemanticsNode node = renderObjectOwner._semantics; assert(MatrixUtils.matrixEquals(node.transform, new Matrix4.identity())); @@ -768,7 +769,8 @@ class _ConcreteSemanticsFragment extends _InterestingSemanticsFragment { @override SemanticsNode establishSemanticsNode(_SemanticsGeometry geometry, SemanticsNode currentSemantics, SemanticsNode parentSemantics) { renderObjectOwner._semantics ??= new SemanticsNode( - handler: renderObjectOwner is SemanticsActionHandler ? renderObjectOwner as dynamic : null + handler: renderObjectOwner is SemanticsActionHandler ? renderObjectOwner as dynamic : null, + showOnScreen: renderObjectOwner.showOnScreen, ); final SemanticsNode node = renderObjectOwner._semantics; if (geometry != null) { @@ -812,7 +814,8 @@ class _ImplicitSemanticsFragment extends _InterestingSemanticsFragment { _haveConcreteNode = currentSemantics == null && annotator != null; if (haveConcreteNode) { renderObjectOwner._semantics ??= new SemanticsNode( - handler: renderObjectOwner is SemanticsActionHandler ? renderObjectOwner as dynamic : null + handler: renderObjectOwner is SemanticsActionHandler ? renderObjectOwner as dynamic : null, + showOnScreen: renderObjectOwner.showOnScreen, ); node = renderObjectOwner._semantics; } else { @@ -2777,6 +2780,17 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { @protected String debugDescribeChildren(String prefix) => ''; + + /// Attempt to make this or a descendant RenderObject visible on screen. + /// + /// If [child] is provided, that [RenderObject] is made visible. If [child] is + /// omitted, this [RenderObject] is made visible. + void showOnScreen([RenderObject child]) { + if (parent is RenderObject) { + final RenderObject renderParent = parent; + renderParent.showOnScreen(child ?? this); + } + } } /// Generic mixin for render objects with one child. diff --git a/packages/flutter/lib/src/rendering/semantics.dart b/packages/flutter/lib/src/rendering/semantics.dart index f2bdd7a9e6..e0a19c215e 100644 --- a/packages/flutter/lib/src/rendering/semantics.dart +++ b/packages/flutter/lib/src/rendering/semantics.dart @@ -143,8 +143,10 @@ class SemanticsNode extends AbstractNode { /// Each semantic node has a unique identifier that is assigned when the node /// is created. SemanticsNode({ - SemanticsActionHandler handler + SemanticsActionHandler handler, + VoidCallback showOnScreen, }) : id = _generateNewId(), + _showOnScreen = showOnScreen, _actionHandler = handler; /// Creates a semantic node to represent the root of the semantics tree. @@ -152,8 +154,10 @@ class SemanticsNode extends AbstractNode { /// The root node is assigned an identifier of zero. SemanticsNode.root({ SemanticsActionHandler handler, - SemanticsOwner owner + VoidCallback showOnScreen, + SemanticsOwner owner, }) : id = 0, + _showOnScreen = showOnScreen, _actionHandler = handler { attach(owner); } @@ -171,6 +175,7 @@ class SemanticsNode extends AbstractNode { final int id; final SemanticsActionHandler _actionHandler; + final VoidCallback _showOnScreen; // GEOMETRY // These are automatically handled by RenderObject's own logic @@ -734,7 +739,14 @@ class SemanticsOwner extends ChangeNotifier { void performAction(int id, SemanticsAction action) { assert(action != null); final SemanticsActionHandler handler = _getSemanticsActionHandlerForId(id, action); - handler?.performAction(action); + if (handler != null) { + handler.performAction(action); + return; + } + + // Default actions if no [handler] was provided. + if (action == SemanticsAction.showOnScreen && _nodes[id]._showOnScreen != null) + _nodes[id]._showOnScreen(); } SemanticsActionHandler _getSemanticsActionHandlerForPosition(SemanticsNode node, Offset position, SemanticsAction action) { diff --git a/packages/flutter/lib/src/rendering/viewport.dart b/packages/flutter/lib/src/rendering/viewport.dart index 89bc9565d4..c8243bae61 100644 --- a/packages/flutter/lib/src/rendering/viewport.dart +++ b/packages/flutter/lib/src/rendering/viewport.dart @@ -591,6 +591,24 @@ abstract class RenderViewportBase get childrenInHitTestOrder; + + @override + void showOnScreen([RenderObject child]) { + // Logic duplicated in [_RenderSingleChildViewport.showOnScreen]. + if (child != null) { + // Move viewport the smallest distance to bring [child] on screen. + final double leadingEdgeOffset = getOffsetToReveal(child, 0.0); + final double trailingEdgeOffset = getOffsetToReveal(child, 1.0); + final double currentOffset = offset.pixels; + if ((currentOffset - leadingEdgeOffset).abs() < (currentOffset - trailingEdgeOffset).abs()) { + offset.jumpTo(leadingEdgeOffset); + } else { + offset.jumpTo(trailingEdgeOffset); + } + } + // Make sure the viewport itself is on screen. + super.showOnScreen(); + } } /// A render object that is bigger on the inside. diff --git a/packages/flutter/lib/src/rendering/viewport_offset.dart b/packages/flutter/lib/src/rendering/viewport_offset.dart index 6b1721e256..d5fc99cf84 100644 --- a/packages/flutter/lib/src/rendering/viewport_offset.dart +++ b/packages/flutter/lib/src/rendering/viewport_offset.dart @@ -152,6 +152,10 @@ abstract class ViewportOffset extends ChangeNotifier { /// being called again, though this should be very rare. void correctBy(double correction); + /// Jumps the scroll position from its current value to the given value, + /// without animation, and without checking if the new value is in range. + void jumpTo(double pixels); + /// The direction in which the user is trying to change [pixels], relative to /// the viewport's [RenderViewport.axisDirection]. /// @@ -208,6 +212,11 @@ class _FixedViewportOffset extends ViewportOffset { _pixels += correction; } + @override + void jumpTo(double pixels) { + // Do nothing, viewport is fixed. + } + @override ScrollDirection get userScrollDirection => ScrollDirection.idle; } diff --git a/packages/flutter/lib/src/widgets/scroll_position.dart b/packages/flutter/lib/src/widgets/scroll_position.dart index 0519f91131..103fb0a0e3 100644 --- a/packages/flutter/lib/src/widgets/scroll_position.dart +++ b/packages/flutter/lib/src/widgets/scroll_position.dart @@ -510,6 +510,7 @@ abstract class ScrollPosition extends ViewportOffset with ScrollMetrics { /// If this method changes the scroll position, a sequence of start/update/end /// scroll notifications will be dispatched. No overscroll notifications can /// be generated by this method. + @override void jumpTo(double value); /// Deprecated. Use [jumpTo] or a custom [ScrollPosition] instead. 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 e7af006eb4..9b75768b50 100644 --- a/packages/flutter/lib/src/widgets/single_child_scroll_view.dart +++ b/packages/flutter/lib/src/widgets/single_child_scroll_view.dart @@ -418,4 +418,23 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix return leadingScrollOffset - (mainAxisExtent - targetMainAxisExtent) * alignment; } + + @override + void showOnScreen([RenderObject child]) { + // Logic duplicated in [RenderViewportBase.showOnScreen]. + if (child != null) { + // Move viewport the smallest distance to bring [child] on screen. + final double leadingEdgeOffset = getOffsetToReveal(child, 0.0); + final double trailingEdgeOffset = getOffsetToReveal(child, 1.0); + final double currentOffset = offset.pixels; + if ((currentOffset - leadingEdgeOffset).abs() < (currentOffset - trailingEdgeOffset).abs()) { + offset.jumpTo(leadingEdgeOffset); + } else { + offset.jumpTo(trailingEdgeOffset); + } + } + + // Make sure the viewport itself is on screen. + super.showOnScreen(); + } } diff --git a/packages/flutter/test/widgets/scrollable_semantics_test.dart b/packages/flutter/test/widgets/scrollable_semantics_test.dart index 1de20fe3a4..d59adc53f5 100644 --- a/packages/flutter/test/widgets/scrollable_semantics_test.dart +++ b/packages/flutter/test/widgets/scrollable_semantics_test.dart @@ -30,7 +30,37 @@ void main() { await flingDown(tester); expect(semantics, includesNodeWith(actions: [SemanticsAction.scrollUp, SemanticsAction.scrollDown])); + }); + testWidgets('showOnScreen works in scrollable', (WidgetTester tester) async { + new SemanticsTester(tester); // enables semantics tree generation + + const double kItemHeight = 40.0; + + final List containers = []; + for (int i = 0; i < 80; i++) + containers.add(new MergeSemantics(child: new Container( + height: kItemHeight, + child: new Text('container $i'), + ))); + + final ScrollController scrollController = new ScrollController( + initialScrollOffset: kItemHeight / 2, + ); + + await tester.pumpWidget(new ListView( + controller: scrollController, + children: containers + )); + + expect(scrollController.offset, kItemHeight / 2); + + final int firstContainerId = tester.renderObject(find.byWidget(containers.first)).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pump(); + await tester.pump(const Duration(seconds: 5)); + + expect(scrollController.offset, 0.0); }); }