[ios11-beta1] Initial UIKit Drag and Drop support #2190

migueldeicaza · 2017-06-08T21:45:32Z

No description provided.

monojenkins · 2017-06-09T00:20:19Z

Build failure

migueldeicaza · 2017-06-09T01:27:54Z

The build breaks with some Jenkins script while dowloading some code, dont know what to do here.

rolfbjarne · 2017-06-10T11:06:36Z

build

monojenkins · 2017-06-10T12:18:13Z

Build failure

rolfbjarne · 2017-06-15T11:16:29Z

src/foundation.cs

+	interface INSItemProviderReading {}
+
+	[Watch (4,0), TV (11,0), Mac (10,13), iOS (11,0)]
+	[Protocol, Model]


I think this protocol is supposed to be implemented by existing types that can be dragged/dropped, in which case [Model] doesn't make sense, it can be only [Protocol].

rolfbjarne · 2017-06-15T11:17:55Z

src/foundation.cs

+
+		[Abstract]
+		[Export ("initWithItemProviderData:typeIdentifier:error:")]
+		NSObject CreateFrom (NSData providerData, string typeIdentifier, [NullAllowed] out NSError outError);


This is an Objective-C constructor, so I don't think binding it as an instance method will work.

You are right, what should we do in this case?

On the one hand, we should flag the classes that implement this, and manually add the constructor there (NSAttributedString, UIColor, UIImage). The other part is more complicated, given a class that implements this interface, probably our best bet is to have a factory method as an extension class that does the work:

static class NSItemProviderExtensions { public INSItemProviderReading CreateFromData (INSItemProviderReading self, NSData providerData, string typeIdentifier, etc) { // something here. }

Filed a bug for this: https://bugzilla.xamarin.com/show_bug.cgi?id=57650

rolfbjarne · 2017-06-15T11:20:05Z

src/uikit.cs

@@ -15405,6 +15435,24 @@ partial interface UIInputViewController : UITextInputDelegate {
 		void HandleInputModeList (UIView fromView, UIEvent withEvent);
 	}

+	[NoWatch, NoTV, iOS (11,0)]
+	[Protocol, Model]


This protocol is supposed to be implemented by existing types, so [Model] (and [BaseType]) should be dropped.

rolfbjarne · 2017-06-15T11:22:18Z

src/uikit.cs

+	interface IUIDropInteractionDelegate {}
+
+	[NoWatch, NoTV, iOS (11,0)]
+	[Protocol, Model]


I don't think this needs [Model].

rolfbjarne · 2017-06-15T11:28:57Z

src/uikit.cs

+	}
+
+	[NoWatch, NoTV, iOS (11,0)]
+	[Protocol, Model]


I don't think this needs [Model].

Dropped the [Model] from protocols that do not need it, as well as the [BaseType] attribute. Inlined the constructor for NSItemProviderReading in UIColor, NSAttributedString and UIImage as we do not have support for making interfaces require a constructor and filed a bug with the remaining work that would have to be done to support something like this: https://bugzilla.xamarin.com/show_bug.cgi?id=57650

monojenkins · 2017-06-21T04:55:48Z

Build failure

timrisi · 2017-06-22T00:53:29Z

src/foundation.cs

+		[Static, Abstract]
+		[Export ("readableTypeIdentifiersForItemProvider", ArgumentSemantic.Copy)]
+		string[] ReadableTypeIdentifiersForItemProvider { get; }
+


Does this actually work? From what I can tell trying to bind the Static property inside the interface doesn't seem to actually generate the code for it.

timrisi · 2017-06-22T01:05:21Z

src/foundation.cs

+		//
+		//[Abstract]
+		//[Export ("initWithItemProviderData:typeIdentifier:error:")]
+		//INSItemProviderReading CreateFrom (NSData providerData, string typeIdentifier, [NullAllowed] NSError outError);


Why is the method being bound separately in the different classes instead of having the classes implement INSItemProviderReading?

See bug: https://bugzilla.xamarin.com/show_bug.cgi?id=57650 for details.

The runtime needs some changes to support the idiom.

As I've stated on the bug I believe the runtime doesn't need changes to make it work, so this can be uncommented (and the CreateFrom method should be a proper Constructor, not a method).

The registrars can provide some help for users, but it needs these methods properly defined in order to have enough information to help users.

What this patch does is to inline the constructor in all three places that conform to this procotol.

I tried doing what you suggested (a) removing the above line, and (b) removing the inlined constructors that this patch contains everywhere and (c) making those classes adopt the NSItemProviderReading interface, but that does not inline the constructor anywhere, nor does the interface gain anything out of having this method declared here.

The result is thus a step backwards, the patch to that is here:

https://gist.github.com/migueldeicaza/e22f44007e7ac093a0076276187dcb16

So I am not sure what is that you are suggesting I do with this.

I meant only uncommenting the code here, inside NSItemProviderReading, not removing the inlined members elsewhere.

In any case I can fix this when I look at the corresponding bug.

…ses the static NSItemProviderReading class method, and tracking the documentation requirements here: https://bugzilla.xamarin.com/show_bug.cgi?id=57735

monojenkins · 2017-06-24T04:05:04Z

Build failure

migueldeicaza · 2017-06-24T14:29:03Z

I am at a loss here, the build failure is an assertion that does not quite make sense:

3 errors found in 1613 static selector validated:
Foundation.NSAttributedString : readableTypeIdentifiersForItemProvider
UIKit.UIColor : readableTypeIdentifiersForItemProvider
UIKit.UIImage : readableTypeIdentifiersForItemProvider


