Skip to content

Commit

Permalink
fix(dragdrop): Raise the DragLeave event properly and fix pointer tra…
Browse files Browse the repository at this point in the history
…cking captured
  • Loading branch information
dr1rrb committed Oct 16, 2020
1 parent 3d81579 commit 19894da
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 45 deletions.
13 changes: 10 additions & 3 deletions src/Uno.UI/UI/Xaml/DragOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ internal class DragOperation

private bool _isRunning;
private State _state = State.None;
private uint _lastFrameId;

private enum State
{
Expand Down Expand Up @@ -59,7 +60,7 @@ internal void Moved(PointerRoutedEventArgs args)

private void EnteredOrMoved(PointerRoutedEventArgs args)
{
if (_state >= State.Completing)
if (_state >= State.Completing || args.FrameId <= _lastFrameId)
{
return;
}
Expand Down Expand Up @@ -88,7 +89,7 @@ async Task Over(CancellationToken ct)

internal void Exited(PointerRoutedEventArgs args)
{
if (_state >= State.Completing)
if (_state >= State.Completing || args.FrameId <= _lastFrameId)
{
return;
}
Expand All @@ -115,7 +116,8 @@ async Task Leave(CancellationToken ct)

internal void Dropped(PointerRoutedEventArgs args)
{
if (_state >= State.Completing)
// For safety, we don't validate the FrameId for the finalizing actions, we rely only on the _state
if (_state >= State.Completing)
{
return;
}
Expand Down Expand Up @@ -146,6 +148,7 @@ async Task Drop(CancellationToken ct)

internal void Aborted(PointerRoutedEventArgs args)
{
// For safety, we don't validate the FrameId for the finalizing actions, we rely only on the _state
if (_state >= State.Completing)
{
return;
Expand Down Expand Up @@ -252,6 +255,10 @@ private void Update(PointerRoutedEventArgs args)

// Updates the view location to follow the pointer
_view.SetLocation(point.Position);

// As we have multiple sources for the pointer events (capture of the dragged element and DragRoot),
// we make sure to not process the same event twice.
_lastFrameId = args.FrameId;
}

private void Enqueue(Func<CancellationToken, Task> action, bool isIgnorable = false)
Expand Down
59 changes: 38 additions & 21 deletions src/Uno.UI/UI/Xaml/DropUITarget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,14 @@ internal class DropUITarget : ICoreDropOperationTarget
// Note: As drag events are routed (so they may be received by multiple elements), we might not have an entry for each drop targets.
// We will instead have entry only for leaf (a.k.a. OriginalSource).
// This is valid as UWP does clear the UIOverride as soon as a DragLeave is raised.
private readonly Dictionary<UIElement, (DraggingState state, DragUIOverride uiOverride, DataPackageOperation acceptedOperation)> _pendingDropTargets
= new Dictionary<UIElement, (DraggingState state, DragUIOverride uiOverride, DataPackageOperation acceptedOperation)>();
private readonly Dictionary<UIElement, (DragUIOverride uiOverride, DataPackageOperation acceptedOperation)> _pendingDropTargets
= new Dictionary<UIElement, (DragUIOverride uiOverride, DataPackageOperation acceptedOperation)>();

private readonly Window _window;
private readonly Predicate<UIElement> _isDropOver;

public DropUITarget(Window window)
{
_window = window;
_isDropOver = elt => _pendingDropTargets.ContainsKey(elt);
}

/// <inheritdoc />
Expand Down Expand Up @@ -78,8 +76,7 @@ private IAsyncOperation<DataPackageOperation> EnterOrOverAsync(CoreDragInfo drag
await deferral.Completed(ct);
}

// TODO
// UpdateState()
UpdateState(args);

return args.AcceptedOperation;
});
Expand All @@ -98,7 +95,7 @@ public IAsyncAction LeaveAsync(CoreDragInfo dragInfo)
_pendingDropTargets.Clear();
Task.WhenAll(leaveTasks);

async Task RaiseLeave(KeyValuePair<UIElement, (DraggingState state, DragUIOverride uiOverride, DataPackageOperation acceptedOperation)> target)
async Task RaiseLeave(KeyValuePair<UIElement, (DragUIOverride uiOverride, DataPackageOperation acceptedOperation)> target)
{
var args = new DragEventArgs(target.Key, dragInfo, target.Value.uiOverride, ptArgs);

Expand Down Expand Up @@ -144,7 +141,10 @@ public IAsyncOperation<DataPackageOperation> DropAsync(CoreDragInfo dragInfo)
PointerRoutedEventArgs ptArgs,
CancellationToken ct)
{
var target = VisualTreeHelper.HitTest(dragInfo.Position, getTestability: _getDropHitTestability, isStale: _isDropOver);
var target = VisualTreeHelper.HitTest(
dragInfo.Position,
getTestability: _getDropHitTestability,
isStale: elt => elt.IsDragOver(ptArgs.Pointer));

// First raise the drag leave event on stale branch if any.
if (target.stale is { } staleBranch)
Expand All @@ -156,17 +156,21 @@ public IAsyncOperation<DataPackageOperation> DropAsync(CoreDragInfo dragInfo)
.ToArray();

Debug.Assert(leftElements.Length > 0);
// TODO: We should raise the event only from the Leaf to the Root of the branch, not the whole tree like that
// This is acceptable as a MVP as we usually have only one Drop target par app.
// Anyway if we Leave a bit too much, we will Enter again below
var leaf = leftElements.First();
var leaveArgs = new DragEventArgs(leaf.elt, dragInfo, leaf.dragState.uiOverride, ptArgs);

staleBranch.Leaf.RaiseDragLeave(leaveArgs);

if (leaveArgs.Deferral is { } deferral)
if (leftElements.Length > 0)
{
await deferral.Completed(ct);
// TODO: We should raise the event only from the Leaf to the Root of the branch, not the whole tree like that
// This is acceptable as a MVP as we usually have only one Drop target par app.
// Anyway if we Leave a bit too much, we will Enter again below
var leaf = leftElements.First();
var leaveArgs = new DragEventArgs(leaf.elt, dragInfo, leaf.dragState.uiOverride, ptArgs);

staleBranch.Leaf.RaiseDragLeave(leaveArgs);

if (leaveArgs.Deferral is { } deferral)
{
await deferral.Completed(ct);
}
}
}

Expand All @@ -175,12 +179,25 @@ public IAsyncOperation<DataPackageOperation> DropAsync(CoreDragInfo dragInfo)
return null;
}

var uiOverride = target.element is {} && _pendingDropTargets.TryGetValue(target.element, out var state)
? state.uiOverride
: new DragUIOverride(dragUIOverride ?? new CoreDragUIOverride());
var args = new DragEventArgs(target.element!, dragInfo, uiOverride, ptArgs);
DragEventArgs args;
if (target.element is {} && _pendingDropTargets.TryGetValue(target.element, out var state))
{
args = new DragEventArgs(target.element!, dragInfo, state.uiOverride, ptArgs)
{
AcceptedOperation = state.acceptedOperation
};
}
else
{
args = new DragEventArgs(target.element!, dragInfo, new DragUIOverride(dragUIOverride ?? new CoreDragUIOverride()), ptArgs);
}

return (target.element!, args);
}

private void UpdateState(DragEventArgs args)
{
_pendingDropTargets[(UIElement)args.OriginalSource] = (args.DragUIOverride, args.AcceptedOperation);
}
}
}
45 changes: 24 additions & 21 deletions src/Uno.UI/UI/Xaml/UIElement.Pointers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,10 @@ internal void RaiseDragLeave(global::Windows.UI.Xaml.DragEventArgs args)

internal void RaiseDrop(global::Windows.UI.Xaml.DragEventArgs args)
{
SafeRaiseEvent(DropEvent, args);
if (_draggingOver?.Remove(args.Pointer) ?? false)
{
SafeRaiseEvent(DropEvent, args);
}
}
#endregion

Expand Down Expand Up @@ -755,10 +758,10 @@ private bool OnNativePointerMoveWithOverCheck(PointerRoutedEventArgs args, bool
if (_gestures.IsValueCreated)
{
_gestures.Value.ProcessMoveEvents(args.GetIntermediatePoints(this), isOverOrCaptured);
//if (_gestures.Value.IsDragging)
//{
// Window.Current.DragDrop.ProcessPointerMovedOverWindow(args);
//}
if (_gestures.Value.IsDragging)
{
Window.Current.DragDrop.ProcessPointerMovedOverWindow(args);
}
}

return handledInManaged;
Expand Down Expand Up @@ -786,10 +789,10 @@ private bool OnPointerMove(PointerRoutedEventArgs args, bool isManagedBubblingEv
// We need to process only events that were not handled by a child control,
// so we should not use them for gesture recognition.
_gestures.Value.ProcessMoveEvents(args.GetIntermediatePoints(this), !isManagedBubblingEvent || isOverOrCaptured);
//if (_gestures.Value.IsDragging)
//{
// Window.Current.DragDrop.ProcessPointerMovedOverWindow(args);
//}
if (_gestures.Value.IsDragging)
{
Window.Current.DragDrop.ProcessPointerMovedOverWindow(args);
}
}

return handledInManaged;
Expand All @@ -815,10 +818,10 @@ private bool OnPointerUp(PointerRoutedEventArgs args, bool isManagedBubblingEven
// so we should not use them for gesture recognition.
var isDragging = _gestures.Value.IsDragging;
_gestures.Value.ProcessUpEvent(args.GetCurrentPoint(this), !isManagedBubblingEvent || isOverOrCaptured);
//if (isDragging)
//{
// Window.Current.DragDrop.ProcessPointerReleased(args);
//}
if (isDragging)
{
Window.Current.DragDrop.ProcessPointerReleased(args);
}
}

// We release the captures on up but only after the released event and processed the gesture
Expand All @@ -842,10 +845,10 @@ private bool OnPointerExited(PointerRoutedEventArgs args, bool isManagedBubbling

handledInManaged |= SetOver(args, false, muteEvent: isManagedBubblingEvent || !isOverOrCaptured);

//if (_gestures.IsValueCreated && _gestures.Value.IsDragging)
//{
// Window.Current.DragDrop.ProcessPointerMovedOverWindow(args);
//}
if (_gestures.IsValueCreated && _gestures.Value.IsDragging)
{
Window.Current.DragDrop.ProcessPointerMovedOverWindow(args);
}

// We release the captures on exit when pointer if not pressed
// Note: for a "Tap" with a finger the sequence is Up / Exited / Lost, so the lost cannot be raised on Up
Expand Down Expand Up @@ -887,10 +890,10 @@ private bool OnPointerCancel(PointerRoutedEventArgs args, bool isManagedBubbling
if (_gestures.IsValueCreated)
{
_gestures.Value.CompleteGesture();
//if (_gestures.Value.IsDragging)
//{
// Window.Current.DragDrop.ProcessPointerCanceled(args);
//}
if (_gestures.Value.IsDragging)
{
Window.Current.DragDrop.ProcessPointerCanceled(args);
}
}

var handledInManaged = false;
Expand Down

0 comments on commit 19894da

Please sign in to comment.