Custom Controls, SetNativeControl and proper event handling

I am trying to use the Xlabs ImageGallery. In looking into the code it seems to have some issues with event handlers and I am just trying to get an understanding as to how it theoretically should be handled. If I create a page with the ImageGallery on it and I move to and from that page, the events will fire, once for each time the page has been displayed. This would indicate that the events are being assigned but not unassigned. Also there is a call to AddObserver in the Draw event (which also accumulates) but there is never a call to RemoveObserver. This is a more complicated control and it seems to use SetNativeControl to add a UIView to a Xamarin page.

I don't want to know specifically how to fix this one control. I am trying to get an understanding of best practice here for controls in general. What is correct handling for the events? The events can be assigned with += in the constructor. However, where should the -= be set? There is a Dispose method in the ImageGallery but it doesn't get called. If I had a UIViewController I could use the OnViewAppear and OnViewDisappear. What would the equivalent pair of methods be for a custom control?

Tagged:

Posts

  • adamkempadamkemp USInsider, Developer Group Leader mod

    In general, don't assume that code in XLabs is correct. I've seen a lot of bad code in it.

    The general advice for events is that they should either be subscribed exactly once ever for that instance (like in a constructor or ViewDidLoad kind of situation) or they should be balanced. If you do += in a function that could be called multiple times (OnElementChanged, ViewWill/DidAppear, MovedToWindow, Draw, etc.) without a corresponding -= then you likely have a bug like you described.

    Draw seems like an especially bad place to do that, and I would immediately question that code's correctness.

  • JeffButterworthJeffButterworth AUMember ✭✭

    Yes that is my understanding as well. But what I still don't understand is where the appropriate place is to do the += and -= when doing a custom control.

  • adamkempadamkemp USInsider, Developer Group Leader mod

    That depends on the event and the type of object subscribing to it. For a Xamarin.Forms custom renderer attaching to an event on its PCL element the correct pattern is to subscribe and unsubscribe in OnElementChanged like this:

    protected override void OnElementChanged (ElementChangedEventArgs<ViewType> e)
    {
        base.OnElementChanged(e);
        if (e.OldElement != null)
        {
            e.OldElement.Event -= EventHandler;
        }
        if (e.NewElement != null)
        {
            e.NewElement.Event += EventHandler;
        }
    }
    

    If that's not the situation you're interested in then you'll have to be more specific about the types of objects and events involved.

  • JeffButterworthJeffButterworth AUMember ✭✭

    @adamkemp that makes sense for a basic control. However, in the case of the ImageGallery in the OnElementChange it creates a view and passes that as a parameter to SetNativeControl. Inside that view there is various event assigning/unassigning that needs to happen. So I should be to assign that view to a private variable in the renderer and (using the pattern you suggested) access that private variable and call a method to unassign the event handlers.

  • JeffButterworthJeffButterworth AUMember ✭✭

    The Dispose method on the view gets called so I can put cleanup code in there. No need to assign the view to a private variable in the renderer. I could have sworn last night when I was looking at it, the dispose wasn't being called. Too long a day yesterday and the benefit of a good night's sleep I guess....

  • adamkempadamkemp USInsider, Developer Group Leader mod

    In a custom UIView you can use MovedToWindow to subscribe and unsubscribe events depending on whether Window is null or not. I prefer that approach over relying on Dispose to get called.

Sign In or Register to comment.