[forms-devel] Making the Effect constructor protected

Thu May 26 18:49:07 UTC 2016

Hi Miguel and co,

I had 2 scenarios for which I was looking to use an Effect. But with some guidance from Jason, I’ve realized that there are better approaches:

a) To apply some changes using Xamarin.Forms portable APIs to arbitrary Views on attach, and then to reset them on detach. For this scenario, Behaviors are a better choice.

b) Similar to (a), but Behaviors would not work because they do not guarantee that the renderer is available (while Effects do). While I needed the renderer, I was just doing a little reflection that was not platform-specific, so I was originally looking to do it in the PCL by subclassing Effect. However, I am now convinced it will be less fragile to just make the extra platform-specific assemblies and subclass PlatformEffect, even if the code in those assemblies is identical.

So in light of this, I am withdrawing my proposal. However, I suggest we add a note to the API documentation for the Effect class to point people at Behaviors who fall into category (a) above. This might save some people from starting down the same path I did.

Thanks everyone for your time,

Alex

> On May 25, 2016, at 10:56 PM, Miguel de Icaza <miguel at xamarin.com> wrote:
> 
> Hello,
> 
> Alex, can you shed some light as to what is the scenario that you wanted to cover by making the constructor accessible beyond the assembly?
> 
> On Wed, May 25, 2016 at 5:45 PM, Jason Smith <jason.smith at xamarin.com <mailto:jason.smith at xamarin.com>> wrote:
> We cannot move where OnElementPropertyChanged is declared without breaking ABI.
> 
> My objection is subclassing Effect directly has uses, but we have not clearly identified even a reasonable subset of them. Without having even the faintest idea of what we are going to use this API for we are about to open it up. However by opening it we drive users away from Behaviors which are more intended for the use case I assume you are thinking of doing with the Effect, which was part of the reason Effect is sealed. The Effect being sealed thing is really just a convenient implementation detail working around our platform limitations.
> 
> On Wed, May 25, 2016 at 2:36 PM, Alex Corrado <corrado at xamarin.com <mailto:corrado at xamarin.com>> wrote:
> 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 <mailto: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/20160526/158aa8e1/attachment.html>