Expose renderers, so you are able to override or extend them on each platform #34
Conversation
# Conflicts: # SampleApp/SampleApp/SampleApp.csproj
# Conflicts: # AutoSuggestBox/AutoSuggestBoxRenderer.cs # AutoSuggestBox/NativeAutoSuggestBox.Android.cs # AutoSuggestBox/NativeAutoSuggestBox.iOS.cs # AutoSuggestBox/dotMorten.Xamarin.Forms.AutoSuggestBox/AutoSuggestBoxRenderer.cs # AutoSuggestBox/dotMorten.Xamarin.Forms.AutoSuggestBox/NativeAutoSuggestBox.Android.cs # AutoSuggestBox/dotMorten.Xamarin.Forms.AutoSuggestBox/NativeAutoSuggestBox.iOS.cs # AutoSuggestBox/dotMorten.Xamarin.Forms.AutoSuggestBox/Platform/AutoSuggestBoxRenderer.cs # AutoSuggestBox/dotMorten.Xamarin.Forms.AutoSuggestBox/Platform/NativeAutoSuggestBox.Android.cs # AutoSuggestBox/dotMorten.Xamarin.Forms.AutoSuggestBox/Platform/NativeAutoSuggestBox.iOS.cs
|
sorry for the delay @dotMorten. I got pulled into something else on the related project, so I haven't had a chance to look back into this during work hours. I did a quick read through of the proposed changes with this PR. This is definitely along the route of what I was thinking when I had asked about making the renderers extendable. It isn't uncommon for the custom internal control (the NativeAutoSuggestBox in this case) to be left as an internal class, or public sealed. Xamarin.Forms does that a fair amount with their renderers (or at least they did, I haven't been as deep in renderers as of late). The main thing that would allow the auto suggest box to be customizable is making the renderers public like they are in the in the PR and then utilizing the CreateNativeControl method to create the control. At a minimum, that would allow developers to change a few of the stylistic elements of the EditText/UITextField/etc. I don't think you would need to go as a far as letting developers extend the NativeAutoSuggestBox class. It looks like there is a lot of state in there could get messed up if developers got a little too crazy with their extending. With my original question/proposal, I think making the renderers public, and making the NativeAutoSuggestBox public sealed would suffice for what I was needing. The main thing was to allow the native control to be styled with borders/colors/layers/etc. Which I think a public sealed native control class would allow for. I would need to give that a trial run to know for sure. I wouldn't want to give you a suggestion that I think might suffice and in the end that not be the case. |
|
I can circle back on this when I have a bit more time to devote to it. My nights and weekends get a little packed. I'd want to try a sample integration with the current changes, and do some customizing of the control. I totally get where you're coming from on the level of extension. Making all the things public is for sure a point of no return. So, I'd want to make sure I'm not suggesting something that makes your life difficult as the maintainer of the project. |
A first stab at addressing #33
FYI @MysterDru is this what you're thinking? Am I on the right track? I want to be absolutely sure I'm following common practices here. before shipping a new public API that I can't change after that.
Would you mind doing a code review? (some white space issues was also fixed so turn on ignore white space to make it a little more reasonable to look at)