  Expected: 0
  But was:  3

STACK TRACE:
at Introspection.ApiSelectorTest.StaticMethods () [0x001a4] in /Users/builder/jenkins/workspace/xamarin-macios-pr-builder/tests/introspection/ApiSelectorTest.cs:625 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, System.Reflection.BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00032] in /Library/Frameworks/Xamarin.iOS.framework/Versions/10.99.0.44/src/mono/mcs/class/corlib/System.Reflection/MonoMethod.cs:305

But these methods are there and respond to that selector:

migueldeicaza · 2017-06-24T14:39:05Z

Sebastien replied: when I inlined these methods, I did not inline the availability attributes.

monojenkins · 2017-06-24T15:53:28Z

Build failure

monojenkins · 2017-06-25T02:50:50Z

Build failure

…vents/properties - hope Apple fixes

monojenkins · 2017-06-26T03:23:19Z

Build failure

rolfbjarne · 2017-06-26T20:09:30Z

build

monojenkins · 2017-06-26T21:24:05Z

Build failure

…re not actually implemented on TV or Watch

monojenkins · 2017-06-27T02:47:09Z

Build failure

monojenkins · 2017-06-28T02:57:15Z

Build failure

dalexsoto · 2017-06-28T03:14:13Z

Unrelated test failure: [FAIL] MessageHandlerTest.DnsFailure(System.Net.Http.NSUrlSessionHandler) : Exception -> https://bugzilla.xamarin.com/show_bug.cgi?id=57762

dalexsoto

One tiny bit from my end :)

dalexsoto · 2017-06-28T03:23:07Z

src/uikit.cs

+	[NoWatch, NoTV, iOS (11,0)]
+	[BaseType (typeof(NSObject))] // If Apple adds a delegate setter: Delegates=new string [] {"Delegate"}, Events=new Type [] { typeof (UIDropInteractionDelegate)})]
+	[DisableDefaultCtor]
+	interface UIDropInteraction : IUIInteraction


Here you want to use UIInteraction interaction instead or the generator will pick the wrong interface

interface UIDropInteraction : UIInteraction

I can do that, but can you explain the reason, I have forgotten :-)

rolfbjarne · 2017-06-28T05:28:45Z

The remaining test failure is not related to this PR, so this is good to go once it's approved by @timrisi / @dalexsoto (I see Alex just added a new comment, so that should be addressed too).

VincentDondain · 2017-06-28T21:17:22Z

src/uikit.cs

+
+		[Abstract]
+		[Export ("canLoadObjectsOfClass:")]
+		bool CanLoadObjectsOfClass (INSItemProviderReading aClass);


CanLoadObjectsOfClass > CanLoadObjects

I do not think this is a good idea.

Let's check what Swift is using, they normally use API name very close to the .net we have been promoting for years (and it also makes ease googl'ability)

See https://bugzilla.xamarin.com/show_bug.cgi?id=58079#c1

Swift code:

session.loadObjects(ofClass: UIImage.self) { imageItems in let images = imageItems as! [UIImage] /* If you do not employ the loadObjects(ofClass:completion:) convenience method of the UIDropSession class, which automatically employs the main thread, explicitly dispatch UI work to the main thread. For example, you can use `DispatchQueue.main.async` method. */ self.imageView.image = images.first }

