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

Remove InternalsVisibleTo from Core to XF.Platforms.* #782

Merged
merged 7 commits into from Mar 7, 2017

Conversation

Projects
None yet
7 participants
@kingces95
Member

kingces95 commented Feb 23, 2017

Description of Change

We hired contractors to build out new backends. They will not live in the XF repo so internal functionality in Core used by our renderers needs to be exposed publicly for use by their renderers.

Core allows our renderers to access its internal members by applying InternalsVisibleToAttribute (IVT) to itself for each platform assembly. This PR comments out those IVT attributes and fixes the resulting build errors. This is done by either (1) exposing an internal class publicly and changing it's namespace to Xamarin.Forms.Internals or (2) exposing a member publicly and adding a [EditorBrowsable(EditorBrowsableState.Never)] to the member. This essentially creates a new scope between internal and public that is expressed as membership in XF.Internals namespace or the presence of EditorBrowsable on a public member and is only used by renderer implementors to feed state and events back into core. Customers should expect no support of this scope and should not bind to any of it's members.

The EditorBrowsable approach I present as preferable to controller interfaces for hiding members from isense. The attribute is more explicit, doesn't require exposing an additional public interface, nor require changing any of the callsites, is easier to apply, and yet still preserves isesne for the XF team (EditorBrowsable only affects isense of referenced assemblies, not referenced projects). What's not to like! Going forward we can convert the existing controller interfaces to use the attribute approach (recommended) or we could add new controller interfaces on top of the attribute approach as the two are not exclusive.

IMHO, ultimately, the contractors code should live in the XF repo (maybe in another branch) and each core/platform assembly pair should be merged into a single assembly. This would eliminate this extra scope as well as the need for ExportRenderer and forwarder types for the core renderers. This conversion from dynamic to static binding could only speed up our load times. Assembly merging has also been shown to result in smaller assemblies after linking.

Bugs Fixed

None

API Changes

None. The public surface area we support is unchanged.

Behavioral Changes

