Draggable feedback positioning (flutter#149040)

Reopened after revert in flutter#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 (flutter#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 flutter#145639 for before video): https://github.com/flutter/flutter/assets/42270125/df34e198-0667-453d-a27a-a79b2e2825a1 - fixes flutter#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 <[email protected]>
victorsanni · Jun 28, 2024 · f5b671a · f5b671a
1 parent 3a97a38
commit f5b671a
Show file tree

Hide file tree

Showing 2 changed files with 138 additions and 5 deletions.
diff --git a/packages/flutter/lib/src/widgets/drag_target.dart b/packages/flutter/lib/src/widgets/drag_target.dart
@@ -830,6 +830,7 @@ class _DragAvatar<T extends Object> extends Drag {
   final List<_DragTargetState<Object>> _enteredTargets = <_DragTargetState<Object>>[];
   Offset _position;
   Offset? _lastOffset;
+  late Offset _overlayOffset;
   OverlayEntry? _entry;
 
   @override
@@ -855,7 +856,14 @@ class _DragAvatar<T extends Object> 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<T extends Object> 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
@@ -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<int>(
+            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<int>(
+            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<int>(
+            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<bool> mountedNotifier = ValueNotifier<bool>(true);
+
+    await tester.pumpWidget(ValueListenableBuilder<bool>(
+      valueListenable: mountedNotifier,
+      builder: (_, bool value, __) => value
+          ? const MaterialApp(
+              home: Scaffold(
+                body: Draggable<int>(
+                  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;