Browse Source

Merge pull request #1162 from AppFlowy-IO/fix/1156

fix: calculate overlap
Nathan.fooo 2 years ago
parent
commit
7037378947

+ 7 - 4
frontend/app_flowy/packages/appflowy_board/lib/src/utils/log.dart

@@ -8,25 +8,28 @@ class Log {
 
   static void info(String? message) {
     if (enableLog) {
-      debugPrint('ℹ️[Info]=> $message');
+      debugPrint('AppFlowyBoard: ℹ️[Info]=> $message');
     }
   }
 
   static void debug(String? message) {
     if (enableLog) {
-      debugPrint('🐛[Debug] - ${DateTime.now().second}=> $message');
+      debugPrint(
+          'AppFlowyBoard: 🐛[Debug] - ${DateTime.now().second}=> $message');
     }
   }
 
   static void warn(String? message) {
     if (enableLog) {
-      debugPrint('🐛[Warn] - ${DateTime.now().second} => $message');
+      debugPrint(
+          'AppFlowyBoard: 🐛[Warn] - ${DateTime.now().second} => $message');
     }
   }
 
   static void trace(String? message) {
     if (enableLog) {
-      debugPrint('❗️[Trace] - ${DateTime.now().second}=> $message');
+      debugPrint(
+          'AppFlowyBoard: ❗️[Trace] - ${DateTime.now().second}=> $message');
     }
   }
 }

+ 8 - 8
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_data.dart

@@ -217,15 +217,15 @@ class AppFlowyBoardController extends ChangeNotifier
     final fromGroupItem = fromGroupController.removeAt(fromGroupIndex);
     if (toGroupController.items.length > toGroupIndex) {
       assert(toGroupController.items[toGroupIndex] is PhantomGroupItem);
-    }
 
-    toGroupController.replace(toGroupIndex, fromGroupItem);
-    onMoveGroupItemToGroup?.call(
-      fromGroupId,
-      fromGroupIndex,
-      toGroupId,
-      toGroupIndex,
-    );
+      toGroupController.replace(toGroupIndex, fromGroupItem);
+      onMoveGroupItemToGroup?.call(
+        fromGroupId,
+        fromGroupIndex,
+        toGroupId,
+        toGroupIndex,
+      );
+    }
   }
 
   @override

+ 1 - 1
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group.dart

@@ -112,7 +112,7 @@ class AppFlowyBoardGroup extends StatefulWidget {
     this.itemMargin = EdgeInsets.zero,
     this.cornerRadius = 0.0,
     this.backgroundColor = Colors.transparent,
-  })  : config = const ReorderFlexConfig(setStateWhenEndDrag: false),
+  })  : config = const ReorderFlexConfig(),
         super(key: key);
 
   @override

+ 2 - 0
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/board_group/group_data.dart