None. The change should result in no behavioral changes.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
Show outdated Hide outdated Xamarin.Forms.Core/IPinchGestureController.cs
{
internal interface IPinchGestureController
public interface IPinchGestureController

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Does not need to be in Internals namespace

@jassmith

jassmith Feb 24, 2017

Member

Does not need to be in Internals namespace

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

It was moved because it was previously internal. Will move it back to XF namespace and expose publicly. Suggest you review to make sure it's the API you want exposed.

@kingces95

kingces95 Feb 24, 2017

Member

It was moved because it was previously internal. Will move it back to XF namespace and expose publicly. Suggest you review to make sure it's the API you want exposed.

Show outdated Hide outdated Xamarin.Forms.Core/IPanGestureController.cs
{
internal interface IPanGestureController
public interface IPanGestureController

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Does not need to be in internals namespace

@jassmith

jassmith Feb 24, 2017

Member

Does not need to be in internals namespace

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

It was moved because it was previously internal. Will move it back to XF namespace and expose publicly. Suggest you review to make sure it's the API you want exposed.

@kingces95

kingces95 Feb 24, 2017

Member

It was moved because it was previously internal. Will move it back to XF namespace and expose publicly. Suggest you review to make sure it's the API you want exposed.

@@ -17,7 +18,8 @@ public class MenuItem : BaseMenuItem, IMenuItemController
public static readonly BindableProperty IconProperty = BindableProperty.Create("Icon", typeof(FileImageSource), typeof(MenuItem), default(FileImageSource));
internal static readonly BindableProperty IsEnabledProperty = BindableProperty.Create("IsEnabled", typeof(bool), typeof(ToolbarItem), true);
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

This is a static member. What convention would you like to use to expose it?

@kingces95

kingces95 Feb 24, 2017

Member

This is a static member. What convention would you like to use to expose it?

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

Actually, there already is a controller. Let me know if the public surface area of that controller is not to your liking.

@kingces95

kingces95 Feb 24, 2017

Member

Actually, there already is a controller. Let me know if the public surface area of that controller is not to your liking.

@@ -159,15 +161,17 @@ protected virtual void UnhookContent(T content)
{
}
internal static int GetIndex(T page)
[EditorBrowsable(EditorBrowsableState.Never)]
public static int GetIndex(T page)

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

Do you have a preference for how we expose static members? We could just leave them as is for now and if a customer needs those methods just tell them to use the isense hidden ones.

@kingces95

kingces95 Feb 24, 2017

Member

Do you have a preference for how we expose static members? We could just leave them as is for now and if a customer needs those methods just tell them to use the isense hidden ones.

@@ -35,7 +36,8 @@ public int NumberOfTapsRequired
public event EventHandler Tapped;
internal void SendTapped(View sender)
[EditorBrowsable(EditorBrowsableState.Never)]
public void SendTapped(View sender)

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@@ -86,7 +87,8 @@ public partial class VisualElement : Element, IAnimatable, IVisualElementControl
public static readonly BindableProperty MinimumHeightRequestProperty = BindableProperty.Create("MinimumHeightRequest", typeof(double), typeof(VisualElement), -1d, propertyChanged: OnRequestChanged);
internal static readonly BindablePropertyKey IsFocusedPropertyKey = BindableProperty.CreateReadOnly("IsFocused", typeof(bool), typeof(VisualElement), default(bool),
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

We agreed statics will just get the Editor attribute.

@kingces95

kingces95 Mar 2, 2017

Member

We agreed statics will just get the Editor attribute.

@@ -210,7 +212,8 @@ public SeparatorVisibility SeparatorVisibility
set { SetValue(SeparatorVisibilityProperty, value); }
}
internal ListViewCachingStrategy CachingStrategy { get; private set; }
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

Already exposed via interface. The explicit implementation is just below. The internal implementation was left so we wouldn't have to touch the callsites. Really, we should go back and remove the explicit implementations of the controller interfaces from our code, replace them all with the EditorBrowser idiom if it's not there already, and revert all the callsites back to directly accessing the member.

@kingces95

kingces95 Feb 24, 2017

Member

Already exposed via interface. The explicit implementation is just below. The internal implementation was left so we wouldn't have to touch the callsites. Really, we should go back and remove the explicit implementations of the controller interfaces from our code, replace them all with the EditorBrowser idiom if it's not there already, and revert all the callsites back to directly accessing the member.

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

IFooController interfaces are there for when customers need to actually use the methods for implementing renderers. Simply hidden from intellisense is there for when they are not. That is the distinction we need to draw with Controller vs EditorBrowsable

@jassmith

jassmith Feb 24, 2017

Member

IFooController interfaces are there for when customers need to actually use the methods for implementing renderers. Simply hidden from intellisense is there for when they are not. That is the distinction we need to draw with Controller vs EditorBrowsable

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

Yep, got that. I'm suggesting we change how we implement the interfaces (not whether we need them). I'd suggest moving from implicit implementation to explicit implementation with the addition of the EditorBrowser attribute. That way our callsites don't go through the interface and so are cleaner. Customers could do the same but will prob go through the interfaces because that is how they'll get isense as well as know that we're supporting the method.

@kingces95

kingces95 Feb 24, 2017

Member

Yep, got that. I'm suggesting we change how we implement the interfaces (not whether we need them). I'd suggest moving from implicit implementation to explicit implementation with the addition of the EditorBrowser attribute. That way our callsites don't go through the interface and so are cleaner. Customers could do the same but will prob go through the interfaces because that is how they'll get isense as well as know that we're supporting the method.

Show outdated Hide outdated Xamarin.Forms.Core/Grid.cs
@@ -212,7 +212,8 @@ internal override void ComputeConstraintForView(View view)
view.ComputedConstraint = result;
}
internal override void InvalidateMeasureInternal(InvalidationTrigger trigger)
[EditorBrowsable(EditorBrowsableState.Never)]
public override void InvalidateMeasureInternal(InvalidationTrigger trigger)

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Lets make a method on the Controller interface which calls this instead.

@jassmith

jassmith Feb 24, 2017

Member

Lets make a method on the Controller interface which calls this instead.

@@ -158,7 +159,8 @@ internal IPlatform Platform
}
// you're not my real dad
internal Element RealParent { get; private set; }
[EditorBrowsable(EditorBrowsableState.Never)]
public Element RealParent { get; private set; }

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

@jassmith

jassmith Feb 24, 2017

Member

Use controller interface for this class

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

The controller interface declares RealParent so it is on the controller interface.

