diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart index 8f6b3e25c6..413d766e26 100644 --- a/packages/flutter/lib/src/material/app_bar.dart +++ b/packages/flutter/lib/src/material/app_bar.dart @@ -920,7 +920,7 @@ class SliverAppBar extends StatefulWidget { /// Whether the app bar should remain visible at the start of the scroll view. /// - /// The app bar can still expand an contract as the user scrolls, but it will + /// The app bar can still expand and contract as the user scrolls, but it will /// remain visible rather than being scrolled out of view. final bool pinned; diff --git a/packages/flutter/lib/src/rendering/viewport.dart b/packages/flutter/lib/src/rendering/viewport.dart index 475e5949d6..052d816bd4 100644 --- a/packages/flutter/lib/src/rendering/viewport.dart +++ b/packages/flutter/lib/src/rendering/viewport.dart @@ -784,24 +784,65 @@ abstract class RenderViewportBase= trailingEdgeOffset); + + if (currentOffset > leadingEdgeOffset) { + // `child` currently starts above the leading edge and can be shown fully + // on screen by scrolling down (which means: moving viewport up). + offset.jumpTo(leadingEdgeOffset); + } else if (currentOffset < trailingEdgeOffset ) { + // `child currently ends below the trailing edge and can be shown fully + // on screen by scrolling up (which means: moving viewport down) + offset.jumpTo(trailingEdgeOffset); + } + // else: `child` is between leading and trailing edge and hence already + // fully shown on screen. No action necessary. + } } /// A render object that is bigger on the inside. 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 f23e9bb3ba..2070cd28f1 100644 --- a/packages/flutter/lib/src/widgets/single_child_scroll_view.dart +++ b/packages/flutter/lib/src/widgets/single_child_scroll_view.dart @@ -586,19 +586,7 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix @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); - } - } - + RenderViewportBase.showInViewport(child: child, viewport: this, offset: offset); // 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 f3a458f518..0f6ecddb8c 100644 --- a/packages/flutter/test/widgets/scrollable_semantics_test.dart +++ b/packages/flutter/test/widgets/scrollable_semantics_test.dart @@ -137,12 +137,6 @@ void main() { await tester.pump(const Duration(seconds: 5)); expect(tester.getTopLeft(find.byWidget(containers.first)).dy, kExpandedAppBarHeight); - final int secondContainerId = tester.renderObject(find.byWidget(containers[1])).debugSemantics.id; - tester.binding.pipelineOwner.semanticsOwner.performAction(secondContainerId, SemanticsAction.showOnScreen); - await tester.pump(); - await tester.pump(const Duration(seconds: 5)); - expect(tester.getTopLeft(find.byWidget(containers[1])).dy, kExpandedAppBarHeight); - semantics.dispose(); }); @@ -168,7 +162,7 @@ void main() { }); final ScrollController scrollController = new ScrollController( - initialScrollOffset: kItemHeight / 2, + initialScrollOffset: 2.5 * kItemHeight, ); await tester.pumpWidget(new Directionality( @@ -195,7 +189,7 @@ void main() { ), )); - expect(scrollController.offset, kItemHeight / 2); + expect(scrollController.offset, 2.5 * kItemHeight); final int id0 = tester.renderObject(find.byWidget(children[0])).debugSemantics.id; tester.binding.pipelineOwner.semanticsOwner.performAction(id0, SemanticsAction.showOnScreen); @@ -203,12 +197,6 @@ void main() { await tester.pump(const Duration(seconds: 5)); expect(tester.getTopLeft(find.byWidget(children[0])).dy, kToolbarHeight); - final int id1 = tester.renderObject(find.byWidget(children[1])).debugSemantics.id; - tester.binding.pipelineOwner.semanticsOwner.performAction(id1, SemanticsAction.showOnScreen); - await tester.pump(); - await tester.pump(const Duration(seconds: 5)); - expect(tester.getTopLeft(find.byWidget(children[1])).dy, kToolbarHeight); - semantics.dispose(); }); @@ -399,6 +387,200 @@ void main() { semantics.dispose(); }); + + group('showOnScreen', () { + + const double kItemHeight = 100.0; + + List children; + ScrollController scrollController; + Widget widgetUnderTest; + + setUp(() { + children = new List.generate(10, (int i) { + return new MergeSemantics( + child: new Container( + height: kItemHeight, + child: new Text('container $i'), + ), + ); + }); + + scrollController = new ScrollController( + initialScrollOffset: kItemHeight / 2, + ); + + widgetUnderTest = new Directionality( + textDirection: TextDirection.ltr, + child: new Center( + child: new Container( + height: 2 * kItemHeight, + child: new ListView( + controller: scrollController, + children: children, + ), + ), + ), + ); + + }); + + testWidgets('brings item above leading edge to leading edge', (WidgetTester tester) async { + semantics = new SemanticsTester(tester); // enables semantics tree generation + + await tester.pumpWidget(widgetUnderTest); + + expect(scrollController.offset, kItemHeight / 2); + + final int firstContainerId = tester.renderObject(find.byWidget(children.first)).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pumpAndSettle(); + + expect(scrollController.offset, 0.0); + + semantics.dispose(); + }); + + testWidgets('brings item below trailing edge to trailing edge', (WidgetTester tester) async { + semantics = new SemanticsTester(tester); // enables semantics tree generation + + await tester.pumpWidget(widgetUnderTest); + + expect(scrollController.offset, kItemHeight / 2); + + final int firstContainerId = tester.renderObject(find.byWidget(children[2])).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pumpAndSettle(); + + expect(scrollController.offset, kItemHeight); + + semantics.dispose(); + }); + + testWidgets('does not change position of items already fully on-screen', (WidgetTester tester) async { + semantics = new SemanticsTester(tester); // enables semantics tree generation + + await tester.pumpWidget(widgetUnderTest); + + expect(scrollController.offset, kItemHeight / 2); + + final int firstContainerId = tester.renderObject(find.byWidget(children[1])).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pumpAndSettle(); + + expect(scrollController.offset, kItemHeight / 2); + + semantics.dispose(); + }); + }); + + group('showOnScreen with negative children', () { + const double kItemHeight = 100.0; + + List children; + ScrollController scrollController; + Widget widgetUnderTest; + + setUp(() { + final Key center = new GlobalKey(); + + children = new List.generate(10, (int i) { + return new SliverToBoxAdapter( + key: i == 5 ? center : null, + child: new MergeSemantics( + key: new ValueKey(i), + child: new Container( + height: kItemHeight, + child: new Text('container $i'), + ), + ), + ); + }); + + scrollController = new ScrollController( + initialScrollOffset: -2.5 * kItemHeight, + ); + + // 'container 0' is at offset -500 + // 'container 1' is at offset -400 + // 'container 2' is at offset -300 + // 'container 3' is at offset -200 + // 'container 4' is at offset -100 + // 'container 5' is at offset 0 + + widgetUnderTest = new Directionality( + textDirection: TextDirection.ltr, + child: new Center( + child: new Container( + height: 2 * kItemHeight, + child: new Scrollable( + controller: scrollController, + viewportBuilder: (BuildContext context, ViewportOffset offset) { + return new Viewport( + cacheExtent: 0.0, + offset: offset, + center: center, + slivers: children + ); + }, + ), + ), + ), + ); + + }); + + testWidgets('brings item above leading edge to leading edge', (WidgetTester tester) async { + semantics = new SemanticsTester(tester); // enables semantics tree generation + + await tester.pumpWidget(widgetUnderTest); + + expect(scrollController.offset, -250.0); + + final int firstContainerId = tester.renderObject(find.byKey(const ValueKey(2))).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pumpAndSettle(); + + expect(scrollController.offset, -300.0); + + semantics.dispose(); + }); + + testWidgets('brings item below trailing edge to trailing edge', (WidgetTester tester) async { + semantics = new SemanticsTester(tester); // enables semantics tree generation + + await tester.pumpWidget(widgetUnderTest); + + expect(scrollController.offset, -250.0); + + final int firstContainerId = tester.renderObject(find.byKey(const ValueKey(4))).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pumpAndSettle(); + + expect(scrollController.offset, -200.0); + + semantics.dispose(); + }); + + testWidgets('does not change position of items already fully on-screen', (WidgetTester tester) async { + semantics = new SemanticsTester(tester); // enables semantics tree generation + + await tester.pumpWidget(widgetUnderTest); + + expect(scrollController.offset, -250.0); + + final int firstContainerId = tester.renderObject(find.byKey(const ValueKey(3))).debugSemantics.id; + tester.binding.pipelineOwner.semanticsOwner.performAction(firstContainerId, SemanticsAction.showOnScreen); + await tester.pumpAndSettle(); + + expect(scrollController.offset, -250.0); + + semantics.dispose(); + }); + + }); + + } Future flingUp(WidgetTester tester, { int repetitions: 1 }) => fling(tester, const Offset(0.0, -200.0), repetitions);