@@ -124,6 +124,8 @@ class AppFlowyGroupController extends ChangeNotifier with EquatableMixin {
       Log.debug('[$AppFlowyGroupController] $groupData add $newItem');
     } else {
       if (index >= groupData._items.length) {
+        Log.warn(
+            '[$AppFlowyGroupController] unexpected items length, index should less than the count of the items. Index: $index, items count: ${items.length}');
         return;
       }
 

+ 6 - 22
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_flex/drag_state.dart

@@ -22,6 +22,8 @@ class FlexDragTargetData extends DragTargetData {
 
   Size? get feedbackSize => _state.feedbackSize;
 
+  bool get isDragging => _state.isDragging();
+
   final String dragTargetId;
 
   Offset dragTargetOffset = Offset.zero;
@@ -48,39 +50,20 @@ class FlexDragTargetData extends DragTargetData {
 
   bool isOverlapWithWidgets(List<GlobalObjectKey> widgetKeys) {
     final renderBox = dragTargetIndexKey.currentContext?.findRenderObject();
-
     if (renderBox == null) return false;
     if (renderBox is! RenderBox) return false;
     final size = feedbackSize ?? Size.zero;
-    final Rect rect = dragTargetOffset & size;
 
+    final Rect dragTargetRect = renderBox.localToGlobal(Offset.zero) & size;
     for (final widgetKey in widgetKeys) {
       final renderObject = widgetKey.currentContext?.findRenderObject();
       if (renderObject != null && renderObject is RenderBox) {
         Rect widgetRect =
             renderObject.localToGlobal(Offset.zero) & renderObject.size;
-        // return rect.overlaps(widgetRect);
-        if (rect.right <= widgetRect.left || widgetRect.right <= rect.left) {
-          return false;
-        }
-
-        if (rect.bottom <= widgetRect.top || widgetRect.bottom <= rect.top) {
-          return false;
-        }
-        return true;
+        return dragTargetRect.overlaps(widgetRect);
       }
     }
 
-    // final HitTestResult result = HitTestResult();
-    // WidgetsBinding.instance.hitTest(result, position);
-    // for (final HitTestEntry entry in result.path) {
-    //   final HitTestTarget target = entry.target;
-    //   if (target is RenderMetaData) {
-    //     print(target.metaData);
-    //   }
-    //   print(target);
-    // }
-
     return false;
   }
 }
@@ -113,7 +96,7 @@ class DraggingState {
   int currentIndex = -1;
 
   /// The widget to move the dragging widget too after the current index.
-  int nextIndex = 0;
+  int nextIndex = -1;
 
   /// Whether or not we are currently scrolling this view to show a widget.
   bool scrolling = false;
@@ -149,6 +132,7 @@ class DraggingState {
     dragStartIndex = -1;
     phantomIndex = -1;
     currentIndex = -1;
+    nextIndex = -1;
     _draggingWidget = null;
   }
 

+ 28 - 13
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_flex/drag_target.dart

@@ -1,3 +1,4 @@
+import 'package:appflowy_board/src/utils/log.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/scheduler.dart';
 import 'package:provider/provider.dart';
@@ -184,8 +185,9 @@ class _ReorderDragTargetState<T extends DragTargetData>
           /// When the drag does not end inside a DragTarget widget, the
           /// drag fails, but we still reorder the widget to the last position it
           /// had been dragged to.
-          onDraggableCanceled: (Velocity velocity, Offset offset) =>
-              widget.onDragEnded(widget.dragTargetData),
+          onDraggableCanceled: (Velocity velocity, Offset offset) {
+            widget.onDragEnded(widget.dragTargetData);
+          },
           child: widget.child,
         );
 
@@ -221,8 +223,11 @@ class DragTargetAnimation {
   // where the widget used to be.
   late AnimationController phantomController;
 
+  // Uses to simulate the insert animation when card was moved from on group to
+  // another group. Check out the [FakeDragTarget].
   late AnimationController insertController;
 
+  // Used to remove the phantom
   late AnimationController deleteController;
 
   DragTargetAnimation({
@@ -238,7 +243,7 @@ class DragTargetAnimation {
         value: 0, vsync: vsync, duration: reorderAnimationDuration);
 
     insertController = AnimationController(
-        value: 0.0, vsync: vsync, duration: const Duration(milliseconds: 200));
+        value: 0.0, vsync: vsync, duration: const Duration(milliseconds: 100));
 
     deleteController = AnimationController(
         value: 0.0, vsync: vsync, duration: const Duration(milliseconds: 1));
@@ -423,7 +428,6 @@ abstract class FakeDragTargetEventData {
 }
 
 class FakeDragTarget<T extends DragTargetData> extends StatefulWidget {
-  final Duration animationDuration;
   final FakeDragTargetEventTrigger eventTrigger;
   final FakeDragTargetEventData eventData;
   final DragTargetOnStarted onDragStarted;
@@ -442,7 +446,6 @@ class FakeDragTarget<T extends DragTargetData> extends StatefulWidget {
     required this.insertAnimationController,
     required this.deleteAnimationController,
     required this.child,
-    this.animationDuration = const Duration(milliseconds: 250),
   }) : super(key: key);
 
   @override
@@ -468,6 +471,7 @@ class _FakeDragTargetState<T extends DragTargetData>
     // });
 
     widget.eventTrigger.fakeOnDragEnded(() {
+      Log.trace("[$FakeDragTarget] on drag end");
       WidgetsBinding.instance.addPostFrameCallback((_) {
         widget.onDragEnded(widget.eventData.dragTargetData as T);
       });
@@ -476,6 +480,13 @@ class _FakeDragTargetState<T extends DragTargetData>
     super.initState();
   }
 
+  @override
+  void dispose() {
+    widget.insertAnimationController
+        .removeStatusListener(_onInsertedAnimationStatusChanged);
+    super.dispose();
+  }
+
   @override
   Widget build(BuildContext context) {
     if (simulateDragging) {
@@ -503,14 +514,18 @@ class _FakeDragTargetState<T extends DragTargetData>
     WidgetsBinding.instance.addPostFrameCallback((_) {
       if (!mounted) return;
       setState(() {
-        simulateDragging = true;
-        widget.deleteAnimationController.reverse(from: 1.0);
-        widget.onWillAccept(widget.eventData.dragTargetData as T);
-        widget.onDragStarted(
-          widget.child,
-          widget.eventData.index,
-          widget.eventData.feedbackSize,
-        );
+        if (widget.onWillAccept(widget.eventData.dragTargetData as T)) {
+          Log.trace("[$FakeDragTarget] on drag start");
+          simulateDragging = true;
+          widget.deleteAnimationController.reverse(from: 1.0);
+          widget.onDragStarted(
+            widget.child,
+            widget.eventData.index,
+            widget.eventData.feedbackSize,
+          );
+        } else {
+          Log.trace("[$FakeDragTarget] cancel start drag");
+        }
       });
     });
   }

+ 4 - 3
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_flex/drag_target_interceptor.dart

@@ -35,7 +35,7 @@ abstract class DragTargetInterceptor {
 
 abstract class OverlapDragTargetDelegate {
   void cancel();
-  void moveTo(
+  void dragTargetDidMoveToReorderFlex(
     String reorderFlexId,
     FlexDragTargetData dragTargetData,
     int dragTargetIndex,
@@ -99,7 +99,8 @@ class OverlappingDragTargetInterceptor extends DragTargetInterceptor {
         if (index != -1) {
           Log.trace(
               '[$OverlappingDragTargetInterceptor] move to $dragTargetId at $index');
-          delegate.moveTo(dragTargetId, dragTargetData, index);
+          delegate.dragTargetDidMoveToReorderFlex(
+              dragTargetId, dragTargetData, index);
 
           columnsState
               .getReorderFlexState(groupId: dragTargetId)
@@ -153,7 +154,7 @@ class CrossReorderFlexDragTargetInterceptor extends DragTargetInterceptor {
       /// it means the dragTarget is dragging on the top of its own list.
       /// Otherwise, it means the dargTarget was moved to another list.
       Log.trace(
-          "[$CrossReorderFlexDragTargetInterceptor] $reorderFlexId accept ${dragTargetData.reorderFlexId} ${reorderFlexId != dragTargetData.reorderFlexId}");
+          "[$CrossReorderFlexDragTargetInterceptor] $reorderFlexId should accept ${dragTargetData.reorderFlexId} : ${reorderFlexId != dragTargetData.reorderFlexId}");
       return reorderFlexId != dragTargetData.reorderFlexId;
     } else {
       Log.trace(

+ 36 - 32
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_flex/reorder_flex.dart

@@ -38,20 +38,13 @@ abstract class ReorderDragTargetIndexKeyStorage {
 
 class ReorderFlexConfig {
   /// The opacity of the dragging widget
-  final double draggingWidgetOpacity = 0.3;
+  final double draggingWidgetOpacity = 0.4;
 
   // How long an animation to reorder an element
-  final Duration reorderAnimationDuration = const Duration(milliseconds: 300);
+  final Duration reorderAnimationDuration = const Duration(milliseconds: 200);
 
   // How long an animation to scroll to an off-screen element
-  final Duration scrollAnimationDuration = const Duration(milliseconds: 300);
-
-  /// Determines if setSatte method needs to be called when the drag is complete.
-  /// Default value is [true].
-  ///
-  /// If the [ReorderFlex] needs to be rebuild after the [ReorderFlex] end dragging,
-  /// the [setStateWhenEndDrag] should set to [true].
-  final bool setStateWhenEndDrag;
+  final Duration scrollAnimationDuration = const Duration(milliseconds: 200);
 
   final bool useMoveAnimation;
 
@@ -59,7 +52,6 @@ class ReorderFlexConfig {
 
   const ReorderFlexConfig({
     this.useMoveAnimation = true,
-    this.setStateWhenEndDrag = true,
   }) : useMovePlaceholder = !useMoveAnimation;
 }
 
@@ -149,6 +141,7 @@ class ReorderFlexState extends State<ReorderFlex>
       reorderAnimationDuration: widget.config.reorderAnimationDuration,
       entranceAnimateStatusChanged: (status) {
         if (status == AnimationStatus.completed) {
+          if (dragState.nextIndex == -1) return;
           setState(() => _requestAnimationToNextIndex());
         }
       },
@@ -377,12 +370,18 @@ class ReorderFlexState extends State<ReorderFlex>
         dragTargetData.dragTargetOffset = offset;
       },
       onDragEnded: (dragTargetData) {
-        if (!mounted) return;
+        if (!mounted) {
+          Log.warn(
+              "[DragTarget]: Group:[${widget.dataSource.identifier}] end dragging but current widget was unmounted");
+          return;
+        }
         Log.debug(
             "[DragTarget]: Group:[${widget.dataSource.identifier}] end dragging");
+
         _notifier.updateDragTargetIndex(-1);
+        _animation.insertController.stop();
 
-        onDragEnded() {
+        setState(() {
           if (dragTargetData.reorderFlexId == widget.reorderFlexId) {
             _onReordered(
               dragState.dragStartIndex,
@@ -391,13 +390,7 @@ class ReorderFlexState extends State<ReorderFlex>
           }
           dragState.endDragging();
           widget.onDragEnded?.call();
-        }
-
-        if (widget.config.setStateWhenEndDrag) {
-          setState(() => onDragEnded());
-        } else {
-          onDragEnded();
-        }
+        });
       },
       onWillAccept: (FlexDragTargetData dragTargetData) {
         // Do not receive any events if the Insert item is animating.
@@ -405,19 +398,23 @@ class ReorderFlexState extends State<ReorderFlex>
           return false;
         }
 
-        assert(widget.dataSource.items.length > dragTargetIndex);
-        if (_interceptDragTarget(dragTargetData, (interceptor) {
-          interceptor.onWillAccept(
-            context: builderContext,
-            reorderFlexState: this,
-            dragTargetData: dragTargetData,
-            dragTargetId: reorderFlexItem.id,
-            dragTargetIndex: dragTargetIndex,
-          );
-        })) {
-          return true;
+        if (dragTargetData.isDragging) {
+          assert(widget.dataSource.items.length > dragTargetIndex);
+          if (_interceptDragTarget(dragTargetData, (interceptor) {
+            interceptor.onWillAccept(
+              context: builderContext,
+              reorderFlexState: this,
+              dragTargetData: dragTargetData,
+              dragTargetId: reorderFlexItem.id,
+              dragTargetIndex: dragTargetIndex,
+            );
+          })) {
+            return true;
+          } else {
+            return handleOnWillAccept(builderContext, dragTargetIndex);
+          }
         } else {
-          return handleOnWillAccept(builderContext, dragTargetIndex);
+          return false;
         }
       },
       onAccept: (dragTargetData) {
@@ -485,6 +482,10 @@ class ReorderFlexState extends State<ReorderFlex>
   }
 
   void resetDragTargetIndex(int dragTargetIndex) {
+    if (dragTargetIndex > widget.dataSource.items.length) {
+      return;
+    }
+
     dragState.setStartDraggingIndex(dragTargetIndex);
     widget.dragStateStorage?.write(
       widget.reorderFlexId,
@@ -521,6 +522,9 @@ class ReorderFlexState extends State<ReorderFlex>
   }
 
   void _onReordered(int fromIndex, int toIndex) {
+    if (toIndex == -1) return;
+    if (fromIndex == -1) return;
+
     if (fromIndex != toIndex) {
       widget.onReorder.call(fromIndex, toIndex);
     }

+ 3 - 2
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_phantom/phantom_controller.dart

@@ -94,7 +94,8 @@ class BoardPhantomController extends OverlapDragTargetDelegate
 
   /// Remove the phantom in the group if it contains phantom
   void _removePhantom(String groupId) {
-    if (delegate.removePhantom(groupId)) {
+    final didRemove = delegate.removePhantom(groupId);
+    if (didRemove) {
       phantomState.notifyDidRemovePhantom(groupId);
       phantomState.removeGroupListener(groupId);
     }
@@ -195,7 +196,7 @@ class BoardPhantomController extends OverlapDragTargetDelegate
   }
 
   @override
-  void moveTo(
+  void dragTargetDidMoveToReorderFlex(
     String reorderFlexId,
     FlexDragTargetData dragTargetData,
     int dragTargetIndex,

+ 24 - 20
frontend/app_flowy/packages/appflowy_board/lib/src/widgets/reorder_phantom/phantom_state.dart

@@ -1,50 +1,54 @@
+import 'package:appflowy_board/src/utils/log.dart';
+
 import 'phantom_controller.dart';
 import 'package:flutter/material.dart';
 
 class GroupPhantomState {
-  final _states = <String, GroupState>{};
+  final _groupStates = <String, GroupState>{};
+  final _groupIsDragging = <String, bool>{};
 
   void setGroupIsDragging(String groupId, bool isDragging) {
-    _stateWithId(groupId).isDragging = isDragging;
+    _groupIsDragging[groupId] = isDragging;
   }
 
   bool isDragging(String groupId) {
-    return _stateWithId(groupId).isDragging;
+    return _groupIsDragging[groupId] ?? false;
   }
 
   void addGroupListener(String groupId, PassthroughPhantomListener listener) {
-    _stateWithId(groupId).notifier.addListener(
-          onInserted: (index) => listener.onInserted?.call(index),
-          onDeleted: () => listener.onDragEnded?.call(),
-        );
+    if (_groupStates[groupId] == null) {
+      Log.debug("[$GroupPhantomState] add group listener: $groupId");
+      _groupStates[groupId] = GroupState();
+      _groupStates[groupId]?.notifier.addListener(
+            onInserted: (index) => listener.onInserted?.call(index),
+            onDeleted: () => listener.onDragEnded?.call(),
+          );
+    }
   }
 
   void removeGroupListener(String groupId) {
-    _stateWithId(groupId).notifier.dispose();
-    _states.remove(groupId);
+    Log.debug("[$GroupPhantomState] remove group listener: $groupId");
+    final groupState = _groupStates.remove(groupId);
+    groupState?.dispose();
   }
 
   void notifyDidInsertPhantom(String groupId, int index) {
-    _stateWithId(groupId).notifier.insert(index);
+    _groupStates[groupId]?.notifier.insert(index);
   }
 
   void notifyDidRemovePhantom(String groupId) {
-    _stateWithId(groupId).notifier.remove();
-  }
-
-  GroupState _stateWithId(String groupId) {
-    var state = _states[groupId];
-    if (state == null) {
-      state = GroupState();
-      _states[groupId] = state;
-    }
-    return state;
+    Log.debug("[$GroupPhantomState] $groupId remove phantom");
+    _groupStates[groupId]?.notifier.remove();
   }
 }
 
 class GroupState {
   bool isDragging = false;
   final notifier = PassthroughPhantomNotifier();
+
+  void dispose() {
+    notifier.dispose();
+  }
 }
 
 abstract class PassthroughPhantomListener {