@kingces95

kingces95 Mar 2, 2017

Member

The controller interface declares RealParent so it is on the controller interface.

@@ -91,7 +91,8 @@ public void SetValue(BindablePropertyKey propertyKey, object value)
SetValue(propertyKey.BindableProperty, value, false, false);
}
protected internal static void SetInheritedBindingContext(BindableObject bindable, object value)
[EditorBrowsable(EditorBrowsableState.Never)]
public static void SetInheritedBindingContext(BindableObject bindable, object value)
{

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

SetInheritedBindingContext(Page, BindingContext);
+ others

@kingces95

kingces95 Feb 24, 2017

Member

SetInheritedBindingContext(Page, BindingContext);
+ others

Show outdated Hide outdated Xamarin.Forms.Core/BindableObject.cs
@@ -171,7 +172,8 @@ internal bool GetIsDefault(BindableProperty targetProperty)
return GetContext(targetProperty) == null;
}
internal object[] GetValues(BindableProperty property0, BindableProperty property1)
[EditorBrowsable(EditorBrowsableState.Never)]
public object[] GetValues(BindableProperty property0, BindableProperty property1)

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

No references. Revert to internal.

@kingces95

kingces95 Feb 24, 2017

Member

No references. Revert to internal.

@@ -202,7 +204,8 @@ internal object[] GetValues(BindableProperty property0, BindableProperty propert
return values;
}
internal object[] GetValues(BindableProperty property0, BindableProperty property1, BindableProperty property2)
[EditorBrowsable(EditorBrowsableState.Never)]
public object[] GetValues(BindableProperty property0, BindableProperty property1, BindableProperty property2)
{

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

var values = label.GetValues(Label.FontFamilyProperty, Label.FontSizeProperty, Label.FontAttributesProperty);
+ others

@kingces95

kingces95 Feb 24, 2017

Member

var values = label.GetValues(Label.FontFamilyProperty, Label.FontSizeProperty, Label.FontAttributesProperty);
+ others

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

there's one with 2 arguments you might want to open as well

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

there's one with 2 arguments you might want to open as well

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

or create one that takes a param BindableProperty [] argument

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

or create one that takes a param BindableProperty [] argument

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

I'd guess the params keyword was not used here in order to prevent GC allocation of the array; that signature could be an optimization. For this PR, I'd prefer not to make any control flow or signature modifications to ensure there are no regressions. More to the point, this API was not requested to be put on a controller iface. So this change is not expanding our supported surface area.

@kingces95

kingces95 Mar 2, 2017

Member

I'd guess the params keyword was not used here in order to prevent GC allocation of the array; that signature could be an optimization. For this PR, I'd prefer not to make any control flow or signature modifications to ensure there are no regressions. More to the point, this API was not requested to be put on a controller iface. So this change is not expanding our supported surface area.

Show outdated Hide outdated Xamarin.Forms.Core/BindableObject.cs
@@ -321,12 +324,14 @@ internal void SetValue(BindableProperty property, object value, bool fromStyle)
SetValue(property, value, fromStyle, true);
}
internal void SetValueCore(BindablePropertyKey propertyKey, object value, SetValueFlags attributes = SetValueFlags.None)
[EditorBrowsable(EditorBrowsableState.Never)]
public void SetValueCore(BindablePropertyKey propertyKey, object value, SetValueFlags attributes = SetValueFlags.None)

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

No references. Revert to internal.

@kingces95

kingces95 Feb 24, 2017

Member

No references. Revert to internal.

{
SetValueCore(propertyKey.BindableProperty, value, attributes, SetValuePrivateFlags.None);
}
internal void SetValueCore(BindableProperty property, object value, SetValueFlags attributes = SetValueFlags.None)
[EditorBrowsable(EditorBrowsableState.Never)]
public void SetValueCore(BindableProperty property, object value, SetValueFlags attributes = SetValueFlags.None)

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

SetValueCore(IsRefreshingProperty, true);
+ many others

@kingces95

kingces95 Feb 24, 2017

Member

SetValueCore(IsRefreshingProperty, true);
+ many others

This comment has been minimized.

@jassmith

jassmith Feb 24, 2017

Member

This particular call site could just be made to be ((IElementController)Element).SetValueFromRenderer (IsRefreshingProperty, true);

