From f5b671aac7239a2de7f12d66c398f193b8a1273e Mon Sep 17 00:00:00 2001 From: Tim Lehmann Date: Fri, 28 Jun 2024 20:14:20 +0200 Subject: [PATCH] Draggable feedback positioning (#149040) Reopened after revert in #147658 Another test was added in the meantime to `draggable_test.dart` that didn't call `gesture.up()`. I added this call to the test and now all tests pass. --- # Original Description (#145647): We changed the coordinates used to position the `Draggable` feedback by transforming them into the `Overlay`s coordinate space. This has no influence on any untransformed `Overlay`, most Flutter apps should be not affected. This PR fixes the positioning of the feedback in transformed context (see #145639 for before video): https://github.com/flutter/flutter/assets/42270125/df34e198-0667-453d-a27a-a79b2e2825a1 - fixes #145639 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat [Data Driven Fixes]: https://github.com/flutter/flutter/wiki/Data-driven-Fixes --------- Co-authored-by: Jesper Bellenbaum --- .../flutter/lib/src/widgets/drag_target.dart | 16 ++- .../flutter/test/widgets/draggable_test.dart | 127 ++++++++++++++++++ 2 files changed, 138 insertions(+), 5 deletions(-) diff --git a/packages/flutter/lib/src/widgets/drag_target.dart b/packages/flutter/lib/src/widgets/drag_target.dart index 5942683887a3f..677b11b062590 100644 --- a/packages/flutter/lib/src/widgets/drag_target.dart +++ b/packages/flutter/lib/src/widgets/drag_target.dart @@ -830,6 +830,7 @@ class _DragAvatar extends Drag { final List<_DragTargetState> _enteredTargets = <_DragTargetState>[]; Offset _position; Offset? _lastOffset; + late Offset _overlayOffset; OverlayEntry? _entry; @override @@ -855,7 +856,14 @@ class _DragAvatar extends Drag { void updateDrag(Offset globalPosition) { _lastOffset = globalPosition - dragStartPoint; - _entry!.markNeedsBuild(); + if (overlayState.mounted) { + final RenderBox box = overlayState.context.findRenderObject()! as RenderBox; + final Offset overlaySpaceOffset = box.globalToLocal(globalPosition); + _overlayOffset = overlaySpaceOffset - dragStartPoint; + + _entry!.markNeedsBuild(); + } + final HitTestResult result = HitTestResult(); WidgetsBinding.instance.hitTestInView(result, globalPosition + feedbackOffset, viewId); @@ -940,11 +948,9 @@ class _DragAvatar extends Drag { } Widget _build(BuildContext context) { - final RenderBox box = overlayState.context.findRenderObject()! as RenderBox; - final Offset overlayTopLeft = box.localToGlobal(Offset.zero); return Positioned( - left: _lastOffset!.dx - overlayTopLeft.dx, - top: _lastOffset!.dy - overlayTopLeft.dy, + left: _overlayOffset.dx, + top: _overlayOffset.dy, child: ExcludeSemantics( excluding: ignoringFeedbackSemantics, child: IgnorePointer( diff --git a/packages/flutter/test/widgets/draggable_test.dart b/packages/flutter/test/widgets/draggable_test.dart index 0432b8530c617..9551b2063a9f7 100644 --- a/packages/flutter/test/widgets/draggable_test.dart +++ b/packages/flutter/test/widgets/draggable_test.dart @@ -3497,6 +3497,133 @@ void main() { await tester.pumpAndSettle(); }); + testWidgets('Drag and drop - feedback matches pointer in scaled MaterialApp', (WidgetTester tester) async { + await tester.pumpWidget(Transform.scale( + scale: 0.5, + child: const MaterialApp( + home: Scaffold( + body: Draggable( + data: 42, + feedback: Text('Feedback'), + child: Text('Source'), + ), + ), + ), + )); + + final Offset location = tester.getTopLeft(find.text('Source')); + final TestGesture gesture = await tester.startGesture(location); + final Offset secondLocation = location + const Offset(100, 100); + await gesture.moveTo(secondLocation); + await tester.pump(); + final Offset appTopLeft = tester.getTopLeft(find.byType(MaterialApp)); + expect(tester.getTopLeft(find.text('Source')), appTopLeft); + expect(tester.getTopLeft(find.text('Feedback')), secondLocation); + + // Finish gesture to release resources. + await gesture.up(); + await tester.pump(); + }); + + testWidgets('Drag and drop - childDragAnchorStrategy works in scaled MaterialApp', (WidgetTester tester) async { + final Key sourceKey = UniqueKey(); + final Key feedbackKey = UniqueKey(); + await tester.pumpWidget(Transform.scale( + scale: 0.5, + child: MaterialApp( + home: Scaffold( + body: Draggable( + data: 42, + feedback: Text('Text', key: feedbackKey), + child: Text('Text', key: sourceKey), + ), + ), + ), + )); + final Finder source = find.byKey(sourceKey); + final Finder feedback = find.byKey(feedbackKey); + + final TestGesture gesture = await tester.startGesture(tester.getCenter(source)); + await tester.pump(); + expect(tester.getTopLeft(source), tester.getTopLeft(feedback)); + + // Finish gesture to release resources. + await gesture.up(); + await tester.pump(); + }); + + testWidgets('Drag and drop - feedback matches pointer in rotated MaterialApp', (WidgetTester tester) async { + await tester.pumpWidget(Transform.rotate( + angle: 1, // ~57 degrees + child: const MaterialApp( + home: Scaffold( + body: Draggable( + data: 42, + feedback: Text('Feedback'), + child: Text('Source'), + ), + ), + ), + )); + + final Offset location = tester.getTopLeft(find.text('Source')); + final TestGesture gesture = await tester.startGesture(location); + final Offset secondLocation = location + const Offset(100, 100); + await gesture.moveTo(secondLocation); + await tester.pump(); + final Offset appTopLeft = tester.getTopLeft(find.byType(MaterialApp)); + expect(tester.getTopLeft(find.text('Source')), appTopLeft); + final Offset feedbackTopLeft = tester.getTopLeft(find.text('Feedback')); + + // Different rotations can incur rounding errors, this makes it more robust + expect(feedbackTopLeft.dx, moreOrLessEquals(secondLocation.dx)); + expect(feedbackTopLeft.dy, moreOrLessEquals(secondLocation.dy)); + + // Finish gesture to release resources. + await gesture.up(); + await tester.pump(); + }); + + testWidgets('Drag and drop - unmounting overlay ends drag gracefully', (WidgetTester tester) async { + final ValueNotifier mountedNotifier = ValueNotifier(true); + + await tester.pumpWidget(ValueListenableBuilder( + valueListenable: mountedNotifier, + builder: (_, bool value, __) => value + ? const MaterialApp( + home: Scaffold( + body: Draggable( + data: 42, + feedback: Text('Feedback'), + child: Text('Source'), + ), + ), + ) + : Container(), + )); + + final Offset location = tester.getCenter(find.text('Source')); + final TestGesture gesture = await tester.startGesture(location); + final Offset secondLocation = location + const Offset(100, 100); + await gesture.moveTo(secondLocation); + await tester.pump(); + expect(find.text('Feedback'), findsOneWidget); + + // Unmount overlay + mountedNotifier.value = false; + await tester.pump(); + + // This should not throw + await gesture.moveTo(location); + + expect(find.byType(Container), findsOneWidget); + expect(find.text('Feedback'), findsNothing); + + // Finish gesture to release resources. + await gesture.up(); + await tester.pump(); + }); + testWidgets('configurable Draggable hit test behavior', (WidgetTester tester) async { const HitTestBehavior hitTestBehavior = HitTestBehavior.deferToChild;