-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[RC1] Implement Platform Args for Drag and Drop EventArgs #16962
[RC1] Implement Platform Args for Drag and Drop EventArgs #16962
Conversation
…ns-Final' into GestureRecognizer-DragNDrop-Extensions-Final
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite a few changes here, so I just wanted to drop a review so that I don't go down a path that was already discussed. My main issues are:
- the apparent "not super required" platform args in the ctors of the event args. Should we add those to banned apis so we don't use it
- the total change of the way
Handled
is now working. This is potentially more breaking and also I am thinking it is still useful on other platforms... maybe?
src/Controls/src/Core/DragAndDrop/PlatformDragStartingEventArgs.cs
Outdated
Show resolved
Hide resolved
/// <summary> | ||
/// Gets the native view attached to the event. | ||
/// </summary> | ||
public object Sender { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be Microsoft.UI.Xaml.UIElement
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the calling methods used just have their sender type as object as such:
void HandleDrop(object sender, Microsoft.UI.Xaml.DragEventArgs e)
{ ... }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could type cast and check for Microsoft.UI.Xaml.UIElement, but I was not sure if we expect that every time or if there was a reason for the 'object'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sender should be the object you are attaching to, so if they are all UIElement, maybe we can keep it consistent
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, this probably should be UIElement - even though it is object. The events are only available on UIElement so should never be anything other than element.
src/Controls/src/Core/Platform/Android/DragAndDropGestureHandler.cs
Outdated
Show resolved
Hide resolved
src/Controls/src/Core/Platform/Android/DragAndDropGestureHandler.cs
Outdated
Show resolved
Hide resolved
…ns-Final' into GestureRecognizer-DragNDrop-Extensions-Final
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few last comments, but looking good. It is more defensive decision making for the func... Not sure if it will cause issues with memory leaks or something. Should be fine, but we can always add an API in RC2 but we can't really take away.
/// <summary> | ||
/// Gets the native view attached to the event. | ||
/// </summary> | ||
public object Sender { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, this probably should be UIElement - even though it is object. The events are only available on UIElement so should never be anything other than element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
Description of Change
This PR exposes platform-specific properties from Drag and Drop Maui Gestures to our customers allowing more customization of Drag and Drop interactions. There are five different events related to drag and drop gestures that use four different EventArg classes that will now contain the following classes:
PlatformDragStartingEventArgs
iOS / Catalyst
Android
Windows
PlatformDropCompletedEventArgs
iOS / Catalyst
Android
Windows
PlatformDropEventArgs
iOS / Catalyst
Android
Windows
PlatformDragEventArgs
iOS / Catalyst
Android
Windows
Sample
Showing overwriting the drag Shadow on iOS
UITest added
DragNDropiOSUITestDemo.mov
Issues Fixed
Fixes #16992