@jassmith

jassmith Feb 24, 2017

Member

This particular call site could just be made to be ((IElementController)Element).SetValueFromRenderer (IsRefreshingProperty, true);

This comment has been minimized.

@kingces95

kingces95 Feb 24, 2017

Member

I propose we do a BindableObject scrub in a subsequent PR and keep this a mechanical change. There's a lot of changes here which makes it hard enough to commit (it's already gone from green to red). Let's get these changes committed as a block with the invariant that no non-trivial control flow changes were made. In general, this is nice for future investigators who could look at the note and dismiss the PR.

@kingces95

kingces95 Feb 24, 2017

Member

I propose we do a BindableObject scrub in a subsequent PR and keep this a mechanical change. There's a lot of changes here which makes it hard enough to commit (it's already gone from green to red). Let's get these changes committed as a block with the invariant that no non-trivial control flow changes were made. In general, this is nice for future investigators who could look at the note and dismiss the PR.

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

if this as to be changed in a different PR (I agree with that), it should be done in a one merged BEFORE, not after, or the public will stay forever

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

if this as to be changed in a different PR (I agree with that), it should be done in a one merged BEFORE, not after, or the public will stay forever

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

As the API is browser attributed it doesn't constitute part of our supported public surface area and as such could be changed back to internal at a later state. I have no objection to doing SetvalueCore -> SetValueFromRenderer scrub but just not as part of this PR; conversion to SetValueFromRenderer is not necessary for IVT removal and as a non-trivial change could potentially break something.

@kingces95

kingces95 Mar 2, 2017

Member

As the API is browser attributed it doesn't constitute part of our supported public surface area and as such could be changed back to internal at a later state. I have no objection to doing SetvalueCore -> SetValueFromRenderer scrub but just not as part of this PR; conversion to SetValueFromRenderer is not necessary for IVT removal and as a non-trivial change could potentially break something.

@StephaneDelcroix

StephaneDelcroix requested changes Mar 2, 2017 edited

I reviewed the Core part, and wow, this is massive ! I've left comments here and there, here are some additional generic ones:

  • whenever you can, use Controller interface
  • when implementing controller interfaces, always implement explicitly
  • if a controller isn’t suitable and you want to turn an internal setter public, consider writing an extension method in the Internals namespace
  • do not open generic helpers we use internally, esp when they can be rewritten in a handful of seconds (ReflectionExtensions, NumericExtensions, ...).

We want this to be done quickly, but we want it to be correct as well. Ping me for additional review when needed.

If unsure, I'm fine if we break this into manageable and reviewable chunks...

