Skip to content

Commit

Permalink
sticky_header: Fix hit-testing of header
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gnprice authored and chrisbobbe committed Dec 30, 2023
1 parent 04e78dc commit f3de9a0
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 12 deletions.
16 changes: 11 additions & 5 deletions lib/widgets/sticky_header.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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
Expand Down
42 changes: 35 additions & 7 deletions test/widgets/sticky_header_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,17 @@ Future<void> _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;
Expand All @@ -145,28 +153,39 @@ Future<void> _checkSequence(
: tester.getBottomRight(finder).inDirection(axis.coordinateDirection);
}

void checkState() {
Future<void> checkState() async {
// Check the header comes from the expected item.
final scrollOffset = controller.position.pixels;
final expectedHeaderIndex = first
? (scrollOffset / 100).floor()
: (extent ~/ 100 - 1) + (scrollOffset / 100).ceil();
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<void> 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);
Expand Down Expand Up @@ -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")));
}
}

Expand Down

0 comments on commit f3de9a0

Please sign in to comment.