From cf45130a3ee1161c4a8bd256b4b1b78fdc0df925 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Sun, 1 Jun 2025 06:00:39 +0200 Subject: [PATCH 1/6] Fix accidental click and accidental cross axis swipe --- lib/src/act/act.dart | 50 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/lib/src/act/act.dart b/lib/src/act/act.dart index 6e5d2c71..047f2ff2 100644 --- a/lib/src/act/act.dart +++ b/lib/src/act/act.dart @@ -499,15 +499,49 @@ class Act { if (!targetFullyInViewport) { // drag the target to the location of the dragStart widget (top left corner) final endDragLocation = dragStartRenderBoxRect.topLeft; - final Offset distanceToEnd = + final Offset fullDistanceToEnd = endDragLocation - globalTargetPositionTopLeft; - await gestures.drag(dragBeginPosition, distanceToEnd); - await binding.pump(duration); - finalDragOffset = distanceToEnd; - addDragEvent( - 'Scrolling to fully reveal $targetName.', - direction: distanceToEnd, - ); + + // Only drag in the direction of the scroll axis, never diagonal + final Offset distanceToEnd; + if (scrollAxis == Axis.vertical) { + distanceToEnd = Offset(0, fullDistanceToEnd.dy); + } else { + distanceToEnd = Offset(fullDistanceToEnd.dx, 0); + } + + // Ensure the drag is always recognized as a drag gesture, not a tap + if (distanceToEnd.distance >= kDragSlopDefault) { + // Distance is large enough, drag directly + await gestures.drag(dragBeginPosition, distanceToEnd); + await binding.pump(duration); + finalDragOffset = distanceToEnd; + addDragEvent( + 'Scrolling to fully reveal $targetName.', + direction: distanceToEnd, + ); + } else if (distanceToEnd.distance > 0) { + // Distance is too small, overshoot then return to ensure drag recognition, not a tap. + const overshootDistance = kDragSlopDefault + 1; + final direction = distanceToEnd / distanceToEnd.distance; + final overshootOffset = direction * overshootDistance; + final returnOffset = overshootOffset - distanceToEnd; + + // First drag: overshoot the target + await gestures.drag(dragBeginPosition, overshootOffset); + await binding.pump(duration); + + // Second drag: return to the correct position + await gestures.drag( + dragBeginPosition + overshootOffset, -returnOffset); + await binding.pump(duration); + + finalDragOffset = distanceToEnd; + addDragEvent( + 'Scrolling to fully reveal $targetName.', + direction: distanceToEnd, + ); + } } final totalDragged = From 96690cb9639b6bd4bd49427adc0f0c8962ea7a98 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Sun, 1 Jun 2025 06:38:23 +0200 Subject: [PATCH 2/6] Add padding parameter --- lib/src/act/act.dart | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/src/act/act.dart b/lib/src/act/act.dart index 047f2ff2..11437f35 100644 --- a/lib/src/act/act.dart +++ b/lib/src/act/act.dart @@ -226,6 +226,11 @@ class Act { /// (for example, if its keys get swapped). Providing this fallback can help avoid /// test failures in dynamic layouts, ensuring the final checks can still succeed. /// + /// [padding] defines areas within the scrollable that should be avoided during + /// dragging. The target will not be positioned within the padding when scrolling is complete. + /// This is useful for avoiding fixed headers, footers, or other UI elements that + /// overlap the scrollable content. + /// /// Usage: /// ```dart /// final firstItem = spotText('Item at index: 0')..existsOnce(); @@ -244,6 +249,7 @@ class Act { Duration duration = const Duration(milliseconds: 50), bool toStart = false, WidgetSelector? fallbackScrollableSelector, + EdgeInsets padding = EdgeInsets.zero, }) { assert( !(moveStep != null && toStart), @@ -474,13 +480,21 @@ class Act { spotScrollableBoundsAfterDrag.snapshotRenderBox(); final viewportGlobalPosition = scrollableSizedRenderBoxAfterDrag.localToGlobal(Offset.zero); - final viewportRect = Rect.fromLTWH( + final fullViewportRect = Rect.fromLTWH( viewportGlobalPosition.dx, viewportGlobalPosition.dy, scrollableSizedRenderBoxAfterDrag.size.width, scrollableSizedRenderBoxAfterDrag.size.height, ); + // Account for padding when determining the usable viewport area + final viewportRect = Rect.fromLTRB( + fullViewportRect.left + padding.left, + fullViewportRect.top + padding.top, + fullViewportRect.right - padding.right, + fullViewportRect.bottom - padding.bottom, + ); + final targetRenderBox = dragTarget.snapshotRenderBox(); final Offset globalTargetPositionTopLeft = targetRenderBox.localToGlobal(Offset.zero); @@ -497,8 +511,22 @@ class Act { Offset finalDragOffset = Offset.zero; if (!targetFullyInViewport) { - // drag the target to the location of the dragStart widget (top left corner) - final endDragLocation = dragStartRenderBoxRect.topLeft; + // Calculate the desired end location, respecting padding + final Offset endDragLocation; + if (scrollAxis == Axis.vertical) { + // Position target at top of usable viewport (excluding padding) + endDragLocation = Offset( + globalTargetPositionTopLeft.dx, + viewportRect.top, + ); + } else { + // Position target at left of usable viewport (excluding padding) + endDragLocation = Offset( + viewportRect.left, + globalTargetPositionTopLeft.dy, + ); + } + final Offset fullDistanceToEnd = endDragLocation - globalTargetPositionTopLeft; From d86bdb6822e5f80840c4aa92d894be81de8ce7e0 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Wed, 6 May 2026 13:31:46 +0200 Subject: [PATCH 3/6] Keep overshoot return drag out of padded area Pick a hittable origin for the second overshoot gesture that lies outside the padded strip. Try the natural direction, then the reverse, then walk the line from dragBegin to the preferred end, then a full 8px grid scan. Threading insideArea through _canBePoked lets the same hit test cover the single-point check and both search phases. Add tests covering padding, cross-axis preservation, no item taps, and no pointer-down inside a 90% top banner (regular + small-items overshoot + upward overshoot + bottom banner toStart). Update the two existing position assertions broken by the new viewport-aligned landing. --- lib/src/act/act.dart | 148 ++++- test/act/act_drag_test.dart | 541 +++++++++++++++++- .../drag/drag_until_visible_test_widget.dart | 254 ++++++++ 3 files changed, 927 insertions(+), 16 deletions(-) diff --git a/lib/src/act/act.dart b/lib/src/act/act.dart index 11437f35..35995b2b 100644 --- a/lib/src/act/act.dart +++ b/lib/src/act/act.dart @@ -334,19 +334,24 @@ class Act { final dragBeginPosition = pokablePositionsAtDragStart.mostCenterHittablePosition!; - void addDragEvent(String details, {Offset? direction}) { + void addDragEvent( + String details, { + Offset? direction, + Offset? origin, + }) { if (timeline.mode != TimelineMode.off) { + final crosshair = origin ?? dragBeginPosition; final screenshot = timeline.takeScreenshotSync( annotators: [ - CrosshairAnnotator(centerPosition: dragBeginPosition), + CrosshairAnnotator(centerPosition: crosshair), if (direction != null) ...[ ArrowAnnotator( - start: dragBeginPosition - const Offset(40, 0), - end: dragBeginPosition - const Offset(40, 0) + direction, + start: crosshair - const Offset(40, 0), + end: crosshair - const Offset(40, 0) + direction, ), ArrowAnnotator( - start: dragBeginPosition + const Offset(40, 0), - end: dragBeginPosition + const Offset(40, 0) + direction, + start: crosshair + const Offset(40, 0), + end: crosshair + const Offset(40, 0) + direction, ), ], ], @@ -552,23 +557,70 @@ class Act { // Distance is too small, overshoot then return to ensure drag recognition, not a tap. const overshootDistance = kDragSlopDefault + 1; final direction = distanceToEnd / distanceToEnd.distance; - final overshootOffset = direction * overshootDistance; + + // The second gesture starts at dragBeginPosition + overshootOffset. + // It must be hittable on the scrollable AND outside the padded + // strip, so it never lands in an obscured area like a fixed + // header/footer. Try the natural overshoot direction; if its + // origin lies in the padded strip, flip the direction; if even + // the flipped origin is unsafe (very thin visible band), search + // along the dragBegin → preferred-end line, then fall back to a + // full 8 px grid scan of the scrollable. + Offset overshootOffset = direction * overshootDistance; + Offset secondGestureOrigin = + dragBeginPosition + overshootOffset; + if (!_canBePoked( + position: secondGestureOrigin, + target: scrollableSizedRenderBoxAfterDrag, + insideArea: viewportRect, + )) { + overshootOffset = -overshootOffset; + secondGestureOrigin = dragBeginPosition + overshootOffset; + } + if (!_canBePoked( + position: secondGestureOrigin, + target: scrollableSizedRenderBoxAfterDrag, + insideArea: viewportRect, + )) { + // Search line points first (closest-to-preferred-end first), + // then fall back to a full 8 px grid scan of the scrollable. + secondGestureOrigin = _findPokablePosition( + scrollable: scrollableSizedRenderBoxAfterDrag, + paddedViewport: viewportRect, + preferredPosition: secondGestureOrigin, + priorityPoints: _lineSamples( + dragBeginPosition, + secondGestureOrigin, + ), + ) ?? + // No hittable point of the scrollable is outside the + // padded strip at all — keep the unsafe origin as a best + // effort. (In practice this only happens when the + // scrollable is fully obscured, in which case + // _findPokablePositions would have already failed when + // picking dragBeginPosition.) + secondGestureOrigin; + } final returnOffset = overshootOffset - distanceToEnd; // First drag: overshoot the target await gestures.drag(dragBeginPosition, overshootOffset); await binding.pump(duration); + addDragEvent( + 'Overshoot drag (1/2) to fully reveal $targetName.', + direction: overshootOffset, + ); // Second drag: return to the correct position - await gestures.drag( - dragBeginPosition + overshootOffset, -returnOffset); + await gestures.drag(secondGestureOrigin, -returnOffset); await binding.pump(duration); - - finalDragOffset = distanceToEnd; addDragEvent( - 'Scrolling to fully reveal $targetName.', - direction: distanceToEnd, + 'Return drag (2/2) to fully reveal $targetName.', + direction: -returnOffset, + origin: secondGestureOrigin, ); + + finalDragOffset = distanceToEnd; } } @@ -878,11 +930,79 @@ ${firstUsefulParent.toStringShort()} (${firstUsefulParent.debugWidgetLocation?.f ); } -/// Checks if the widget is visible and not covered by another widget +/// Finds a hittable position on [scrollable] that is outside the padded +/// strip (i.e. inside [paddedViewport]). +/// +/// First tries [priorityPoints] in order — the first one that is hittable on +/// [scrollable] AND inside [paddedViewport] is returned. If none qualify, +/// scans every point of [scrollable] on a [gridSize] grid and returns the +/// hit closest to [preferredPosition]. Returns null if no point of +/// [scrollable] is both hittable and outside the padded strip. +Offset? _findPokablePosition({ + required RenderBox scrollable, + required Rect paddedViewport, + required Offset preferredPosition, + List priorityPoints = const [], + int gridSize = 8, +}) { + for (final candidate in priorityPoints) { + if (_canBePoked( + position: candidate, + target: scrollable, + insideArea: paddedViewport, + )) { + return candidate; + } + } + + final scrollableTopLeft = scrollable.localToGlobal(Offset.zero); + Offset? best; + double bestDist = double.infinity; + for (int x = 0; x < scrollable.size.width; x += gridSize) { + for (int y = 0; y < scrollable.size.height; y += gridSize) { + final candidate = + scrollableTopLeft + Offset(x.toDouble(), y.toDouble()); + if (!_canBePoked( + position: candidate, + target: scrollable, + insideArea: paddedViewport, + )) { + continue; + } + final d = (candidate - preferredPosition).distance; + if (d < bestDist) { + bestDist = d; + best = candidate; + } + } + } + return best; +} + +/// Returns the points of the line from [from] to [to] in [gridSize] steps, +/// ordered from [to] back to [from] (closest-to-[to] first). +List _lineSamples(Offset from, Offset to, {int gridSize = 8}) { + final delta = to - from; + final steps = (delta.distance / gridSize).ceil(); + if (steps == 0) return [from]; + return [ + for (int i = steps; i >= 0; i--) from + delta * (i / steps), + ]; +} + +/// True iff a hit-test at [position] reaches [target] (the target is the +/// topmost hit, not covered by any overlay). When [insideArea] is given, +/// also requires [position] to lie inside that rect — useful for ignoring +/// points that fall in a padded/obscured strip even if they happen to hit +/// [target]. bool _canBePoked({ required Offset position, required RenderObject target, + Rect? insideArea, }) { + if (insideArea != null && !insideArea.contains(position)) { + return false; + } final binding = WidgetsBinding.instance; // do the tap, hit test the position of [target] diff --git a/test/act/act_drag_test.dart b/test/act/act_drag_test.dart index 884134cb..4c71fbfc 100644 --- a/test/act/act_drag_test.dart +++ b/test/act/act_drag_test.dart @@ -60,11 +60,12 @@ void dragTests() { secondItem.existsOnce(); final position = secondItem.snapshotRenderBox().localToGlobal(Offset.zero); + // Target lands at the top of the viewport (cross-axis x preserved). expect( position, within( distance: 10, - from: const Offset(278.9, 443.0), + from: const Offset(278.9, 287.0), ), ); }, @@ -166,11 +167,12 @@ void dragTests() { secondItem.existsOnce(); final position = secondItem.snapshotRenderBox().localToGlobal(Offset.zero); + // Target lands at the left of the viewport (cross-axis y preserved). expect( position, within( distance: 10, - from: const Offset(606.0, 318.0), + from: const Offset(150.0, 318.0), ), ); }, @@ -405,4 +407,539 @@ void dragTests() { }, ); }); + + group('Padding parameter', () { + testWidgets( + 'padding.top keeps target below padded area in vertical ListView', + (tester) async { + await tester.pumpWidget( + const DragUntilVisibleSingleDirectionTestWidget(axis: Axis.vertical), + ); + + final firstItem = spotKey(const ValueKey('item-3')) + ..existsOnce(); + // Use a mid-list target so the scrollable can reach the desired + // alignment (later targets get clamped at max scroll). + final secondItem = spotKey(const ValueKey('item-15')) + ..doesNotExist(); + + const paddingTop = 100.0; + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + padding: const EdgeInsets.only(top: paddingTop), + ); + secondItem.existsOnce(); + + final scrollable = + spot().withChild(secondItem).last().snapshotRenderBox(); + final viewportTop = scrollable.localToGlobal(Offset.zero).dy; + final targetTop = + secondItem.snapshotRenderBox().localToGlobal(Offset.zero).dy; + // Target's top is at or below the padded edge (within 1px tolerance). + expect(targetTop, greaterThanOrEqualTo(viewportTop + paddingTop - 1)); + // ...but not far below it - the drag aligns with the padded edge. + expect(targetTop, lessThan(viewportTop + paddingTop + 5)); + }, + ); + + testWidgets( + 'padding.left keeps target right of padded area in horizontal ListView', + (tester) async { + await tester.pumpWidget( + const DragUntilVisibleSingleDirectionTestWidget( + axis: Axis.horizontal, + ), + ); + + final firstItem = spotKey(const ValueKey('item-2')) + ..existsOnce(); + final secondItem = spotKey(const ValueKey('item-12')) + ..doesNotExist(); + + const paddingLeft = 80.0; + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + padding: const EdgeInsets.only(left: paddingLeft), + ); + secondItem.existsOnce(); + + final scrollable = + spot().withChild(secondItem).last().snapshotRenderBox(); + final viewportLeft = scrollable.localToGlobal(Offset.zero).dx; + final targetLeft = + secondItem.snapshotRenderBox().localToGlobal(Offset.zero).dx; + expect(targetLeft, greaterThanOrEqualTo(viewportLeft + paddingLeft - 1)); + expect(targetLeft, lessThan(viewportLeft + paddingLeft + 5)); + }, + ); + + testWidgets( + 'padding still triggers a final scroll when target overlaps padded area', + (tester) async { + // Verifies that when the target's natural position would place it + // inside the padded area, dragUntilVisible performs an additional + // drag to move it out of the padding. + await tester.pumpWidget( + const DragUntilVisibleSingleDirectionTestWidget(axis: Axis.vertical), + ); + + final firstItem = spotKey(const ValueKey('item-3')); + final secondItem = spotKey(const ValueKey('item-10')) + ..doesNotExist(); + + const paddingTop = 60.0; + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + padding: const EdgeInsets.only(top: paddingTop), + ); + + final scrollable = + spot().withChild(secondItem).last().snapshotRenderBox(); + final viewportTop = scrollable.localToGlobal(Offset.zero).dy; + final targetTop = + secondItem.snapshotRenderBox().localToGlobal(Offset.zero).dy; + expect(targetTop, greaterThanOrEqualTo(viewportTop + paddingTop - 1)); + }, + ); + }); + + group('Cross-axis fix', () { + testWidgets( + 'final adjustment never scrolls the outer cross-axis scrollable ' + '(vertical inner)', + (tester) async { + await tester.pumpWidget( + const CrossAxisNestedScrollableTestWidget(innerAxis: Axis.vertical), + ); + + final outerStateFinder = + find.byType(CrossAxisNestedScrollableTestWidget); + final outerState = + tester.state( + outerStateFinder, + ); + + final firstItem = spotText('Item at index: 3', exact: true) + ..existsOnce(); + final secondItem = spotText('Item at index: 25', exact: true) + ..doesNotExist(); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + ); + secondItem.existsOnce(); + + // Outer (horizontal) scrollable must not have moved. + expect(outerState.outerController.offset, 0.0); + }, + ); + + testWidgets( + 'final adjustment never scrolls the outer cross-axis scrollable ' + '(horizontal inner)', + (tester) async { + await tester.pumpWidget( + const CrossAxisNestedScrollableTestWidget( + innerAxis: Axis.horizontal, + ), + ); + + final outerStateFinder = + find.byType(CrossAxisNestedScrollableTestWidget); + final outerState = + tester.state( + outerStateFinder, + ); + + final firstItem = spotText('Item at index: 2', exact: true) + ..existsOnce(); + final secondItem = spotText('Item at index: 20', exact: true) + ..doesNotExist(); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + ); + secondItem.existsOnce(); + + // Outer (vertical) scrollable must not have moved. + expect(outerState.outerController.offset, 0.0); + }, + ); + }); + + group('Final adjustment drag never registers as tap', () { + testWidgets( + 'tappable items are never tapped during dragUntilVisible (vertical)', + (tester) async { + final taps = {}; + await tester.pumpWidget( + DragUntilVisibleTappableTestWidget( + axis: Axis.vertical, + onItemTap: (index) { + taps[index] = (taps[index] ?? 0) + 1; + }, + ), + ); + + final firstItem = spotText('Item at index: 3', exact: true) + ..existsOnce(); + final secondItem = spotText('Item at index: 27', exact: true) + ..doesNotExist(); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + ); + secondItem.existsOnce(); + + expect(taps, isEmpty); + }, + ); + + testWidgets( + 'tappable items are never tapped during dragUntilVisible (horizontal)', + (tester) async { + final taps = {}; + await tester.pumpWidget( + DragUntilVisibleTappableTestWidget( + axis: Axis.horizontal, + onItemTap: (index) { + taps[index] = (taps[index] ?? 0) + 1; + }, + ), + ); + + final firstItem = spotText('Item at index: 2', exact: true) + ..existsOnce(); + final secondItem = spotText('Item at index: 20', exact: true) + ..doesNotExist(); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + ); + secondItem.existsOnce(); + + expect(taps, isEmpty); + }, + ); + + testWidgets( + 'engineered small final adjustment does not register as tap', + (tester) async { + // Engineer a final adjustment smaller than kDragSlopDefault (20): + // moveStep is tuned so step 1 stops with the target's top just a few + // pixels above the viewport top, leaving < 20 px of final alignment. + // Without the overshoot/return fix this would register as a tap on + // whichever item happens to lie under dragBeginPosition. + final taps = {}; + await tester.pumpWidget( + DragUntilVisibleTappableTestWidget( + axis: Axis.vertical, + onItemTap: (index) { + taps[index] = (taps[index] ?? 0) + 1; + }, + ), + ); + + // Items are 100 tall. Default cacheExtent is 250. Target item 8 sits + // at content y=800 (outside initial cache [-250, 700]). A single drag + // of -810 puts scroll past the target by 10 px — small final drag. + final firstItem = spotText('Item at index: 3', exact: true) + ..existsOnce(); + final secondItem = spotText('Item at index: 8', exact: true); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + moveStep: const Offset(0, -810), + ); + secondItem.existsOnce(); + + expect(taps, isEmpty); + }, + ); + }); + + group('No pointer-down inside padded/obscured area', () { + testWidgets( + 'top banner covers 90% of scrollable - no pointer down lands on banner', + (tester) async { + // A top banner covers the top 90% of the scrollable, simulating a + // fixed header that overlaps the content. With padding.top matching + // the banner height, dragUntilVisible must keep its touch points + // below the banner, otherwise it would tap it. + final pointerDowns = []; + int bannerTaps = 0; + + const topBannerHeight = 450.0; + + await tester.pumpWidget( + DragInObscuredAreaTestWidget( + onPointerDown: (event) => pointerDowns.add(event.position), + onTopBannerTap: () => bannerTaps++, + topBannerHeight: topBannerHeight, + ), + ); + + // dragStart must have a visible portion below the banner. + // With banner covering top 450 px and items 100 px tall, item 4 + // (content y 400-500) has its bottom 50 px visible at startup. + final firstItem = spotKey(const ValueKey('item-4')); + final secondItem = spotKey(const ValueKey('item-15')); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + padding: const EdgeInsets.only(top: topBannerHeight), + ); + + final obscuredRect = _topBannerRect(secondItem, topBannerHeight); + for (final pos in pointerDowns) { + expect( + obscuredRect.contains(pos), + isFalse, + reason: + 'Pointer down at $pos lands inside top banner $obscuredRect.', + ); + } + expect( + bannerTaps, + 0, + reason: 'The top banner registered $bannerTaps tap(s) during drag.', + ); + }, + ); + + testWidgets( + 'overshoot path (downward) keeps every pointer down outside top banner', + (tester) async { + // Engineer a final adjustment smaller than kDragSlopDefault so the + // overshoot/return path runs. The overshoot starts a SECOND gesture + // at dragBeginPosition + overshootOffset. Verify that gesture's + // pointer down still lands below the banner. + final pointerDowns = []; + int bannerTaps = 0; + + const topBannerHeight = 450.0; + + await tester.pumpWidget( + DragInObscuredAreaTestWidget( + onPointerDown: (event) => pointerDowns.add(event.position), + onTopBannerTap: () => bannerTaps++, + topBannerHeight: topBannerHeight, + ), + ); + + // Items 100 tall, target item 15 (content y=1500). Drag of -1060 + // overshoots the desired alignment (target.top = padding.top = 450) + // by ~10 px, so distanceToEnd.dy is +10 (downward) → overshoot path. + final firstItem = spotKey(const ValueKey('item-4')); + final secondItem = spotKey(const ValueKey('item-15')); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + moveStep: const Offset(0, -1060), + padding: const EdgeInsets.only(top: topBannerHeight), + ); + + final obscuredRect = _topBannerRect(secondItem, topBannerHeight); + for (final pos in pointerDowns) { + expect( + obscuredRect.contains(pos), + isFalse, + reason: + 'Pointer down at $pos lands inside top banner $obscuredRect.', + ); + } + expect(bannerTaps, 0); + }, + ); + + testWidgets( + 'overshoot path with smaller items keeps every pointer down outside ' + 'top banner', + (tester) async { + // TODO remove + timeline.mode = TimelineMode.always; + // Smaller items shrink the visible band's hittable area, pushing + // dragBeginPosition closer to the banner edge. With overshoot of + // kDragSlopDefault + 1 (= 21), the SECOND gesture can land inside + // the banner if dragBeginPosition is < 21 px below it. + final pointerDowns = []; + int bannerTaps = 0; + + const topBannerHeight = 450.0; + + await tester.pumpWidget( + DragInObscuredAreaTestWidget( + onPointerDown: (event) { + pointerDowns.add(event.position); + timeline.addEvent(details: 'Tap at ${event.position.dy}', eventType: 'tap down'); + }, + onTopBannerTap: () => bannerTaps++, + topBannerHeight: topBannerHeight, + itemHeight: 60, + ), + ); + + // Items 60 tall, count 30. Item 7 (content y=420-480) is partially + // visible just below the banner; item 25 (content y=1500) is the + // target. moveStep -1040 leaves target 10 px BELOW the desired + // alignment so the overshoot path runs upward. + final firstItem = spotKey(const ValueKey('item-7')); + final secondItem = spotKey(const ValueKey('item-25')); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + moveStep: const Offset(0, -1040), + padding: const EdgeInsets.only(top: topBannerHeight), + ); + + final obscuredRect = _topBannerRect(secondItem, topBannerHeight); + for (final pos in pointerDowns) { + expect( + obscuredRect.contains(pos), + isFalse, + reason: + 'Pointer down at $pos lands inside top banner $obscuredRect.', + ); + } + expect(bannerTaps, 0); + }, + ); + + testWidgets( + 'overshoot path (upward) keeps every pointer down outside top banner', + (tester) async { + // Engineer an UPWARD small final adjustment: target lands just below + // the desired position so distanceToEnd.dy is negative. The + // overshoot/return path then offsets the SECOND gesture by + // overshootOffset (upward), pushing its pointer-down position closer + // to the banner. Verify that down still lands below it. + final pointerDowns = []; + int bannerTaps = 0; + + const topBannerHeight = 450.0; + + await tester.pumpWidget( + DragInObscuredAreaTestWidget( + onPointerDown: (event) => pointerDowns.add(event.position), + onTopBannerTap: () => bannerTaps++, + topBannerHeight: topBannerHeight, + ), + ); + + // Items 100 tall, target item 15 (content y=1500). Drag of -1040 + // leaves the target ~10 px BELOW the desired alignment, so + // distanceToEnd.dy is -10 (upward) → overshoot path going upward. + final firstItem = spotKey(const ValueKey('item-4')); + final secondItem = spotKey(const ValueKey('item-15')); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + moveStep: const Offset(0, -1040), + padding: const EdgeInsets.only(top: topBannerHeight), + ); + + final obscuredRect = _topBannerRect(secondItem, topBannerHeight); + for (final pos in pointerDowns) { + expect( + obscuredRect.contains(pos), + isFalse, + reason: + 'Pointer down at $pos lands inside top banner $obscuredRect.', + ); + } + expect(bannerTaps, 0); + }, + ); + + testWidgets( + 'bottom banner: no pointer down lands on banner during drag toStart', + (tester) async { + // Bottom banner simulates a fixed footer. Use toStart=true so the + // drag traverses the scrollable in the opposite direction; padding + // .bottom matches the banner height. + final pointerDowns = []; + int bannerTaps = 0; + + const bottomBannerHeight = 450.0; + + await tester.pumpWidget( + DragInObscuredAreaTestWidget( + onPointerDown: (event) => pointerDowns.add(event.position), + onBottomBannerTap: () => bannerTaps++, + bottomBannerHeight: bottomBannerHeight, + ), + ); + + // Initially we need the dragStart visible at the top (above the + // bottom banner). Item 0 (content y=0..100) is fully above any + // bottom banner < 400 px tall — its top 50 px (0..50) is above the + // banner when it covers the bottom 450 px. + final firstItem = spotKey(const ValueKey('item-0')); + // Scroll to a target near the start; with toStart we drag from end + // to start. First scroll forward to expose a target far in the list. + final farItem = spotKey(const ValueKey('item-15')); + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: farItem, + padding: const EdgeInsets.only(bottom: bottomBannerHeight), + ); + + // Now drag back toStart to bring item 0 back into view, with + // padding.bottom set so target lands above the bottom banner. + pointerDowns.clear(); + bannerTaps = 0; + await act.dragUntilVisible( + dragStart: farItem, + dragTarget: firstItem, + toStart: true, + padding: const EdgeInsets.only(bottom: bottomBannerHeight), + ); + + final scrollableBox = spot() + .withChild(firstItem) + .last() + .snapshotRenderBox(); + final topLeft = scrollableBox.localToGlobal(Offset.zero); + final obscuredRect = Rect.fromLTWH( + topLeft.dx, + topLeft.dy + scrollableBox.size.height - bottomBannerHeight, + scrollableBox.size.width, + bottomBannerHeight, + ); + for (final pos in pointerDowns) { + expect( + obscuredRect.contains(pos), + isFalse, + reason: 'Pointer down at $pos lands inside bottom banner ' + '$obscuredRect.', + ); + } + expect(bannerTaps, 0); + }, + ); + }); +} + +Rect _topBannerRect(WidgetSelector child, double bannerHeight) { + final scrollableBox = + spot().withChild(child).last().snapshotRenderBox(); + final topLeft = scrollableBox.localToGlobal(Offset.zero); + return Rect.fromLTWH( + topLeft.dx, + topLeft.dy, + scrollableBox.size.width, + bannerHeight, + ); } diff --git a/test/timeline/drag/drag_until_visible_test_widget.dart b/test/timeline/drag/drag_until_visible_test_widget.dart index d2bd899b..1f1ffb32 100644 --- a/test/timeline/drag/drag_until_visible_test_widget.dart +++ b/test/timeline/drag/drag_until_visible_test_widget.dart @@ -21,6 +21,7 @@ class DragUntilVisibleSingleDirectionTestWidget extends StatelessWidget { return IgnorePointer( ignoring: ignorePointerAtIndices.contains(index), child: Container( + key: ValueKey('item-$index'), height: 100, color: index.isEven ? Colors.red : Colors.blue, child: Center(child: Text('Item at index: $index')), @@ -156,3 +157,256 @@ class NestedScrollDragUntilVisibleTestWidget extends StatelessWidget { ); } } + +/// A scrollable list where every item registers tap callbacks. Used to verify +/// that [act.dragUntilVisible] never produces an accidental tap, even when the +/// final adjustment drag is shorter than [kDragSlopDefault]. +class DragUntilVisibleTappableTestWidget extends StatelessWidget { + const DragUntilVisibleTappableTestWidget({ + super.key, + required this.axis, + required this.onItemTap, + this.itemCount = 30, + this.mainAxisItemSize = 100, + }); + + final Axis axis; + final void Function(int index) onItemTap; + final int itemCount; + final double mainAxisItemSize; + + @override + Widget build(BuildContext context) { + final items = List.generate( + itemCount, + (index) { + return GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: () => onItemTap(index), + child: Container( + width: axis == Axis.horizontal ? mainAxisItemSize : null, + height: axis == Axis.vertical ? mainAxisItemSize : null, + color: index.isEven ? Colors.red : Colors.blue, + child: Center(child: Text('Item at index: $index')), + ), + ); + }, + ); + + final direction = axis == Axis.vertical ? 'Vertical' : 'Horizontal'; + return MaterialApp( + home: Scaffold( + appBar: AppBar(title: Text('$direction Tappable Scrollable')), + body: Center( + child: SizedBox( + height: 800, + child: Center( + child: ConstrainedBox( + constraints: const BoxConstraints( + maxWidth: 500, + maxHeight: 450, + ), + child: ListView.builder( + scrollDirection: axis, + itemCount: items.length, + itemBuilder: (_, index) => items[index], + ), + ), + ), + ), + ), + ), + ); + } +} + +/// A scrollable with optional fixed banners covering the top and/or bottom +/// of the scrollable content (simulating a fixed header / footer that +/// overlaps the scrollable). Each banner registers its own `onTap` so tests +/// can assert it was never tapped during a drag. A [Listener] wraps the +/// entire tree so tests can observe every pointer-down event globally. +class DragInObscuredAreaTestWidget extends StatelessWidget { + const DragInObscuredAreaTestWidget({ + super.key, + required this.onPointerDown, + this.onTopBannerTap, + this.onBottomBannerTap, + this.scrollableHeight = 500, + this.topBannerHeight = 0, + this.bottomBannerHeight = 0, + this.itemHeight = 100, + this.itemCount = 30, + }); + + final void Function(PointerDownEvent event) onPointerDown; + + /// Tapped when the top banner receives a recognized tap. Null = no banner. + final VoidCallback? onTopBannerTap; + + /// Tapped when the bottom banner receives a recognized tap. Null = no banner. + final VoidCallback? onBottomBannerTap; + + final double scrollableHeight; + final double topBannerHeight; + final double bottomBannerHeight; + final double itemHeight; + final int itemCount; + + @override + Widget build(BuildContext context) { + final items = List.generate(itemCount, (index) { + return Container( + key: ValueKey('item-$index'), + height: itemHeight, + color: index.isEven ? Colors.red : Colors.blue, + child: Center(child: Text('Item at index: $index')), + ); + }); + + Widget banner({required VoidCallback? onTap, required String label}) { + return GestureDetector( + behavior: HitTestBehavior.opaque, + onTap: onTap, + child: ColoredBox( + color: const Color(0xCC008800), + child: Center(child: Text(label)), + ), + ); + } + + return MaterialApp( + home: Scaffold( + body: Listener( + behavior: HitTestBehavior.translucent, + onPointerDown: onPointerDown, + child: Center( + child: SizedBox( + width: 500, + height: scrollableHeight, + child: Stack( + children: [ + ListView.builder( + itemCount: items.length, + itemBuilder: (_, index) => items[index], + ), + if (topBannerHeight > 0) + Positioned( + top: 0, + left: 0, + right: 0, + height: topBannerHeight, + child: banner( + onTap: onTopBannerTap, + label: 'TOP BANNER', + ), + ), + if (bottomBannerHeight > 0) + Positioned( + bottom: 0, + left: 0, + right: 0, + height: bottomBannerHeight, + child: banner( + onTap: onBottomBannerTap, + label: 'BOTTOM BANNER', + ), + ), + ], + ), + ), + ), + ), + ), + ); + } +} + +/// A scrollable nested in another scrollable on the perpendicular axis. Used +/// to verify that [act.dragUntilVisible] never accidentally scrolls the outer +/// scrollable on the cross axis. +class CrossAxisNestedScrollableTestWidget extends StatefulWidget { + const CrossAxisNestedScrollableTestWidget({ + super.key, + required this.innerAxis, + }); + + /// The axis of the inner scrollable. The outer one is the perpendicular axis. + final Axis innerAxis; + + @override + State createState() => + CrossAxisNestedScrollableTestWidgetState(); +} + +class CrossAxisNestedScrollableTestWidgetState + extends State { + /// Controller for the OUTER scrollable; used by tests to assert it didn't + /// scroll on the cross axis. + final ScrollController outerController = ScrollController(); + + @override + void dispose() { + outerController.dispose(); + super.dispose(); + } + + @override + Widget build(BuildContext context) { + final outerAxis = + widget.innerAxis == Axis.vertical ? Axis.horizontal : Axis.vertical; + + final innerItems = List.generate( + 30, + (index) => Container( + width: widget.innerAxis == Axis.horizontal ? 100 : 200, + height: widget.innerAxis == Axis.vertical ? 100 : 200, + color: index.isEven ? Colors.red : Colors.blue, + child: Center(child: Text('Item at index: $index')), + ), + ); + + final innerScrollable = SizedBox( + width: widget.innerAxis == Axis.horizontal ? 400 : 200, + height: widget.innerAxis == Axis.vertical ? 400 : 200, + child: ListView.builder( + scrollDirection: widget.innerAxis, + itemCount: innerItems.length, + itemBuilder: (_, index) => innerItems[index], + ), + ); + + // Outer is overflowed on its scroll axis (via a spacer after the inner + // scrollable) to give it room to scroll, while keeping the inner + // scrollable at offset 0 so its first items are fully on-screen at + // startup. We use Row/Column so the inner SizedBox isn't forced to stretch + // by tight parent constraints. + final Widget outerChild = outerAxis == Axis.horizontal + ? Row( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + innerScrollable, + const SizedBox(width: 1300), + ], + ) + : Column( + crossAxisAlignment: CrossAxisAlignment.start, + mainAxisSize: MainAxisSize.min, + children: [ + innerScrollable, + const SizedBox(height: 1100), + ], + ); + + return MaterialApp( + home: Scaffold( + appBar: AppBar(title: const Text('Cross-axis nested')), + body: SingleChildScrollView( + controller: outerController, + scrollDirection: outerAxis, + child: outerChild, + ), + ), + ); + } +} From 04d7318fbcbc883588fd341bee25900ccd8e21b3 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Wed, 6 May 2026 13:49:45 +0200 Subject: [PATCH 4/6] Cover original cross-axis bug in tests The previous cross-axis tests used uniform-width items, so dragStart and target shared the same cross-axis position; with that, the diagonal drag the cross-axis lock was meant to prevent never appeared in the first place and the test passed even with the lock removed. Reshape CrossAxisNestedScrollableTestWidget to mirror the original bug report: each list item is a wide tile with a small avatar at the opposite cross-axis end. dragStart picks the avatar, target picks the tile, so their cross-axis offset is large. Start the outer scrollable at a non-zero offset so it has scroll room in either cross-axis direction. Without that, a diagonal drag in the non-scrollable direction would silently no-op and hide the bug. Verified by reverting both cf45130 and 96690cb in lib: the new tests fail (outerHasMoved == true), confirming they catch the bug. --- test/act/act_drag_test.dart | 46 +++++----- .../drag/drag_until_visible_test_widget.dart | 88 +++++++++++++++---- 2 files changed, 92 insertions(+), 42 deletions(-) diff --git a/test/act/act_drag_test.dart b/test/act/act_drag_test.dart index 4c71fbfc..208d5184 100644 --- a/test/act/act_drag_test.dart +++ b/test/act/act_drag_test.dart @@ -508,39 +508,41 @@ void dragTests() { group('Cross-axis fix', () { testWidgets( - 'final adjustment never scrolls the outer cross-axis scrollable ' - '(vertical inner)', + 'avatar dragStart + tile target: outer cross-axis scrollable does not ' + 'move (vertical inner)', (tester) async { + // Reproduces the original use case for the cross-axis fix: the user + // drags from a small AVATAR inside a list item and wants the whole + // TILE visible. Avatar and tile sit at different cross-axis + // positions; without locking the final drag to the scroll axis the + // diagonal would scroll the outer (horizontal) scrollable. await tester.pumpWidget( const CrossAxisNestedScrollableTestWidget(innerAxis: Axis.vertical), ); - final outerStateFinder = - find.byType(CrossAxisNestedScrollableTestWidget); final outerState = tester.state( - outerStateFinder, + find.byType(CrossAxisNestedScrollableTestWidget), ); - final firstItem = spotText('Item at index: 3', exact: true) + final dragStart = spotKey(const ValueKey('avatar-3')) ..existsOnce(); - final secondItem = spotText('Item at index: 25', exact: true) + final target = spotKey(const ValueKey('tile-24')) ..doesNotExist(); await act.dragUntilVisible( - dragStart: firstItem, - dragTarget: secondItem, + dragStart: dragStart, + dragTarget: target, ); - secondItem.existsOnce(); + target.existsOnce(); - // Outer (horizontal) scrollable must not have moved. - expect(outerState.outerController.offset, 0.0); + expect(outerState.outerHasMoved, isFalse); }, ); testWidgets( - 'final adjustment never scrolls the outer cross-axis scrollable ' - '(horizontal inner)', + 'avatar dragStart + tile target: outer cross-axis scrollable does not ' + 'move (horizontal inner)', (tester) async { await tester.pumpWidget( const CrossAxisNestedScrollableTestWidget( @@ -548,26 +550,24 @@ void dragTests() { ), ); - final outerStateFinder = - find.byType(CrossAxisNestedScrollableTestWidget); final outerState = tester.state( - outerStateFinder, + find.byType(CrossAxisNestedScrollableTestWidget), ); - final firstItem = spotText('Item at index: 2', exact: true) + final dragStart = spotKey(const ValueKey('avatar-2')) ..existsOnce(); - final secondItem = spotText('Item at index: 20', exact: true) + final target = spotKey(const ValueKey('tile-21')) ..doesNotExist(); await act.dragUntilVisible( - dragStart: firstItem, - dragTarget: secondItem, + dragStart: dragStart, + dragTarget: target, ); - secondItem.existsOnce(); + target.existsOnce(); // Outer (vertical) scrollable must not have moved. - expect(outerState.outerController.offset, 0.0); + expect(outerState.outerHasMoved, isFalse); }, ); }); diff --git a/test/timeline/drag/drag_until_visible_test_widget.dart b/test/timeline/drag/drag_until_visible_test_widget.dart index 1f1ffb32..325ad660 100644 --- a/test/timeline/drag/drag_until_visible_test_widget.dart +++ b/test/timeline/drag/drag_until_visible_test_widget.dart @@ -341,8 +341,18 @@ class CrossAxisNestedScrollableTestWidget extends StatefulWidget { class CrossAxisNestedScrollableTestWidgetState extends State { /// Controller for the OUTER scrollable; used by tests to assert it didn't - /// scroll on the cross axis. - final ScrollController outerController = ScrollController(); + /// scroll on the cross axis. Started at a non-zero offset so the outer can + /// scroll in either cross-axis direction — without that, a diagonal drag + /// going against the available scroll direction would never move the + /// outer and the test would silently miss the bug. + static const double _outerInitialOffset = 400; + final ScrollController outerController = + ScrollController(initialScrollOffset: _outerInitialOffset); + + /// Whether the outer scrollable's offset has changed from its initial + /// position. Tests assert this stays false. + bool get outerHasMoved => + (outerController.offset - _outerInitialOffset).abs() > 0.5; @override void dispose() { @@ -355,19 +365,50 @@ class CrossAxisNestedScrollableTestWidgetState final outerAxis = widget.innerAxis == Axis.vertical ? Axis.horizontal : Axis.vertical; - final innerItems = List.generate( - 30, - (index) => Container( - width: widget.innerAxis == Axis.horizontal ? 100 : 200, - height: widget.innerAxis == Axis.vertical ? 100 : 200, + // Each item is a "list tile" with an avatar at one end. dragStart + // typically points at the avatar (small, off-center), target points at + // the tile (full width/height). They sit at different cross-axis + // positions, which is the original case the cross-axis fix was about: + // without locking the drag to the scroll axis the final adjustment + // became diagonal and scrolled an outer scrollable. + final innerItems = List.generate(30, (index) { + final tile = Container( + key: ValueKey('tile-$index'), + width: widget.innerAxis == Axis.horizontal ? 100 : 400, + height: widget.innerAxis == Axis.vertical ? 100 : 400, color: index.isEven ? Colors.red : Colors.blue, - child: Center(child: Text('Item at index: $index')), - ), - ); + ); + final avatar = Container( + key: ValueKey('avatar-$index'), + width: 40, + height: 40, + color: Colors.amber, + child: Center(child: Text('$index')), + ); + return SizedBox( + width: widget.innerAxis == Axis.horizontal ? 100 : 400, + height: widget.innerAxis == Axis.vertical ? 100 : 400, + child: Stack( + children: [ + tile, + // Avatar sits at the OPPOSITE end of the cross axis from the + // tile's origin so dragStart (the avatar) and target (the tile) + // have a substantial cross-axis offset. Without locking the + // final drag to the scroll axis, the outer cross-axis scrollable + // would scroll by roughly that offset. + Positioned( + right: widget.innerAxis == Axis.vertical ? 8 : null, + bottom: widget.innerAxis == Axis.horizontal ? 8 : null, + child: avatar, + ), + ], + ), + ); + }); final innerScrollable = SizedBox( - width: widget.innerAxis == Axis.horizontal ? 400 : 200, - height: widget.innerAxis == Axis.vertical ? 400 : 200, + width: 400, + height: 400, child: ListView.builder( scrollDirection: widget.innerAxis, itemCount: innerItems.length, @@ -375,26 +416,35 @@ class CrossAxisNestedScrollableTestWidgetState ), ); - // Outer is overflowed on its scroll axis (via a spacer after the inner - // scrollable) to give it room to scroll, while keeping the inner - // scrollable at offset 0 so its first items are fully on-screen at - // startup. We use Row/Column so the inner SizedBox isn't forced to stretch - // by tight parent constraints. + // Outer has spacers on BOTH sides of the inner scrollable so it can scroll + // in either direction from the initial offset. With an initial offset + // matching the leading spacer, the inner scrollable appears at the start + // of the visible area at startup AND the outer can scroll either way — + // this is what lets the test detect a diagonal drag regardless of its + // sign. We use Row/Column so the inner SizedBox isn't forced to stretch. final Widget outerChild = outerAxis == Axis.horizontal ? Row( crossAxisAlignment: CrossAxisAlignment.start, mainAxisSize: MainAxisSize.min, children: [ + const SizedBox( + width: CrossAxisNestedScrollableTestWidgetState + ._outerInitialOffset, + ), innerScrollable, - const SizedBox(width: 1300), + const SizedBox(width: 1000), ], ) : Column( crossAxisAlignment: CrossAxisAlignment.start, mainAxisSize: MainAxisSize.min, children: [ + const SizedBox( + height: CrossAxisNestedScrollableTestWidgetState + ._outerInitialOffset, + ), innerScrollable, - const SizedBox(height: 1100), + const SizedBox(height: 800), ], ); From ba09710aaab97b64d84632d8a4cfe0ed01d00561 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Wed, 6 May 2026 14:14:49 +0200 Subject: [PATCH 5/6] Format and fix pre-existing test breakages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - dart format on lib/ and test/ touched by previous commits. - Pull main's fix for spotText "chain with parents" — the branch was using bare ColoredBox (which matches multiple instances in a typical Flutter tree); main uses a custom _MyWidget. - Suppress deprecated_member_use infos in generate_widget_matchers.dart for Radio.groupValue / Radio.onChanged. The file is generator-test code that intentionally exercises the old API. --- lib/src/act/act.dart | 6 ++---- test/act/act_drag_test.dart | 13 +++++++------ test/generate_widget_matchers.dart | 2 ++ test/widgets/text_test.dart | 17 +++++++++++++---- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/src/act/act.dart b/lib/src/act/act.dart index 35995b2b..ba27668d 100644 --- a/lib/src/act/act.dart +++ b/lib/src/act/act.dart @@ -567,8 +567,7 @@ class Act { // along the dragBegin → preferred-end line, then fall back to a // full 8 px grid scan of the scrollable. Offset overshootOffset = direction * overshootDistance; - Offset secondGestureOrigin = - dragBeginPosition + overshootOffset; + Offset secondGestureOrigin = dragBeginPosition + overshootOffset; if (!_canBePoked( position: secondGestureOrigin, target: scrollableSizedRenderBoxAfterDrag, @@ -960,8 +959,7 @@ Offset? _findPokablePosition({ double bestDist = double.infinity; for (int x = 0; x < scrollable.size.width; x += gridSize) { for (int y = 0; y < scrollable.size.height; y += gridSize) { - final candidate = - scrollableTopLeft + Offset(x.toDouble(), y.toDouble()); + final candidate = scrollableTopLeft + Offset(x.toDouble(), y.toDouble()); if (!_canBePoked( position: candidate, target: scrollable, diff --git a/test/act/act_drag_test.dart b/test/act/act_drag_test.dart index 208d5184..7d7676de 100644 --- a/test/act/act_drag_test.dart +++ b/test/act/act_drag_test.dart @@ -470,7 +470,8 @@ void dragTests() { final viewportLeft = scrollable.localToGlobal(Offset.zero).dx; final targetLeft = secondItem.snapshotRenderBox().localToGlobal(Offset.zero).dx; - expect(targetLeft, greaterThanOrEqualTo(viewportLeft + paddingLeft - 1)); + expect( + targetLeft, greaterThanOrEqualTo(viewportLeft + paddingLeft - 1)); expect(targetLeft, lessThan(viewportLeft + paddingLeft + 5)); }, ); @@ -781,7 +782,9 @@ void dragTests() { DragInObscuredAreaTestWidget( onPointerDown: (event) { pointerDowns.add(event.position); - timeline.addEvent(details: 'Tap at ${event.position.dy}', eventType: 'tap down'); + timeline.addEvent( + details: 'Tap at ${event.position.dy}', + eventType: 'tap down'); }, onTopBannerTap: () => bannerTaps++, topBannerHeight: topBannerHeight, @@ -907,10 +910,8 @@ void dragTests() { padding: const EdgeInsets.only(bottom: bottomBannerHeight), ); - final scrollableBox = spot() - .withChild(firstItem) - .last() - .snapshotRenderBox(); + final scrollableBox = + spot().withChild(firstItem).last().snapshotRenderBox(); final topLeft = scrollableBox.localToGlobal(Offset.zero); final obscuredRect = Rect.fromLTWH( topLeft.dx, diff --git a/test/generate_widget_matchers.dart b/test/generate_widget_matchers.dart index a7329a55..b4698c47 100644 --- a/test/generate_widget_matchers.dart +++ b/test/generate_widget_matchers.dart @@ -131,7 +131,9 @@ void main() { imports: "import 'package:flutter/material.dart';", ); _generateWidget>( + // ignore: deprecated_member_use builder: () => + // ignore: deprecated_member_use Radio(value: 'a', groupValue: 'a', onChanged: (_) {}), imports: "import 'package:flutter/material.dart';", ); diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index dca1d465..68586bf5 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -183,8 +183,7 @@ void main() { await tester.pumpWidget( _stage( children: [ - ColoredBox( - color: Colors.red, + _MyWidget( child: Text('a'), ), RotatedBox( @@ -197,10 +196,10 @@ void main() { ), ); - spot().spotText('a').existsOnce(); + spot<_MyWidget>().spotText('a').existsOnce(); spot().spotText('b').existsOnce(); - spot().spotText('b').doesNotExist(); + spot<_MyWidget>().spotText('b').doesNotExist(); spot().spotText('a').doesNotExist(); }); }); @@ -406,3 +405,13 @@ Widget _stage({required List children}) { ), ); } + +class _MyWidget extends StatelessWidget { + const _MyWidget({required this.child}); + + final Widget child; + @override + Widget build(BuildContext context) { + return child; + } +} From 7f93d473604b39439c0469c536f9f4c988fd8c83 Mon Sep 17 00:00:00 2001 From: Pascal Welsch Date: Wed, 6 May 2026 14:28:59 +0200 Subject: [PATCH 6/6] Cover the second-origin search and drop unreachable grid scan Add a thin-visible-band test (visible band 30 px between two 235 px banners) that forces both natural and reversed overshoot origins into the obscured strip, so the line-sample search inside _findPokablePosition runs. Drop the grid-scan fallback. The line samples always include dragBeginPosition (i=0), and dragBeginPosition is always safe per _findPokablePositions, so the search always finds at least that. The grid was unreachable. dragUntilVisible coverage on lib/src/act/act.dart: 95% (all new code fully covered; remaining uncovered lines are pre-existing error paths). --- lib/src/act/act.dart | 68 +++++++++---------------------- test/act/act_drag_test.dart | 79 +++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 50 deletions(-) diff --git a/lib/src/act/act.dart b/lib/src/act/act.dart index ba27668d..3bbda4e2 100644 --- a/lib/src/act/act.dart +++ b/lib/src/act/act.dart @@ -581,24 +581,19 @@ class Act { target: scrollableSizedRenderBoxAfterDrag, insideArea: viewportRect, )) { - // Search line points first (closest-to-preferred-end first), - // then fall back to a full 8 px grid scan of the scrollable. + // Walk the line from dragBeginPosition toward the preferred + // origin, closest-to-preferred first, and pick the first safe + // point. dragBeginPosition itself is always safe (the earlier + // _findPokablePositions ensured that), so the search always + // finds at least that as a fallback. secondGestureOrigin = _findPokablePosition( - scrollable: scrollableSizedRenderBoxAfterDrag, - paddedViewport: viewportRect, - preferredPosition: secondGestureOrigin, - priorityPoints: _lineSamples( - dragBeginPosition, - secondGestureOrigin, - ), - ) ?? - // No hittable point of the scrollable is outside the - // padded strip at all — keep the unsafe origin as a best - // effort. (In practice this only happens when the - // scrollable is fully obscured, in which case - // _findPokablePositions would have already failed when - // picking dragBeginPosition.) - secondGestureOrigin; + scrollable: scrollableSizedRenderBoxAfterDrag, + paddedViewport: viewportRect, + priorityPoints: _lineSamples( + dragBeginPosition, + secondGestureOrigin, + ), + )!; } final returnOffset = overshootOffset - distanceToEnd; @@ -929,20 +924,14 @@ ${firstUsefulParent.toStringShort()} (${firstUsefulParent.debugWidgetLocation?.f ); } -/// Finds a hittable position on [scrollable] that is outside the padded -/// strip (i.e. inside [paddedViewport]). -/// -/// First tries [priorityPoints] in order — the first one that is hittable on -/// [scrollable] AND inside [paddedViewport] is returned. If none qualify, -/// scans every point of [scrollable] on a [gridSize] grid and returns the -/// hit closest to [preferredPosition]. Returns null if no point of -/// [scrollable] is both hittable and outside the padded strip. +/// Returns the first point in [priorityPoints] that is hittable on +/// [scrollable] AND inside [paddedViewport]. Returns null if no candidate +/// qualifies. Callers should include a known-safe fallback (e.g. the +/// original [dragBeginPosition]) at the end of [priorityPoints]. Offset? _findPokablePosition({ required RenderBox scrollable, required Rect paddedViewport, - required Offset preferredPosition, - List priorityPoints = const [], - int gridSize = 8, + required List priorityPoints, }) { for (final candidate in priorityPoints) { if (_canBePoked( @@ -953,28 +942,7 @@ Offset? _findPokablePosition({ return candidate; } } - - final scrollableTopLeft = scrollable.localToGlobal(Offset.zero); - Offset? best; - double bestDist = double.infinity; - for (int x = 0; x < scrollable.size.width; x += gridSize) { - for (int y = 0; y < scrollable.size.height; y += gridSize) { - final candidate = scrollableTopLeft + Offset(x.toDouble(), y.toDouble()); - if (!_canBePoked( - position: candidate, - target: scrollable, - insideArea: paddedViewport, - )) { - continue; - } - final d = (candidate - preferredPosition).distance; - if (d < bestDist) { - bestDist = d; - best = candidate; - } - } - } - return best; + return null; } /// Returns the points of the line from [from] to [to] in [gridSize] steps, diff --git a/test/act/act_drag_test.dart b/test/act/act_drag_test.dart index 7d7676de..5542a538 100644 --- a/test/act/act_drag_test.dart +++ b/test/act/act_drag_test.dart @@ -930,6 +930,85 @@ void dragTests() { expect(bannerTaps, 0); }, ); + + testWidgets( + 'thin visible band between two banners triggers second-origin search', + (tester) async { + // Visible band of 30 px is narrower than 2 * overshoot (= 42 px), so + // BOTH the natural and reversed second-gesture origins land inside a + // banner. This is the case the line + grid search in + // _findPokablePosition handles. Verify no banner registers a tap and + // no pointer down lands inside either banner. + final pointerDowns = []; + var topTaps = 0; + var bottomTaps = 0; + + const scrollableHeight = 500.0; + const topBannerHeight = 235.0; + const bottomBannerHeight = 235.0; + + await tester.pumpWidget( + DragInObscuredAreaTestWidget( + onPointerDown: (event) => pointerDowns.add(event.position), + onTopBannerTap: () => topTaps++, + onBottomBannerTap: () => bottomTaps++, + topBannerHeight: topBannerHeight, + bottomBannerHeight: bottomBannerHeight, + itemHeight: 24, + itemCount: 60, + ), + ); + + // Item 10 at content y=240 sits inside the 30 px visible band + // [235, 265] at scroll=0. dragBeginPosition picks (centerX, +248) + // (closest grid point to dragStart's center). Target item 35 at + // content y=840 leaves the target 10 px below the desired alignment + // after a -595 step, so the overshoot path runs upward. Both + // natural origin (+227) and reversed origin (+269) fall outside the + // visible band [235, 265) — line + grid search must pick a safe + // position. + final firstItem = spotKey(const ValueKey('item-10')); + final secondItem = spotKey(const ValueKey('item-35')); + + await act.dragUntilVisible( + dragStart: firstItem, + dragTarget: secondItem, + moveStep: const Offset(0, -595), + padding: const EdgeInsets.symmetric(vertical: topBannerHeight), + ); + + final scrollableBox = + spot().withChild(secondItem).last().snapshotRenderBox(); + final topLeft = scrollableBox.localToGlobal(Offset.zero); + final topBanner = Rect.fromLTWH( + topLeft.dx, + topLeft.dy, + scrollableBox.size.width, + topBannerHeight, + ); + final bottomBanner = Rect.fromLTWH( + topLeft.dx, + topLeft.dy + scrollableHeight - bottomBannerHeight, + scrollableBox.size.width, + bottomBannerHeight, + ); + + for (final pos in pointerDowns) { + expect( + topBanner.contains(pos), + isFalse, + reason: 'Pointer down at $pos lands inside top banner.', + ); + expect( + bottomBanner.contains(pos), + isFalse, + reason: 'Pointer down at $pos lands inside bottom banner.', + ); + } + expect(topTaps, 0); + expect(bottomTaps, 0); + }, + ); }); }