VincentDondain · 2017-06-28T21:19:41Z

src/uikit.cs

+
+		[Abstract]
+		[Export ("hasItemsConformingToTypeIdentifiers:")]
+		bool HasItemsConformingToTypeIdentifiers (string[] typeIdentifiers);


Not sure it's the best C# name we can come up with, maybe HasConformingItems?

VincentDondain · 2017-06-28T21:29:01Z

src/uikit.cs

+		CGSize Size { get; }
+
+		[Export ("retargetedPreviewWithTarget:")]
+		UITargetedDragPreview RetargetedPreviewWithTarget (UIDragPreviewTarget newTarget);


RetargetedPreviewWithTarget > RetargetedPreview

Went with GetRetargetedPreview

monojenkins · 2017-06-28T21:55:13Z

Build failure

monojenkins · 2017-07-01T02:50:11Z

Build failure

Addressed in the comments.

praeclarum · 2017-07-11T15:49:13Z

I notice that UIDragInteraction is missing from this. Is that intentional?

rolfbjarne · 2017-07-11T15:51:27Z

@praeclarum yeah, we know it's not complete. We have tests that flag everything we haven't implemented.

[ios11-beta1] Initial UIKit Drag and Drop support

30d911a

dnfclas added the cla-already-signed label Jun 8, 2017

rolfbjarne requested changes Jun 15, 2017

View reviewed changes

rolfbjarne mentioned this pull request Jun 21, 2017

[Foundation] Update to xcode9 beta 1 #2235

Closed

timrisi previously requested changes Jun 22, 2017

View reviewed changes

Updated with Feedback from Tim Risi, inlined in the implementing clas…

4849fa7

…ses the static NSItemProviderReading class method, and tracking the documentation requirements here: https://bugzilla.xamarin.com/show_bug.cgi?id=57735

NSItemProviderReading inlined methods get availability attributes

3c56df8

UIDropInteraction only has a getter Delegate method

14d9105

UIDropInteraction - without the delegate setter, we can not provide e…

fcb81d9

…vents/properties - hope Apple fixes

Ignore what the header files say, the NSItemProviderReading methods a…

8a44db7

…re not actually implemented on TV or Watch

Add 'Editability' as an acceptable word

0643d17

dalexsoto requested changes Jun 28, 2017

View reviewed changes

rolfbjarne approved these changes Jun 28, 2017

View reviewed changes

Address Alex Soto's feedback

6c87a94

VincentDondain requested changes Jun 28, 2017

View reviewed changes

dalexsoto approved these changes Jun 28, 2017

View reviewed changes

migueldeicaza added 2 commits June 30, 2017 21:35

Rename some methods, fix types

897f203

Add Emacs backup files to .gitignore

631e676

VincentDondain approved these changes Jul 1, 2017

View reviewed changes

rolfbjarne approved these changes Jul 10, 2017

View reviewed changes

rolfbjarne merged commit 35462f0 into xamarin:xcode9 Jul 10, 2017

[ios11-beta1] Initial UIKit Drag and Drop support #2190

[ios11-beta1] Initial UIKit Drag and Drop support #2190

Conversation

migueldeicaza commented Jun 8, 2017

monojenkins commented Jun 9, 2017

migueldeicaza commented Jun 9, 2017

rolfbjarne commented Jun 10, 2017

monojenkins commented Jun 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Jun 21, 2017

Choose a reason for hiding this comment

timrisi Jun 22, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Jun 24, 2017

migueldeicaza commented Jun 24, 2017

migueldeicaza commented Jun 24, 2017

monojenkins commented Jun 24, 2017

monojenkins commented Jun 25, 2017

monojenkins commented Jun 26, 2017

rolfbjarne commented Jun 26, 2017

monojenkins commented Jun 26, 2017

monojenkins commented Jun 27, 2017

monojenkins commented Jun 28, 2017

dalexsoto commented Jun 28, 2017

dalexsoto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rolfbjarne commented Jun 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monojenkins commented Jun 28, 2017

monojenkins commented Jul 1, 2017

praeclarum commented Jul 11, 2017

rolfbjarne commented Jul 11, 2017

timrisi Jun 22, 2017 •

edited