{
internal class ActionSheetArguments
public class ActionSheetArguments

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

as this is now public, the online doc is no longer required

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

as this is now public, the online doc is no longer required

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

Online doc?

@kingces95

kingces95 Mar 2, 2017

Member

Online doc?

Show outdated Hide outdated Xamarin.Forms.Core/Application.cs
public static Application Current
{
get { return s_current; }
internal set
{
set {

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

I'm almost willing to Approve this PR without further review because of this ideally placed opening brace 👍

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

I'm almost willing to Approve this PR without further review because of this ideally placed opening brace 👍

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

Hehe, no such luck...

@kingces95

kingces95 Mar 2, 2017

Member

Hehe, no such luck...

@@ -48,11 +49,13 @@ public IAppLinks AppLinks
}
}
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

is this supported by XS or VS4Mac ?

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

is this supported by XS or VS4Mac ?

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

Yes

@kingces95
@@ -202,7 +204,8 @@ internal object[] GetValues(BindableProperty property0, BindableProperty propert
return values;
}
internal object[] GetValues(BindableProperty property0, BindableProperty property1, BindableProperty property2)
[EditorBrowsable(EditorBrowsableState.Never)]
public object[] GetValues(BindableProperty property0, BindableProperty property1, BindableProperty property2)
{

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

there's one with 2 arguments you might want to open as well

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

there's one with 2 arguments you might want to open as well

@@ -202,7 +204,8 @@ internal object[] GetValues(BindableProperty property0, BindableProperty propert
return values;
}
internal object[] GetValues(BindableProperty property0, BindableProperty property1, BindableProperty property2)
[EditorBrowsable(EditorBrowsableState.Never)]
public object[] GetValues(BindableProperty property0, BindableProperty property1, BindableProperty property2)
{

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

or create one that takes a param BindableProperty [] argument

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

or create one that takes a param BindableProperty [] argument

{
INavigation _inner;
Lazy<List<Page>> _modalStack = new Lazy<List<Page>>(() => new List<Page>());
Lazy<List<Page>> _pushStack = new Lazy<List<Page>>(() => new List<Page>());
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

why is the attribute needed in the Internals namespace ?
I'd decorate the class with the attribute, but when you knowingly use the class, platform implementers could benefit from a little help in their editors...

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

why is the attribute needed in the Internals namespace ?
I'd decorate the class with the attribute, but when you knowingly use the class, platform implementers could benefit from a little help in their editors...

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

Good point. On the subsequent Internals scrub I'll check to see if adding the attribute to the class simply hides all the members.

@kingces95

kingces95 Mar 2, 2017

Member

Good point. On the subsequent Internals scrub I'll check to see if adding the attribute to the class simply hides all the members.

{
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

can be public without restrictions

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

can be public without restrictions

public static double Clamp(this double self, double min, double max)
{
return Math.Min(max, Math.Max(self, min));
}
[EditorBrowsable(EditorBrowsableState.Never)]

This comment has been minimized.

@StephaneDelcroix
@StephaneDelcroix

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

Agreed. Could be. But let's save changing the public surface area for another PR. The goal of this PR is just to remove IVT and as we can see that's a pretty big change as is.

@kingces95

kingces95 Mar 2, 2017

Member

Agreed. Could be. But let's save changing the public surface area for another PR. The goal of this PR is just to remove IVT and as we can see that's a pretty big change as is.

{
internal static class Performance
public static class Performance

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

this doesn't need to be public, it's a tool we use but other implementors might want to use other ones

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

this doesn't need to be public, it's a tool we use but other implementors might want to use other ones

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

It's in the internals namespace so its not part of our supported public surface.

@kingces95

kingces95 Mar 2, 2017

Member

It's in the internals namespace so its not part of our supported public surface.

{
internal static class ReflectionExtensions
public static class ReflectionExtensions

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

I think this is only used by the Binding system and the Xaml parser. no need to open it. everyone can rewrite the same

@StephaneDelcroix

StephaneDelcroix Mar 2, 2017

Member

I think this is only used by the Binding system and the Xaml parser. no need to open it. everyone can rewrite the same

This comment has been minimized.

@kingces95

kingces95 Mar 2, 2017

Member

Again, in the internals namespace so it's not part of the supported public surface area. It's prob here so it can be shared across all the platforms. If we want to refactor to pull this into each platform or into a utils assembly we can do that in a separate PR.

@kingces95

kingces95 Mar 2, 2017

Member

Again, in the internals namespace so it's not part of the supported public surface area. It's prob here so it can be shared across all the platforms. If we want to refactor to pull this into each platform or into a utils assembly we can do that in a separate PR.

@kingces95 kingces95 merged commit e6d5186 into master Mar 7, 2017

1 check was pending

Windows-Debug-C8 Started TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Debug
Details
@mdizzy

This comment has been minimized.

Show comment
Hide comment
@mdizzy

mdizzy May 21, 2017

Out of curiosity, will this be published in an official release? I'm building some renderers that need things like SendTapped on the TapGestureRecognizer, basically I'm creating irregular shaped views and in order to do so need to implement custom gesture handling.

mdizzy commented May 21, 2017

Out of curiosity, will this be published in an official release? I'm building some renderers that need things like SendTapped on the TapGestureRecognizer, basically I'm creating irregular shaped views and in order to do so need to implement custom gesture handling.

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho May 23, 2017

Member

@mdizzy is already in public beta 2.3.5-pre

Member

rmarinho commented May 23, 2017

@mdizzy is already in public beta 2.3.5-pre

@rmarinho rmarinho deleted the removeivt branch Jun 22, 2017

@rookiejava rookiejava referenced this pull request Jun 27, 2017

Merged

Remove InternalsVisibleTo from Maps #1019

4 of 4 tasks complete

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts modified the milestone: 3.1.0 Jun 1, 2018

@samhouts samhouts added this to the 2.3.5 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment