From f3de9a057d6242811746acbc4de1749a892c1600 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Thu, 28 Dec 2023 19:56:28 -0800 Subject: [PATCH] sticky_header: Fix hit-testing of header Our `hitTestChildren` implementation wasn't using `hitTestBoxChild` correctly: it was attempting to do the job of `childMainAxisPosition`, but `hitTestBoxChild` calls `childMainAxisPosition` for itself. The implementation of `childMainAxisPosition` wasn't correct either. Fix both, and add some tests; sticky_header hadn't had any tests on its hit-test behavior. Fixes: #327 --- lib/widgets/sticky_header.dart | 16 +++++++---- test/widgets/sticky_header_test.dart | 42 +++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/lib/widgets/sticky_header.dart b/lib/widgets/sticky_header.dart index 597b13bf15..0a1e042e07 100644 --- a/lib/widgets/sticky_header.dart +++ b/lib/widgets/sticky_header.dart @@ -592,11 +592,8 @@ class _RenderSliverStickyHeaderList extends RenderSliver with RenderSliverHelper assert(child != null); assert(geometry!.hitTestExtent > 0.0); if (header != null) { - final headerParentData = (header!.parentData as SliverPhysicalParentData); - final headerOffset = headerParentData.paintOffset - .inDirection(constraints.axisDirection); if (hitTestBoxChild(BoxHitTestResult.wrap(result), header!, - mainAxisPosition: mainAxisPosition - headerOffset, + mainAxisPosition: mainAxisPosition, crossAxisPosition: crossAxisPosition)) { return true; } @@ -609,8 +606,17 @@ class _RenderSliverStickyHeaderList extends RenderSliver with RenderSliverHelper double childMainAxisPosition(RenderObject child) { if (child == this.child) return 0.0; assert(child == header); + // We use Sliver*Physical*ParentData, so the header's position is stored in + // physical coordinates. To meet the spec of `childMainAxisPosition`, we + // need to convert to the sliver's coordinate system. final headerParentData = (header!.parentData as SliverPhysicalParentData); - return headerParentData.paintOffset.inDirection(constraints.axisDirection); + final paintOffset = headerParentData.paintOffset; + return switch (constraints.axisDirection) { + AxisDirection.right => paintOffset.dx, + AxisDirection.left => geometry!.layoutExtent - header!.size.width - paintOffset.dx, + AxisDirection.down => paintOffset.dy, + AxisDirection.up => geometry!.layoutExtent - header!.size.height - paintOffset.dy, + }; } @override diff --git a/test/widgets/sticky_header_test.dart b/test/widgets/sticky_header_test.dart index 55e62f2d19..011899c11a 100644 --- a/test/widgets/sticky_header_test.dart +++ b/test/widgets/sticky_header_test.dart @@ -132,9 +132,17 @@ Future _checkSequence( header: _Header(i, height: 20), child: _Item(i, height: 100)))))); - final extent = tester.getSize(find.byType(StickyHeaderListView)).onAxis(axis); + final overallSize = tester.getSize(find.byType(StickyHeaderListView)); + final extent = overallSize.onAxis(axis); assert(extent % 100 == 0); + // A position `inset` from the center of the edge the header is found on. + Offset headerInset(double inset) { + return overallSize.center(Offset.zero) + + offsetInDirection(axis.coordinateDirection, + (extent / 2 - inset) * (headerAtCoordinateEnd ? 1 : -1)); + } + final first = !(reverse ^ reverseHeader); final itemFinder = first ? find.byType(_Item).first : find.byType(_Item).last; @@ -145,7 +153,8 @@ Future _checkSequence( : tester.getBottomRight(finder).inDirection(axis.coordinateDirection); } - void checkState() { + Future checkState() async { + // Check the header comes from the expected item. final scrollOffset = controller.position.pixels; final expectedHeaderIndex = first ? (scrollOffset / 100).floor() @@ -153,20 +162,30 @@ Future _checkSequence( check(tester.widget<_Item>(itemFinder).index).equals(expectedHeaderIndex); check(_headerIndex(tester)).equals(expectedHeaderIndex); + // Check the layout of the header and item. final expectedItemInsetExtent = 100 - (first ? scrollOffset % 100 : (-scrollOffset) % 100); + final double expectedHeaderInsetExtent = + allowOverflow ? 20 : math.min(20, expectedItemInsetExtent); check(insetExtent(itemFinder)).equals(expectedItemInsetExtent); - check(insetExtent(find.byType(_Header))).equals( - allowOverflow ? 20 : math.min(20, expectedItemInsetExtent)); + check(insetExtent(find.byType(_Header))).equals(expectedHeaderInsetExtent); + + // Check the header gets hit when it should, and not when it shouldn't. + await tester.tapAt(headerInset(1)); + await tester.tapAt(headerInset(expectedHeaderInsetExtent - 1)); + check(_Header.takeTapCount()).equals(2); + await tester.tapAt(headerInset(extent - 1)); + await tester.tapAt(headerInset(extent - (expectedHeaderInsetExtent - 1))); + check(_Header.takeTapCount()).equals(0); } Future jumpAndCheck(double position) async { controller.jumpTo(position); await tester.pump(); - checkState(); + await checkState(); } - checkState(); + await checkState(); await jumpAndCheck(5); await jumpAndCheck(10); await jumpAndCheck(20); @@ -210,12 +229,21 @@ class _Header extends StatelessWidget { final int index; final double height; + static int takeTapCount() { + final result = _tapCount; + _tapCount = 0; + return result; + } + static int _tapCount = 0; + @override Widget build(BuildContext context) { return SizedBox( height: height, width: height, // TODO clean up - child: Text("Header $index")); + child: GestureDetector( + onTap: () => _tapCount++, + child: Text("Header $index"))); } }