sticky_header: Fix hit-testing when header overflows sliver

When the sticky header overflows the sliver that provides it -- that
is, when the sliver boundary is scrolled to within the area the
header covers -- the existing code already got the right visual
result, painting the header at its full size.

But it didn't work properly for hit-testing: trying to tap the
header in the portion where it's overflowing wouldn't work, and
would instead go through to whatever's underneath (like the top of
the next sliver).  That's because the geometry it was reporting from
this `performLayout` method didn't reflect the geometry it would
actually paint in the `paint` method.  When hit-testing, that
reported geometry gets interpreted by the framework code before
calling this render object's other methods.

Specifically, this sliver was reporting to its parent that its
paint region (described by `paintOrigin` and `paintExtent`) was
the same as the child sliver's paint region.  In reality, this
sliver in this case will paint a larger region than that.

Fix by accurately reporting this sliver's paint region (via
`paintOrigin` and `paintExtent`), while adjusting `headerOffset`
to be relative to the new truthful paint region rather than the
old inaccurate one.

This fix lets us mark a swath of test cases as no longer skipped.

On the other hand, this change introduces a different bug in this
overflow case: the child sliver is now painted in the wrong place if
_headerAtCoordinateEnd().  (It gets lined up with the inner edge of
the header, even though it's too short to reach the viewport edge
from there.)  That bug is latent as far as the Zulip app is concerned,
so we leave it for now with a TODO.

Other than that, after this fix, sticky headers overflowing into
the next sliver seem to work completely correctly... as long as
the viewport paints the slivers in the necessary order.  We'll
take care of that in an upcoming PR.

Author: gnprice

lib/widgets/sticky_header.dart

@@ -630,22 +630,30 @@ class _RenderSliverStickyHeaderList extends RenderSliver with RenderSliverHelper
 
           // This sliver's paint region consists entirely of the header.
           final paintExtent = headerExtent;
-          headerOffset = _headerAtCoordinateEnd()
-            ? childExtent - headerExtent // TODO buggy, should be zero
-            : 0.0;
+          headerOffset = 0.0;
 
           // Its layout region (affecting where the next sliver begins layout)
           // is that given by the child sliver.
           final layoutExtent = childExtent;
 
           // The paint origin places this sliver's paint region relative to its
-          // layout region.
-          final paintOrigin = 0.0; // TODO buggy
+          // layout region so that they share the edge the header appears at
+          // (which should be the edge of the viewport).
+          final headerGrowthPlacement =
+            _widget.headerPlacement._byGrowth(constraints.growthDirection);
+          final paintOrigin = switch (headerGrowthPlacement) {
+            _HeaderGrowthPlacement.growthStart => 0.0,
+            _HeaderGrowthPlacement.growthEnd => layoutExtent - paintExtent,
+          };
+          // TODO the child sliver should be painted at offset -paintOrigin
+          //   (This bug doesn't matter so long as the header is opaque,
+          //   because the header covers the child in that case.
+          //   For that reason the Zulip message list isn't affected.)
 
           geometry = SliverGeometry( // TODO review interaction with other slivers
             scrollExtent: geometry.scrollExtent,
             layoutExtent: layoutExtent,
-            paintExtent: childExtent, // TODO buggy
+            paintExtent: paintExtent,
             paintOrigin: paintOrigin,
             maxPaintExtent: math.max(geometry.maxPaintExtent, paintExtent),
             hasVisualOverflow: geometry.hasVisualOverflow

test/widgets/sticky_header_test.dart

@@ -228,6 +228,48 @@ void main() {
     await tester.pump();
     checkState(103, item:   0, header:   0);
   });
+
+  testWidgets('hit-testing for header overflowing sliver', (tester) async {
+    final controller = ScrollController();
+    await tester.pumpWidget(Directionality(textDirection: TextDirection.ltr,
+      child: CustomScrollView(
+        controller: controller,
+        slivers: [
+          SliverStickyHeaderList(
+            headerPlacement: HeaderPlacement.scrollingStart,
+            delegate: SliverChildListDelegate(
+              List.generate(100, (i) => StickyHeaderItem(
+                allowOverflow: true,
+                header: _Header(i, height: 20),
+                child: _Item(i, height: 100))))),
+          SliverStickyHeaderList(
+            headerPlacement: HeaderPlacement.scrollingStart,
+            delegate: SliverChildListDelegate(
+              List.generate(100, (i) => StickyHeaderItem(
+                allowOverflow: true,
+                header: _Header(100 + i, height: 20),
+                child: _Item(100 + i, height: 100))))),
+        ])));
+
+    const topExtent = 100 * 100;
+    for (double topHeight in [5, 10, 15, 20]) {
+      controller.jumpTo(topExtent - topHeight);
+      await tester.pump();
+      // The top sliver occupies height [topHeight].
+      // Its header overhangs by `20 - topHeight`.
+
+      final expected = <Condition<Object?>>[];
+      for (int y = 1; y < 20; y++) {
+        await tester.tapAt(Offset(400, y.toDouble()));
+        expected.add((it) => it.isA<_Header>().index.equals(99));
+      }
+      for (int y = 21; y < 40; y += 2) {
+        await tester.tapAt(Offset(400, y.toDouble()));
+        expected.add((it) => it.isA<_Item>().index.equals(100));
+      }
+      check(_TapLogged.takeTapLog()).deepEquals(expected);
+    }
+  });
 }
 
 enum _SliverConfig {
@@ -255,19 +297,6 @@ Future<void> _checkSequence(
   final headerPlacement = reverseHeader ^ reverse
     ? HeaderPlacement.scrollingEnd : HeaderPlacement.scrollingStart;
 
-  if (allowOverflow
-      && ((sliverConfig == _SliverConfig.backToBack
-           && (reverse ^ reverseHeader))
-       || (sliverConfig == _SliverConfig.followed
-           && (reverse ^ reverseHeader ^ !reverseGrowth)))) {
-    // (The condition for this skip is pretty complicated; it's just the
-    // conditions where the bug gets triggered, and I haven't tried to
-    // work through why this exact set of cases is what's affected.
-    // The important thing is they all get fixed in an upcoming commit.)
-    markTestSkipped('bug in header overflowing sliver'); // TODO fix
-    return;
-  }
-
   Widget buildItem(int i) {
     return StickyHeaderItem(
       allowOverflow: allowOverflow,
@@ -377,7 +406,15 @@ Future<void> _checkSequence(
       100 - (first ? scrollOffset % 100 : (-scrollOffset) % 100);
     final double expectedHeaderInsetExtent =
       allowOverflow ? 20 : math.min(20, expectedItemInsetExtent);
-    check(insetExtent(itemFinder)).equals(expectedItemInsetExtent);
+    if (expectedItemInsetExtent < expectedHeaderInsetExtent) {
+      // TODO there's a bug here if the header isn't opaque;
+      //   this check would exercise the bug:
+      // check(insetExtent(itemFinder)).equals(expectedItemInsetExtent);
+      // Instead, check that things will be fine if the header is opaque.
+      check(insetExtent(itemFinder)).isLessOrEqual(expectedHeaderInsetExtent);
+    } else {
+      check(insetExtent(itemFinder)).equals(expectedItemInsetExtent);
+    }
     check(insetExtent(find.byType(_Header))).equals(expectedHeaderInsetExtent);
 
     // Check the header gets hit when it should, and not when it shouldn't.
@@ -572,6 +609,10 @@ class _Header extends StatelessWidget implements _TapLogged {
   }
 }
 
+extension _HeaderChecks on Subject<_Header> {
+  Subject<int> get index => has((x) => x.index, 'index');
+}
+
 class _Item extends StatelessWidget implements _TapLogged {
   const _Item(this.index, {required this.height});
 
@@ -595,6 +636,10 @@ class _Item extends StatelessWidget implements _TapLogged {
   }
 }
 
+extension _ItemChecks on Subject<_Item> {
+  Subject<int> get index => has((x) => x.index, 'index');
+}
+
 /// Sets [DeviceGestureSettings.touchSlop] for the child subtree
 /// to the given value, by inserting a [MediaQuery].
 ///