[forms-devel] Making the Effect constructor protected

[Alex Corrado]

Hey,

Thank you for saving me from writing a spec Miguel :)

Jason, to address your concern, we are already allowing people to subclass Effect indirectly, because PlatformEffect subclasses it and we are encouraging people to subclass that. So I don’t think making the Effect constructor public adds any additional constraints on our end.

The only (minor) API issue I see is that PlatformEffect declares `OnElementPropertyChanged`, when that should probably be declared in Effect itself. I’m not sure if we could move that without breaking ABI. But Effect also exposes the Element directly, so a subclassor could just subscribe to the property changed event themselves. 

-Alex

> On May 25, 2016, at 5:03 PM, Miguel de Icaza <miguel at xamarin.com> wrote:
> 
> Hello,
> 
> I think we have enough information on this thread as to what is needed and we should make a decision.
> 
> Currently the Effect class can not be subclassed.   If we think that there are problems with the current implementation that would prevent us from opening that, let us have a discussion about what are the design problems with Effect, so we can decide what to do about it.
> 
> MIguel.
> 
> On Tue, May 24, 2016 at 3:15 PM, Jason Smith <jason.smith at xamarin.com <mailto:jason.smith at xamarin.com>> wrote:
> Hi Alex,
> 
> The issue I see with this is we just are not sure that we are ready to expose the effects API as is publicly. Once it is protected instead of internal that is forever a type users can inherit. That said, please submit a spec so we can have a formal discussion/review about it.
> 
> Jason
> 
> On Tue, May 24, 2016 at 12:13 PM, Alex Corrado <corrado at xamarin.com <mailto:corrado at xamarin.com>> wrote:
> Hello Forms people,
> 
> As it stands now, it is impossible to create a cross-platform Effect in a PCL. To enable this, I propose we make the Xamarin.Forms.Effect constructor `protected` instead of `internal`.
> 
> I have already tried it and found it to work great, and I can’t think of any way this could break any existing code. I already submitted a PR, https://github.com/xamarin/Xamarin.Forms/pull/178 <https://github.com/xamarin/Xamarin.Forms/pull/178> , but was told to discuss here first.
> 
> Cheers,
> Alex Corrado
> 
> 
> _______________________________________________
> forms-devel mailing list
> forms-devel at lists.xamarin.com <mailto:forms-devel at lists.xamarin.com>
> http://lists.ximian.com/mailman/listinfo/forms-devel <http://lists.ximian.com/mailman/listinfo/forms-devel>
> 
> 
> _______________________________________________
> forms-devel mailing list
> forms-devel at lists.xamarin.com <mailto:forms-devel at lists.xamarin.com>
> http://lists.ximian.com/mailman/listinfo/forms-devel <http://lists.ximian.com/mailman/listinfo/forms-devel>
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ximian.com/pipermail/forms-devel/attachments/20160525/f209ce05/attachment-0001.html>