Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dragdrop): don't fire Drop if AcceptedOperation is None #18808

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private void OnHostDragLeave(object sender, DragEventArgs e)
=> e.Effects = ToDropEffects(_manager.ProcessAborted(_fakePointerId));

private void OnHostDrop(object sender, DragEventArgs e)
=> e.Effects = ToDropEffects(_manager.ProcessDropped(new DragEventSource(_fakePointerId, e)));
=> e.Effects = ToDropEffects(_manager.ProcessReleased(new DragEventSource(_fakePointerId, e)));

public void StartNativeDrag(CoreDragInfo info)
{
Expand Down Expand Up @@ -244,7 +244,7 @@ private static async Task<DataObject> ToDataObject(DataPackageView src, Cancella
return dst;
}

private class DragEventSource : IDragEventSource
private readonly struct DragEventSource : IDragEventSource
ramezgerges marked this conversation as resolved.
Show resolved Hide resolved
{
private readonly DragEventArgs _wpfArgs;
private static long _nextFrameId;
Expand Down
4 changes: 2 additions & 2 deletions src/Uno.UI.Runtime.Skia.X11/X11DragDropExtension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ private void ProcessXdndLeave(XClientMessageEvent ev)
private void ProcessXdndDrop(XClientMessageEvent ev)
{
var pos = _currentSession!.Value.LastPosition!.Value;
var acceptedOperation = _manager.ProcessDropped(new DragEventSource((int)pos.X, (int)pos.Y));
var acceptedOperation = _manager.ProcessReleased(new DragEventSource((int)pos.X, (int)pos.Y));
_currentSession = null;

if (this.Log().IsEnabled(LogLevel.Trace))
Expand Down Expand Up @@ -297,7 +297,7 @@ private void ProcessXdndDrop(XClientMessageEvent ev)
// TODO: uno-to-outside dragging
public void StartNativeDrag(CoreDragInfo info) => throw new System.NotImplementedException();

private class DragEventSource(int x, int y) : IDragEventSource
private readonly struct DragEventSource(int x, int y) : IDragEventSource
{
private static long _nextFrameId;
private readonly Point _location = new Point(x, y);
Expand Down
61 changes: 61 additions & 0 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_UIElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1682,6 +1682,67 @@ public async Task When_ScaleTransform_HitTest(bool addClip)
#if HAS_UNO
#region Drag and Drop

[TestMethod]
[RunsOnUIThread]
[UnoWorkItem("https://github.com/unoplatform/uno/issues/18770")]
[DataRow(true)]
[DataRow(false)]
#if !HAS_INPUT_INJECTOR
[Ignore("InputInjector is not supported on this platform.")]
#endif
public async Task When_DragDrop_AcceptedOperation_None(bool setAcceptedOperation)
{
if (TestServices.WindowHelper.IsXamlIsland)
{
Assert.Inconclusive("Drag and drop doesn't work in Uno islands.");
}

var injector = InputInjector.TryCreate() ?? throw new InvalidOperationException("Failed to init the InputInjector");
using var mouse = injector.GetMouse();

var dropCount = 0;
Rectangle source, target;
var sp = new StackPanel
{
(source = new Rectangle
{
Width = 100,
Height = 100,
Fill = new SolidColorBrush(Microsoft.UI.Colors.LightCoral),
CanDrag = true
}),
(target = new Rectangle
{
Width = 100,
Height = 100,
Fill = new SolidColorBrush(Microsoft.UI.Colors.LightCoral),
AllowDrop = true
})
};

target.Drop += (_, _) => dropCount++;
if (setAcceptedOperation)
{
target.DragOver += (_, e) => e.AcceptedOperation = DataPackageOperation.Copy;
}

await UITestHelper.Load(sp);

mouse.MoveTo(source.GetAbsoluteBoundsRect().GetCenter());
await TestServices.WindowHelper.WaitForIdle();
mouse.Press();
await TestServices.WindowHelper.WaitForIdle();
for (int i = 1; i <= 10; i++)
{
mouse.MoveBy(0, (target.GetAbsoluteBoundsRect().GetMidY() - source.GetAbsoluteBoundsRect().GetMidY()) * 0.1);
await TestServices.WindowHelper.WaitForIdle();
}
mouse.Release();
await TestServices.WindowHelper.WaitForIdle();

Assert.AreEqual(setAcceptedOperation ? 1 : 0, dropCount);
}

[TestMethod]
[RunsOnUIThread]
[DataRow(true)]
Expand Down
4 changes: 2 additions & 2 deletions src/Uno.UI/UI/Xaml/DragDrop/DragDropExtension.macOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private void OnDraggingExited(NSDraggingInfo draggingInfo)

private bool OnPerformDragOperation(NSDraggingInfo draggingInfo)
{
var operation = _manager.ProcessDropped(new DragEventSource(_fakePointerId, draggingInfo, _window));
var operation = _manager.ProcessReleased(new DragEventSource(_fakePointerId, draggingInfo, _window));
return (operation != DataPackageOperation.None);
}

Expand All @@ -88,7 +88,7 @@ public void StartNativeDrag(CoreDragInfo info)
(NSImage image, CGPoint screenPoint, NSDragOperation operation) =>
{
// The drop was completed externally
_manager.ProcessDropped(new DragEventSource(info.SourceId, _window));
_manager.ProcessReleased(new DragEventSource(info.SourceId, _window));
return;
};

Expand Down
6 changes: 3 additions & 3 deletions src/Uno.UI/UI/Xaml/DragDrop/DragDropManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ public DataPackageOperation ProcessMoved(IDragEventSource src)
/// The last accepted operation.
/// Be aware that due to the async processing of dragging in UWP, this might not be the up to date.
/// </returns>
public DataPackageOperation ProcessDropped(IDragEventSource src)
=> FindOperation(src)?.Dropped(src) ?? DataPackageOperation.None;
public DataPackageOperation ProcessReleased(IDragEventSource src)
=> FindOperation(src)?.Released(src) ?? DataPackageOperation.None;

/// <summary>
/// This method is expected to be invoked when pointer involved in a drag operation
Expand Down Expand Up @@ -135,7 +135,7 @@ private void RegisterWindowHandlers()

private void OnPointerMoved(object snd, PointerRoutedEventArgs e) => ProcessMoved(e);

private void OnPointerReleased(object snd, PointerRoutedEventArgs e) => ProcessDropped(e);
private void OnPointerReleased(object snd, PointerRoutedEventArgs e) => ProcessReleased(e);

private void OnPointerCanceled(object snd, PointerRoutedEventArgs e) => ProcessAborted(e.Pointer.PointerId);
}
Expand Down
26 changes: 16 additions & 10 deletions src/Uno.UI/UI/Xaml/DragDrop/DragOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,20 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Uno.UI.Xaml.Core;
using Windows.ApplicationModel.DataTransfer;
using Windows.ApplicationModel.DataTransfer.DragDrop;
using Windows.ApplicationModel.DataTransfer.DragDrop.Core;
using Windows.System;
using Windows.UI.Core;
using Windows.UI.Input;
using Microsoft.UI.Xaml.Input;
using Uno.UI.Dispatching;

namespace Microsoft.UI.Xaml
{
/// <summary>
/// The state machine of a dragging operation from the initiation of a dragging event
/// when a pointer holds and moves to the completion of the event when the pointer
/// is released
/// </summary>
internal class DragOperation
{
private static readonly TimeSpan _deferralTimeout = TimeSpan.FromSeconds(30); // Random value!
Expand All @@ -35,7 +33,7 @@ internal class DragOperation
private State _state = State.None;
private uint _lastFrameId;
private bool _hasRequestedNativeDrag;
private DataPackageOperation _acceptedOperation;
private DataPackageOperation _acceptedOperation = DataPackageOperation.Copy | DataPackageOperation.Move | DataPackageOperation.Link;

private enum State
{
Expand Down Expand Up @@ -98,7 +96,7 @@ internal DataPackageOperation Aborted()
return _acceptedOperation;
}

internal DataPackageOperation Dropped(IDragEventSource src)
internal DataPackageOperation Released(IDragEventSource src)
{
// For safety, we don't validate the FrameId for the finalizing actions, we rely only on the _state
if (_state >= State.Completing)
Expand All @@ -107,7 +105,15 @@ internal DataPackageOperation Dropped(IDragEventSource src)
}

Update(src);
Enqueue(RaiseDrop);

if (_acceptedOperation is DataPackageOperation.None)
{
Abort();
}
else
{
Enqueue(RaiseDrop);
}

return _acceptedOperation;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Uno.UI/UI/Xaml/UIElement.Pointers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1100,7 +1100,7 @@ internal bool OnPointerUp(PointerRoutedEventArgs args, BubblingContext ctx = def
GestureRecognizer.ProcessUpEvent(currentPoint, isOverOrCaptured && !ctx.IsCleanup);
if (isDragging)
{
XamlRoot.VisualTree.ContentRoot.InputManager.DragDrop.ProcessDropped(args);
XamlRoot.VisualTree.ContentRoot.InputManager.DragDrop.ProcessReleased(args);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ internal DataPackageOperation ProcessMoved(IDragEventSource src)
/// Be aware that due to the async processing of dragging in UWP, this might not be the up to date.
/// </returns>
internal DataPackageOperation ProcessDropped(IDragEventSource src)
=> _manager?.ProcessDropped(src) ?? DataPackageOperation.None;
=> _manager?.ProcessReleased(src) ?? DataPackageOperation.None;

/// <summary>
/// This method is expected to be invoked when pointer involved in a drag operation
Expand Down Expand Up @@ -169,7 +169,7 @@ internal interface IDragDropManager
/// The last accepted operation.
/// Be aware that due to the async processing of dragging in UWP, this might not be the up to date.
/// </returns>
DataPackageOperation ProcessDropped(IDragEventSource src);
DataPackageOperation ProcessReleased(IDragEventSource src);

/// <summary>
/// This method is expected to be invoked when pointer involved in a drag operation
Expand Down
Loading