Prevent viewport.showOnScreen from scrolling the viewport if the specified...

Prevent viewport.showOnScreen from scrolling the viewport if the specified Rect is already visible.  (#56413)

packages/flutter/lib/src/material/app_bar.dart

@@ -817,12 +817,18 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate {
     @required this.topPadding,
     @required this.floating,
     @required this.pinned,
+    @required this.vsync,
     @required this.snapConfiguration,
     @required this.stretchConfiguration,
+    @required this.showOnScreenConfiguration,
     @required this.shape,
     @required this.toolbarHeight,
     @required this.leadingWidth,
   }) : assert(primary || topPadding == 0.0),
+       assert(
+         !floating || (snapConfiguration == null && showOnScreenConfiguration == null) || vsync != null,
+         'vsync cannot be null when snapConfiguration or showOnScreenConfiguration is not null, and floating is true',
+       ),
        _bottomHeight = bottom?.preferredSize?.height ?? 0.0;
 
   final Widget leading;
@@ -860,12 +866,18 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate {
   @override
   double get maxExtent => math.max(topPadding + (expandedHeight ?? (toolbarHeight ?? kToolbarHeight) + _bottomHeight), minExtent);
 
+  @override
+  final TickerProvider vsync;
+
   @override
   final FloatingHeaderSnapConfiguration snapConfiguration;
 
   @override
   final OverScrollHeaderStretchConfiguration stretchConfiguration;
 
+  @override
+  final PersistentHeaderShowOnScreenConfiguration showOnScreenConfiguration;
+
   @override
   Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) {
     final double visibleMainHeight = maxExtent - shrinkOffset - topPadding;
@@ -935,8 +947,10 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate {
         || topPadding != oldDelegate.topPadding
         || pinned != oldDelegate.pinned
         || floating != oldDelegate.floating
+        || vsync != oldDelegate.vsync
         || snapConfiguration != oldDelegate.snapConfiguration
         || stretchConfiguration != oldDelegate.stretchConfiguration
+        || showOnScreenConfiguration != oldDelegate.showOnScreenConfiguration
         || forceElevated != oldDelegate.forceElevated
         || toolbarHeight != oldDelegate.toolbarHeight
         || leadingWidth != leadingWidth;
@@ -1325,9 +1339,14 @@ class SliverAppBar extends StatefulWidget {
   /// into view.
   ///
   /// If [snap] is true then a scroll that exposes the floating app bar will
-  /// trigger an animation that slides the entire app bar into view. Similarly if
-  /// a scroll dismisses the app bar, the animation will slide the app bar
-  /// completely out of view.
+  /// trigger an animation that slides the entire app bar into view. Similarly
+  /// if a scroll dismisses the app bar, the animation will slide the app bar
+  /// completely out of view. Additionally, setting [snap] to true will fully
+  /// expand the floating app bar when the framework tries to reveal the
+  /// contents of the app bar by calling [RenderObject.showOnScreen]. For
+  /// example, when a [TextField] in the floating app bar gains focus, if [snap]
+  /// is true, the framework will always fully expand the floating app bar, in
+  /// order to reveal the focused [TextField].
   ///
   /// Snapping only applies when the app bar is floating, not when the app bar
   /// appears at the top of its scroll view.
@@ -1382,17 +1401,21 @@ class SliverAppBar extends StatefulWidget {
 class _SliverAppBarState extends State<SliverAppBar> with TickerProviderStateMixin {
   FloatingHeaderSnapConfiguration _snapConfiguration;
   OverScrollHeaderStretchConfiguration _stretchConfiguration;
+  PersistentHeaderShowOnScreenConfiguration _showOnScreenConfiguration;
 
   void _updateSnapConfiguration() {
     if (widget.snap && widget.floating) {
       _snapConfiguration = FloatingHeaderSnapConfiguration(
-        vsync: this,
         curve: Curves.easeOut,
         duration: const Duration(milliseconds: 200),
       );
     } else {
       _snapConfiguration = null;
     }
+
+    _showOnScreenConfiguration = widget.floating & widget.snap
+      ? const PersistentHeaderShowOnScreenConfiguration(minShowOnScreenExtent: double.infinity)
+      : null;
   }
 
   void _updateStretchConfiguration() {
@@ -1438,6 +1461,7 @@ class _SliverAppBarState extends State<SliverAppBar> with TickerProviderStateMix
         floating: widget.floating,
         pinned: widget.pinned,
         delegate: _SliverAppBarDelegate(
+          vsync: this,
           leading: widget.leading,
           automaticallyImplyLeading: widget.automaticallyImplyLeading,
           title: widget.title,
@@ -1464,6 +1488,7 @@ class _SliverAppBarState extends State<SliverAppBar> with TickerProviderStateMix
           shape: widget.shape,
           snapConfiguration: _snapConfiguration,
           stretchConfiguration: _stretchConfiguration,
+          showOnScreenConfiguration: _showOnScreenConfiguration,
           toolbarHeight: widget.toolbarHeight,
           leadingWidth: widget.leadingWidth,
         ),

packages/flutter/lib/src/rendering/viewport.dart

@@ -727,58 +727,26 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
       final Matrix4 transform = target.getTransformTo(pivot);
       final Rect bounds = MatrixUtils.transformRect(transform, rect);
 
+      // Convert `rect`'s leading edge from `pivot`'s RenderBox coordinate
+      // system to the scrollOffset within `pivot.parent`. For `up` and `left`
+      // AxisDirections here, the leading edge of the render box is the
+      // bottom/right edge.
       final GrowthDirection growthDirection = pivotParent.constraints.growthDirection;
       switch (applyGrowthDirectionToAxisDirection(axisDirection, growthDirection)) {
         case AxisDirection.up:
-          double offset;
-          switch (growthDirection) {
-            case GrowthDirection.forward:
-              offset = bounds.bottom;
-              break;
-            case GrowthDirection.reverse:
-              offset = bounds.top;
-              break;
-          }
-          leadingScrollOffset += pivot.size.height - offset;
+          leadingScrollOffset += pivot.size.height - bounds.bottom;
           targetMainAxisExtent = bounds.height;
           break;
         case AxisDirection.right:
-          double offset;
-          switch (growthDirection) {
-            case GrowthDirection.forward:
-              offset = bounds.left;
-              break;
-            case GrowthDirection.reverse:
-              offset = bounds.right;
-              break;
-          }
-          leadingScrollOffset += offset;
+          leadingScrollOffset += bounds.left;
           targetMainAxisExtent = bounds.width;
           break;
         case AxisDirection.down:
-          double offset;
-          switch (growthDirection) {
-            case GrowthDirection.forward:
-              offset = bounds.top;
-              break;
-            case GrowthDirection.reverse:
-              offset = bounds.bottom;
-              break;
-          }
-          leadingScrollOffset += offset;
+          leadingScrollOffset += bounds.top;
           targetMainAxisExtent = bounds.height;
           break;
         case AxisDirection.left:
-          double offset;
-          switch (growthDirection) {
-            case GrowthDirection.forward:
-              offset = bounds.right;
-              break;
-            case GrowthDirection.reverse:
-              offset = bounds.left;
-              break;
-          }
-          leadingScrollOffset += pivot.size.width - offset;
+          leadingScrollOffset += pivot.size.width - bounds.right;
           targetMainAxisExtent = bounds.width;
           break;
       }
@@ -792,14 +760,44 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
     assert(child.parent == this);
     assert(child is RenderSliver);
     final RenderSliver sliver = child as RenderSliver;
+
+    // This step assumes the viewport's layout is up-to-date, i.e., if
+    // offset.pixels is changed after the last performLayout, the new scroll
+    // position will not be accounted for.
+    final Matrix4 transform = target.getTransformTo(this);
+    Rect targetRect = MatrixUtils.transformRect(transform, rect);
+
+    // So far leadingScrollOffset is the scroll offset of `rect` in the `child`
+    // sliver's sliver coordinate system. The sign of this value indicates
+    // whether the `rect` protrudes the leading edge of the `child` sliver. When
+    // this value is non-negative and `child`'s `maxScrollObstructionExtent` is
+    // greater than 0, we assume `rect` can't be obstructed by the leading edge
+    // of the viewport (i.e. its pinned to the leading edge).
+    final bool isPinned = sliver.geometry.maxScrollObstructionExtent > 0
+                      && leadingScrollOffset >= 0;
+
     final double extentOfPinnedSlivers = maxScrollObstructionExtentBefore(sliver);
+    // The additional scroll offset needed to move the leading edge of the
+    // `target` to align with the leading edge of the viewport.
     leadingScrollOffset = scrollOffsetOf(sliver, leadingScrollOffset);
+
     switch (sliver.constraints.growthDirection) {
       case GrowthDirection.forward:
+        if (isPinned && alignment <= 0)
+          return RevealedOffset(offset: double.infinity, rect: targetRect);
         leadingScrollOffset -= extentOfPinnedSlivers;
         break;
       case GrowthDirection.reverse:
-        // Nothing to do.
+        if (isPinned && alignment >= 1)
+          return RevealedOffset(offset: double.negativeInfinity, rect: targetRect);
+        switch (axis) {
+          case Axis.vertical:
+            leadingScrollOffset -= targetRect.height;
+            break;
+          case Axis.horizontal:
+            leadingScrollOffset -= targetRect.width;
+            break;
+        }
         break;
     }
 
@@ -816,9 +814,6 @@ abstract class RenderViewportBase<ParentDataClass extends ContainerParentDataMix
     final double targetOffset = leadingScrollOffset - (mainAxisExtent - targetMainAxisExtent) * alignment;
     final double offsetDifference = offset.pixels - targetOffset;
 
-    final Matrix4 transform = target.getTransformTo(this);
-    Rect targetRect = MatrixUtils.transformRect(transform, rect);
-
     switch (axisDirection) {
       case AxisDirection.down:
         targetRect = targetRect.translate(0.0, offsetDifference);

packages/flutter/lib/src/widgets/sliver_persistent_header.dart

@@ -6,6 +6,7 @@
 
 import 'package:flutter/foundation.dart';
 import 'package:flutter/rendering.dart';
+import 'package:flutter/scheduler.dart' show TickerProvider;
 
 import 'framework.dart';
 
@@ -59,6 +60,12 @@ abstract class SliverPersistentHeaderDelegate {
   /// different value.
   double get maxExtent;
 
+  /// A [TickerProvider] to use when animating the header's size changes.
+  ///
+  /// Must not be null if the persistent header is a floating header, and
+  /// [snapConfiguration] or [showOnScreenConfiguration] is not null.
+  TickerProvider get vsync => null;
+
   /// Specifies how floating headers should animate in and out of view.
   ///
   /// If the value of this property is null, then floating headers will
@@ -81,6 +88,12 @@ abstract class SliverPersistentHeaderDelegate {
   /// Defaults to null.
   OverScrollHeaderStretchConfiguration get stretchConfiguration => null;
 
+  /// Specifies how floating headers and pinned pinned headers should behave in
+  /// response to [RenderObject.showOnScreen] calls.
+  ///
+  /// Defaults to null.
+  PersistentHeaderShowOnScreenConfiguration get showOnScreenConfiguration => null;
+
   /// Whether this delegate is meaningfully different from the old delegate.
   ///
   /// If this returns false, then the header might not be rebuilt, even though
@@ -346,7 +359,8 @@ class _SliverPinnedPersistentHeader extends _SliverPersistentHeaderRenderObjectW
   @override
   _RenderSliverPersistentHeaderForWidgetsMixin createRenderObject(BuildContext context) {
     return _RenderSliverPinnedPersistentHeaderForWidgets(
-      stretchConfiguration: delegate.stretchConfiguration
+      stretchConfiguration: delegate.stretchConfiguration,
+      showOnScreenConfiguration: delegate.showOnScreenConfiguration,
     );
   }
 }
@@ -356,9 +370,11 @@ class _RenderSliverPinnedPersistentHeaderForWidgets extends RenderSliverPinnedPe
   _RenderSliverPinnedPersistentHeaderForWidgets({
     RenderBox child,
     OverScrollHeaderStretchConfiguration stretchConfiguration,
+    PersistentHeaderShowOnScreenConfiguration showOnScreenConfiguration,
   }) : super(
     child: child,
     stretchConfiguration: stretchConfiguration,
+    showOnScreenConfiguration: showOnScreenConfiguration,
   );
 }
 
@@ -374,15 +390,19 @@ class _SliverFloatingPersistentHeader extends _SliverPersistentHeaderRenderObjec
   @override
   _RenderSliverPersistentHeaderForWidgetsMixin createRenderObject(BuildContext context) {
     return _RenderSliverFloatingPersistentHeaderForWidgets(
+      vsync: delegate.vsync,
       snapConfiguration: delegate.snapConfiguration,
       stretchConfiguration: delegate.stretchConfiguration,
+      showOnScreenConfiguration: delegate.showOnScreenConfiguration,
     );
   }
 
   @override
   void updateRenderObject(BuildContext context, _RenderSliverFloatingPersistentHeaderForWidgets renderObject) {
+    renderObject.vsync = delegate.vsync;
     renderObject.snapConfiguration = delegate.snapConfiguration;
     renderObject.stretchConfiguration = delegate.stretchConfiguration;
+    renderObject.showOnScreenConfiguration = delegate.showOnScreenConfiguration;
   }
 }
 
@@ -390,12 +410,16 @@ class _RenderSliverFloatingPinnedPersistentHeaderForWidgets extends RenderSliver
   with _RenderSliverPersistentHeaderForWidgetsMixin {
   _RenderSliverFloatingPinnedPersistentHeaderForWidgets({
     RenderBox child,
+    @required TickerProvider vsync,
     FloatingHeaderSnapConfiguration snapConfiguration,
     OverScrollHeaderStretchConfiguration stretchConfiguration,
+    PersistentHeaderShowOnScreenConfiguration showOnScreenConfiguration,
   }) : super(
     child: child,
+    vsync: vsync,
     snapConfiguration: snapConfiguration,
     stretchConfiguration: stretchConfiguration,
+    showOnScreenConfiguration: showOnScreenConfiguration,
   );
 }
 
@@ -411,15 +435,19 @@ class _SliverFloatingPinnedPersistentHeader extends _SliverPersistentHeaderRende
   @override
   _RenderSliverPersistentHeaderForWidgetsMixin createRenderObject(BuildContext context) {
     return _RenderSliverFloatingPinnedPersistentHeaderForWidgets(
+      vsync: delegate.vsync,
       snapConfiguration: delegate.snapConfiguration,
       stretchConfiguration: delegate.stretchConfiguration,
+      showOnScreenConfiguration: delegate.showOnScreenConfiguration,
     );
   }
 
   @override
   void updateRenderObject(BuildContext context, _RenderSliverFloatingPinnedPersistentHeaderForWidgets renderObject) {
+    renderObject.vsync = delegate.vsync;
     renderObject.snapConfiguration = delegate.snapConfiguration;
     renderObject.stretchConfiguration = delegate.stretchConfiguration;
+    renderObject.showOnScreenConfiguration = delegate.showOnScreenConfiguration;
   }
 }
 
@@ -427,11 +455,15 @@ class _RenderSliverFloatingPersistentHeaderForWidgets extends RenderSliverFloati
   with _RenderSliverPersistentHeaderForWidgetsMixin {
   _RenderSliverFloatingPersistentHeaderForWidgets({
     RenderBox child,
+    @required TickerProvider vsync,
     FloatingHeaderSnapConfiguration snapConfiguration,
     OverScrollHeaderStretchConfiguration stretchConfiguration,
+    PersistentHeaderShowOnScreenConfiguration showOnScreenConfiguration,
   }) : super(
     child: child,
+    vsync: vsync,
     snapConfiguration: snapConfiguration,
     stretchConfiguration: stretchConfiguration,
+    showOnScreenConfiguration: showOnScreenConfiguration,
   );
 }

packages/flutter/test/material/app_bar_test.dart

@@ -1975,6 +1975,41 @@ void main() {
     expect(tester.getCenter(appBarTitle).dy, tester.getCenter(toolbar).dy);
   });
 
+  testWidgets('SliverAppBar configures the delegate properly', (WidgetTester tester) async {
+    Future<void> buildAndVerifyDelegate({ bool pinned, bool floating, bool snap }) async {
+      await tester.pumpWidget(
+        MaterialApp(
+          home: CustomScrollView(
+            slivers: <Widget>[
+              SliverAppBar(
+                title: const Text('Jumbo'),
+                pinned: pinned,
+                floating: floating,
+                snap: snap,
+              ),
+            ],
+          ),
+        ),
+      );
+
+      final SliverPersistentHeaderDelegate delegate = tester
+        .widget<SliverPersistentHeader>(find.byType(SliverPersistentHeader))
+        .delegate;
+
+      // Ensure we have a non-null vsync when it's needed.
+      if (!floating || (delegate.snapConfiguration == null && delegate.showOnScreenConfiguration == null))
+        expect(delegate.vsync, isNotNull);
+
+      expect(delegate.showOnScreenConfiguration != null, snap && floating);
+    }
+
+    await buildAndVerifyDelegate(pinned: false, floating: true, snap: false);
+    await buildAndVerifyDelegate(pinned: false, floating: true, snap: true);
+
+    await buildAndVerifyDelegate(pinned: true, floating: true, snap: false);
+    await buildAndVerifyDelegate(pinned: true, floating: true, snap: true);
+  });
+
   testWidgets('AppBar respects toolbarHeight', (WidgetTester tester) async {
     await tester.pumpWidget(
       MaterialApp(

packages/flutter/test/rendering/sliver_persistent_header_test.dart

@@ -49,7 +49,7 @@ void main() {
 class TestRenderSliverFloatingPersistentHeader extends RenderSliverFloatingPersistentHeader {
   TestRenderSliverFloatingPersistentHeader({
     RenderBox child,
-  }) : super(child: child);
+  }) : super(child: child, vsync: null, showOnScreenConfiguration: null);
 
   @override
   double get maxExtent => 200;
@@ -61,7 +61,7 @@ class TestRenderSliverFloatingPersistentHeader extends RenderSliverFloatingPersi
 class TestRenderSliverFloatingPinnedPersistentHeader extends RenderSliverFloatingPinnedPersistentHeader {
   TestRenderSliverFloatingPinnedPersistentHeader({
     RenderBox child,
-  }) : super(child: child);
+  }) : super(child: child, vsync: null, showOnScreenConfiguration: null);
 
   @override
   double get maxExtent => 200;

packages/flutter/test/widgets/editable_text_show_on_screen_test.dart

@@ -10,6 +10,35 @@ import 'package:flutter/material.dart';
 import 'package:flutter/widgets.dart';
 import 'package:flutter/services.dart';
 
+class _TestSliverPersistentHeaderDelegate extends SliverPersistentHeaderDelegate {
+  _TestSliverPersistentHeaderDelegate({
+    this.minExtent,
+    this.maxExtent,
+    this.child,
+    this.vsync = const TestVSync(),
+    this.showOnScreenConfiguration = const PersistentHeaderShowOnScreenConfiguration(),
+  });
+
+  final Widget child;
+
+  @override
+  final double maxExtent;
+
+  @override
+  final double minExtent;
+
+  @override
+  final TickerProvider vsync;
+
+  @override
+  final PersistentHeaderShowOnScreenConfiguration showOnScreenConfiguration;
+
+  @override
+  Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) => child;
+
+  @override
+  bool shouldRebuild(_TestSliverPersistentHeaderDelegate oldDelegate) => true;
+}
 
 void main() {
   const TextStyle textStyle = TextStyle();
@@ -339,6 +368,131 @@ void main() {
     expect(scrollController.offset, greaterThan(0.0));
     expect(find.byKey(container), findsNothing);
   });
+
+  testWidgets(
+    'A pinned persistent header should not scroll when its descendant EditableText gains focus',
+    (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/25507.
+      ScrollController controller;
+      final TextEditingController textEditingController = TextEditingController();
+      final FocusNode focusNode = FocusNode();
+
+      const Key headerKey = Key('header');
+      await tester.pumpWidget(
+        MaterialApp(
+          home: Center(
+            child: SizedBox(
+              height: 600.0,
+              width: 600.0,
+              child: CustomScrollView(
+                controller: controller = ScrollController(initialScrollOffset: 0),
+                slivers: List<Widget>.generate(50, (int i) {
+                  return i == 10
+                  ? SliverPersistentHeader(
+                    pinned: true,
+                    floating: false,
+                    delegate: _TestSliverPersistentHeaderDelegate(
+                      minExtent: 50,
+                      maxExtent: 50,
+                      child: Container(
+                        alignment: Alignment.topCenter,
+                        child: EditableText(
+                          key: headerKey,
+                          backgroundCursorColor: Colors.grey,
+                          controller: textEditingController,
+                          focusNode: focusNode,
+                          style: textStyle,
+                          cursorColor: cursorColor,
+                        ),
+                      ),
+                    ),
+                  )
+                  : SliverToBoxAdapter(
+                    child: Container(
+                      height: 100.0,
+                      child: Text('Tile $i'),
+                    ),
+                  );
+                }),
+              ),
+            ),
+          ),
+        ),
+      );
+
+      // The persistent header should now be pinned at the top.
+      controller.jumpTo(100.0 * 15);
+      await tester.pumpAndSettle();
+      expect(controller.offset, 100.0 * 15);
+
+      focusNode.requestFocus();
+      await tester.pumpAndSettle();
+      // The scroll offset should remain the same.
+      expect(controller.offset, 100.0 * 15);
+  });
+
+  testWidgets(
+    'A pinned persistent header should not scroll when its descendant EditableText gains focus (no animation)',
+    (WidgetTester tester) async {
+      // Regression test for https://github.com/flutter/flutter/issues/25507.
+      ScrollController controller;
+      final TextEditingController textEditingController = TextEditingController();
+      final FocusNode focusNode = FocusNode();
+
+      const Key headerKey = Key('header');
+      await tester.pumpWidget(
+        MaterialApp(
+          home: Center(
+            child: SizedBox(
+              height: 600.0,
+              width: 600.0,
+              child: CustomScrollView(
+                controller: controller = ScrollController(initialScrollOffset: 0),
+                slivers: List<Widget>.generate(50, (int i) {
+                  return i == 10
+                    ? SliverPersistentHeader(
+                      pinned: true,
+                      floating: false,
+                      delegate: _TestSliverPersistentHeaderDelegate(
+                        minExtent: 50,
+                        maxExtent: 50,
+                        vsync: null,
+                        child: Container(
+                          alignment: Alignment.topCenter,
+                          child: EditableText(
+                            key: headerKey,
+                            backgroundCursorColor: Colors.grey,
+                            controller: textEditingController,
+                            focusNode: focusNode,
+                            style: textStyle,
+                            cursorColor: cursorColor,
+                          ),
+                        ),
+                      ),
+                    )
+                    : SliverToBoxAdapter(
+                      child: Container(
+                        height: 100.0,
+                        child: Text('Tile $i'),
+                      ),
+                    );
+                }),
+              ),
+            ),
+          ),
+        ),
+      );
+
+      // The persistent header should now be pinned at the top.
+      controller.jumpTo(100.0 * 15);
+      await tester.pumpAndSettle();
+      expect(controller.offset, 100.0 * 15);
+
+      focusNode.requestFocus();
+      await tester.pumpAndSettle();
+      // The scroll offset should remain the same.
+      expect(controller.offset, 100.0 * 15);
+  });
 }
 
 class NoImplicitScrollPhysics extends AlwaysScrollableScrollPhysics {