Add ReactiveUserControl<ViewModel> helper class #349

Closed
wants to merge 4 commits into from

7 participants

Brad Phelan Fixes #348 358bffb
@paulcbetts paulcbetts commented on the diff Jul 18, 2013
ReactiveUI.Platforms/Xaml/ReactiveUserControl.cs
+ get { return (ViewModelT)GetValue(ViewModelProperty); }
+ set { SetValue(ViewModelProperty, value); }
+ }
+
+ object IViewFor.ViewModel
+ {
+ get { return (ViewModelT)GetValue(ViewModelProperty); }
+ set { SetValue(ViewModelProperty, value); }
+ }
+ #endregion
+
+ /// <summary>
+ /// Gets or sets additional content for the UserControl. The additional
+ /// content will have it's DataContext set to an instance of the ViewModel
+ /// </summary>
+ public UIElement AdditionalContent
@paulcbetts
ReactiveUI member

This Seems Weird™. Why add this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@paulcbetts paulcbetts commented on the diff Jul 18, 2013
ReactiveUI.Platforms/Xaml/ReactiveUserControl.cs
+ public static readonly DependencyProperty AdditionalContentProperty =
+ DependencyProperty.Register("AdditionalContent", typeof(UIElement), typeof(ReactiveUserControl<ViewModelT>),
+ new PropertyMetadata(null));
+
+ public IObservable<ViewModelT> ViewModelObservable()
+ {
+ return this.WhenAny(x => x.ViewModel, x => x.Value)
+ .Where(v => v != null);
+ }
+
+ public UniformGrid DataContextHost { get; private set; }
+
+ public ReactiveUserControl()
+ {
+ DataContextHost = new UniformGrid();
+ this.Content = DataContextHost;
@paulcbetts
ReactiveUI member

Doesn't this effectively remove the standard content of the UserControl (i.e. the Xaml that you created)?

If it did what you said then my project wouldn't work and I'd have empty controls everywhere. What it does is provide a wrapper for internal XAML and sets it's DataContext to be the ViewModel which can't be overriden by setting DataContext on an instance of the control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@paulcbetts paulcbetts commented on the diff Jul 18, 2013
ReactiveUI.Platforms/Xaml/ReactiveUserControl.cs
+ DataContextHost = new UniformGrid();
+ this.Content = DataContextHost;
+
+ // If AdditionalContent changes then we need to
+ // add it to the visual tree
+ this.WhenAny(p => p.AdditionalContent, p => p.Value)
+ .Where(v => v != null)
+ .Subscribe(v =>
+ {
+ DataContextHost.Children.Clear();
+ DataContextHost.Children.Add(v);
+ });
+
+ // If the ViewModel changes we need to ensure the
+ // AdditionalContent get's the correct DataContext
+ this.WhenAny(x => x.ViewModel, x => x.Value)
@paulcbetts
ReactiveUI member

Binding ViewModel => DataContext is a good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@paulcbetts paulcbetts and 1 other commented on an outdated diff Jul 18, 2013
ReactiveUI.Platforms/Xaml/ReactiveUserControl.cs
+
+ public static class ReactiveUserControlMixins
+ {
+ /// <summary>
+ /// Manage lifetime of a stream of disposables relative to the
+ /// lifetime of the Control. Disposes the previous disposable in the stream
+ /// when the next is arrives and then dispose the last one when the UI is shut down
+ /// </summary>
+ /// <param name="This"></param>
+ /// <param name="control"></param>
+ public static void SeriallyDisposeWith(this IObservable<IDisposable> This, Control control)
+ {
+ var disposer = new SerialDisposable();
+ This.Subscribe(d => disposer.Disposable = d);
+ control.Unloaded += (s, e) => disposer.Dispose();
+ control.Dispatcher.ShutdownStarted += (s, e) => disposer.Dispose();
@paulcbetts
ReactiveUI member

This is memory leak city, Dispatcher is a global

Ok then I would have deregister that as well somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@paulcbetts paulcbetts commented on an outdated diff Jul 18, 2013
ReactiveUI.Platforms/Xaml/ReactiveUserControl.cs
+
+ /// <summary>
+ /// Manage the lifetime of a single disposable relative to the lifetime
+ /// of the control. The disposable is disposed either when the control
+ /// is unloaded or the dispatcher is shutdown.
+ ///
+ /// Be careful because the unload event is called on items in a DataTemplate
+ /// which are recycled in an ItemsControl. You may have to recreate the
+ /// disposable on the load event.
+ /// </summary>
+ /// <param name="This"></param>
+ /// <param name="contrl"></param>
+ public static void DisposeWith(this IDisposable This, Control contrl)
+ {
+ contrl.Unloaded += (s, e) => This.Dispose();
+ contrl.Dispatcher.ShutdownStarted += (s, e) => This.Dispose();
@paulcbetts
ReactiveUI member

Ditto

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

"Weird" is not exactly constructive criticism. What don't you understand? Normally you have to manually implement IViewFor in all your Views and then make sure you set your DataContext correctly. This does it all for you in two line of code.

@paulcbetts
ReactiveUI member

Normally you have to manually implement IViewFor in all your Views and then make sure you set your DataContext correctly

I wasn't referring to the PR in general, just the AdditionalContent stuff - I'm not sure I grok that bit

@bradphelan

Oh ok :)

@paulcbetts
ReactiveUI member

I think that ReactiveViewControl is generally A Good Idea™, though it's a huge bummer that XAML is dumb and doesn't really cooperate. I'm not sure it needs all of these Bells and Whistles though - something more like your ReactiveWindow, but maybe with the ViewModel => DataContext glue

@bradphelan

I know you like to do statically typed binding from code behind. I'm luke warm to that for various reasons. I do most of my binding in XAML. The pros outweight the cons in my mind. Anyway XAML has a bizarre concept of scope with regards to DataContext. If in the code behind you write

this.DataContext = ViewModel

you would expect in your XAML for this always to be the case for the binding. However if in an instance of your user control you do something like

<c:MyControl DataContext="{Binding Foo}" Value="{Binding Bar}"/>

then you override the internal DataContext which is just plain dumb from a scope point of view. Normally if you did it manually you would create a Grid to hold your ViewModel DataContext as a shield against the top level DataContext. This magic just the Right Thing automatically.

@bradphelan

Note all this would be easier with Observable handling but I can't get the Events library to build. ( I've submitted an issue on this )

Observable.CombineLatest
    ( control.Events().Loadeded()
    , control.Dispatcher.Events().ShutdownStarted()
    )
    .Take(1)
    .Subscribe(e=>disposable.Dispose());

No worries with leaky boats. The above is a single shot event handler that unsubscribes after the first of either event type is received. Much nicer that the class I built.

@bradphelan

I've updated the pull request to fix the memory leaks. Any comments?

@paulcbetts
ReactiveUI member

@bradphelan Are you by chance running Windows 8.1? I still need to fix this so that the Events library builds there.

@bradphelan
@jlaanstra
ReactiveUI member

Generic user controls do not work on WinRT or Windows Phone. I think the AdditionalContent/DataContextHost stuff needs to go for this to be merged.

Thoughts?

@paulcbetts
ReactiveUI member

They don't really work on WPF either afaik, no?

@paulcbetts
ReactiveUI member

@jlaanstra Are you sure? I thought that it was supported in the new XAML parser that only Windows Workflow uses

@shiftkey

In WPF and targeting .NET Framework 4, you can use XAML 2009 features together with x:TypeArguments, but only for loose XAML (XAML that is not markup-compiled). Markup-compiled XAML for WPF and the BAML form of XAML do not currently support the XAML 2009 keywords and features.

Every time I've tried to get generic controls working in WPF in a sane way I've come off second best.

@jlaanstra
ReactiveUI member

In WPF and targeting .NET Framework 4, you can use XAML 2009 features together with x:TypeArguments , but only for loose XAML (XAML that is not markup-compiled). Markup-compiled XAML for WPF and the BAML form of XAML do not currently support the XAML 2009 keywords and features.

Every time I've tried to get generic controls working in WPF in a sane way I've come off second best.

You seem to be right. I think this is not worth it then.

@bradphelan

I have no problem and I use my generic ReactiveUserControl<T> all the time. The trick is to create a shim to shield XAML from the generics. I have about 8 views currently in my code using ReactiveUserControl<T>. This is how I do it.

using ReactiveUI;
using WeinCad.Controls.ViewModel;

namespace WeinCad.Controls.View
{
    /// <summary>
    /// Interaction logic for MeasuringView.xaml
    /// </summary>
    public class MeasuringViewBase : ReactiveUserControl<MeasuringViewModel> { }

    public partial class MeasuringView
    {
        public MeasuringView()
        {
            InitializeComponent();
        }

    }
}

and the XAML is so

<local:MeasuringViewBase xmlns:WPF="clr-namespace:Weingartner.Numerics.EyeShot.WPF;assembly=Weingartner.Numerics"  
             x:Class="WeinCad.Controls.View.MeasuringView"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:local="clr-namespace:WeinCad.Controls.View"
             xmlns:c="clr-namespace:Weingartner.Controls;assembly=Weingartner.Controls"
             xmlns:viewModel="clr-namespace:WeinCad.Controls.ViewModel"
             xmlns:rx="clr-namespace:ReactiveUI.Ext;assembly=ReactiveUI.Ext"
             mc:Ignorable="d" d:DesignWidth="770" d:DesignHeight="270" 
             >
    <UserControl.Resources>
        <ResourceDictionary>
            <ResourceDictionary.MergedDictionaries>
                <ResourceDictionary Source="/Themes/generic.xaml"/>
            </ResourceDictionary.MergedDictionaries>
        </ResourceDictionary>
    </UserControl.Resources>

Works brilliantly all the time.

@jen20

@bradphelan Don't you have to inherit from MeasuringViewBase for this to do anything useful?

@bradphelan
@bradphelan
@tlycken

Did this PR just die out of low interest, or has it been worked with elsewhere? I've come to need this functionality too, so I'd be very interested in seeing this merged :)

@bradphelan In the meantime, do you mind if I just copy-paste your class definition into our project to keep moving forward?

@bradphelan

Fine by me. I'd forgotten about it. The code we use in house has probably migrated a bit since this pull request

@bradphelan

Here's the latest code we use. Not sure if it has changed much but two years is a while.

using System.Collections;
using System.Collections.Generic;
using System.Drawing;
using System.Reactive;
using System.Windows.Media;
using ReactiveUI.Ext;
using System;
using System.Reactive.Disposables;
using System.Reactive.Linq;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Controls.Primitives;
using System.Windows.Markup;
using Weingartner.Lens;
using Color = System.Windows.Media.Color;

namespace ReactiveUI
{
    /// <summary>
    /// A generic control for wrapping up the concept of a View that has a ViewModel.
    /// Due to XAML not being friendly for generic types it is recommended to use the 
    /// following pattern
    /// 
    ///    namespace BradsAwsomeNamespace
    ///    {
    ///        public class BradViewModel : ReactiveObject {
    ///             string _Name;
    ///             public string Name 
    ///             {
    ///                 get { return _Name; }
    ///                 set { this.RaiseAndSetIfChanged(ref _Name, value); }
    ///             }
    ///             string _PhoneNumber;
    ///             public string PhoneNumber 
    ///             {
    ///                 get { return _PhoneNumber; }
    ///                 set { this.RaiseAndSetIfChanged(ref _PhoneNumber, value); }
    ///             }
    ///        }
    ///
    ///        public class BradViewBase : ReactiveUserControl<BradViewModel> { }
    ///        public partial class BradView : BradViewBase
    ///        {
    ///            public BradView()
    ///            {
    ///                InitializeComponent();
    ///            }
    ///        }
    ///    }
    ///
    /// and then in XAML do
    /// 
    ///    <l:BradViewBase x:Class="WeinCad.Controls.View.MoineauMillingView"
    ///             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    ///             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    ///             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
    ///             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
    ///             xmlns:l="clr-namespace:WeinCad.Controls.View"
    ///             mc:Ignorable="d" 
    ///             d:DesignHeight="300" d:DesignWidth="300">
    ///         <Grid>
    ///             <Label Content={Binding Name}/>
    ///             <Label Content={Binding PhoneNumber}/>
    ///         </Grid>
    ///    </l:BradViewBase>
    ///
    /// and use your new control like
    /// 
    ///     <DataTemplate>
    ///         <c:BradView ViewModel={Binding BradviewModel}/>
    ///     </DataTemplate>
    ///
    /// There are also helper methods to manage lifetimes of IDisposableResources. For example
    /// you may wish in your code behind to register to a timer but dispose of it when the
    /// user control is no longer used. Using ReactiveUserControl this is easy.
    /// 
    ///     _Counter = Observable.Interval(TimeSpan.Seconds(1))
    ///         .ToProperty(this, p => p.Counter);
    ///         
    ///     _Counter.DisposeWith(this);
    ///     
    /// Now the _Counter will be disposed when the control is unloaded or the
    /// dispatcher is shut down. 
    /// 
    ///
    /// Also note that the DataContext within the XAML associated with the user
    /// control is not the same as the DataContext of the user control when it
    /// is inserted into client XAML. For the purposes of sanity the DataContext
    /// inside the user control is the ViewModel instance. If a user of the
    /// user control manually sets the DataContext as in
    /// 
    ///     <c:BradView DataContext={Binding FooPath} ViewModel={Binding Model}/>
    ///     
    /// which is effectively the same thing as
    ///
    ///     <c:BradView ViewModel={Binding Path=FooPath.Model}/>
    ///     
    /// has no effect on the DataContext inside the XAML defining the BradView
    /// which will always remain the ViewModel instance.
    ///     
    /// </summary>
    /// <typeparam name="ViewModelT"></typeparam>
    [ContentProperty("AdditionalContent")]
    public class ReactiveUserControl<ViewModelT> : UserControl, IViewFor<ViewModelT>
        where ViewModelT : class
    {
        #region IViewFor<ViewModelT>
        public static readonly DependencyProperty ViewModelProperty =
            DependencyProperty.Register("ViewModel", typeof(ViewModelT), typeof(ReactiveUserControl<ViewModelT>), new PropertyMetadata(null));

        public ViewModelT ViewModel
        {
            get { return ( ViewModelT ) GetValue(ViewModelProperty); }
            set { SetValue(ViewModelProperty, value); }
        }

        object IViewFor.ViewModel
        {
            get { return ( ViewModelT ) GetValue(ViewModelProperty); }
            set { SetValue(ViewModelProperty, value); }
        }
        #endregion

        /// <summary>
        /// Gets or sets additional content for the UserControl. The additional
        /// content will have it's DataContext set to an instance of the ViewModel
        /// </summary>
        public UIElement AdditionalContent
        {
            get { return ( UIElement ) GetValue(AdditionalContentProperty); }
            set { SetValue(AdditionalContentProperty, value); }
        }

        public static readonly DependencyProperty AdditionalContentProperty =
            DependencyProperty.Register("AdditionalContent", typeof(UIElement), typeof(ReactiveUserControl<ViewModelT>),
              new PropertyMetadata(null));

        public IObservable<ViewModelT> ViewModelObservable()
        {
            return this.WhenAnyValue(x => x.ViewModel)
                .Where(v => v != null);
        }

        public UniformGrid DataContextHost { get; private set; }

        public ReactiveUserControl()
        {
            DataContextHost = new UniformGrid();
            this.Content = DataContextHost;
            AdditionalContent = new UIElement();

            // If AdditionalContent changes then we need to
            // add it to the visual tree
            this.WhenAnyValue(p => p.AdditionalContent)
                .Where(v => v != null)
                //.ObserveOn(RxApp.MainThreadScheduler)
                .Subscribe(v =>
                {
                    DataContextHost.Children.Clear();
                    DataContextHost.Children.Add(v);
                });

            // If the ViewModel changes we need to ensure the
            // AdditionalContent get's the correct DataContext
            this.WhenAnyValue(x => x.ViewModel)
                //.ObserveOn(RxApp.MainThreadScheduler)
                .BindTo(DataContextHost, x => x.DataContext);

            DesignUnsafeConstruct();
            if ( !System.ComponentModel.DesignerProperties.GetIsInDesignMode(this) )
            {

                DesignSafeConstruct();

            }
        }

        public virtual void DesignUnsafeConstruct()
        {
        }

        public virtual void DesignSafeConstruct()
        {
        }
    }

    public static class ReactiveUserControlMixins
    {
        public static void LoadUnloadHandler(this FrameworkElement control, Func<IEnumerable<IDisposable>> action)
        {
            LoadUnloadHandler(control, () => (IDisposable) new CompositeDisposable(action()));
        }

        public static void LoadUnloadHandler(this FrameworkElement control, Func<IDisposable> action)
        {
            bool state = false;
            var cleanup = Disposable.Empty;
            Observable.Merge
                ( control.Events().Loaded.Select(x => true)
                , control.Events().Unloaded.Select(x => false)
                )
                .Subscribe(isLoadEvent =>
                {
                    if (!state)
                    {
                        // unloaded state
                        if (isLoadEvent)
                        {
                            state = true;
                            cleanup = action();
                        }
                    }
                    else
                    {
                        // loaded state
                        if (!isLoadEvent)
                        {
                            state = false;
                            cleanup.Dispose();
                        }
                    }

                });
        }


        public static void LoadUnloadHandler<T>(this IObservable<T> @this, FrameworkElement control, Action<T> action)
        {
            LoadUnloadHandler(control, ()=>@this.Subscribe(action));
        }


        public static IDisposable SubscribeDisposable<T>
            ( this IObservable<T> observable
            , Func<T,IDisposable> action
            )
        {
            var d = Disposable.Empty;
            return observable
                .Finally(() => d.Dispose())
                .Subscribe
                (e =>
                 {
                     d.Dispose();
                     d = action(e);
                 });
        }

        public static IDisposable SubscribeDisposable<T>
            ( this IObservable<T> observable
            , Func<T,IEnumerable<IDisposable>> action)
        {
            return SubscribeDisposable( observable , t => ( IDisposable ) new CompositeDisposable(action(t)));
        }
    }


    public class LensUserControl<ViewModelT> : ReactiveUserControl<ILens<ViewModelT>>
    {
        public override void DesignUnsafeConstruct(){
            ViewModel = Lens.Empty<ViewModelT>();
        }
    }

    public class ReactiveWindow<ViewModelT> : Window, IViewFor<ViewModelT>
        where ViewModelT : class 
    {
        #region IViewFor<ViewModel>
        public static readonly DependencyProperty ViewModelProperty =
            DependencyProperty.Register("ViewModel", typeof(ViewModelT), typeof(ReactiveUserControl<ViewModelT>), new PropertyMetadata(null));

        public ViewModelT ViewModel
        {
            get { return (ViewModelT)GetValue(ViewModelProperty); }
            set { SetValue(ViewModelProperty, value); }
        }

        object IViewFor.ViewModel
        {
            get { return (ViewModelT)GetValue(ViewModelProperty); }
            set { SetValue(ViewModelProperty, value); }
        }
        #endregion

        public ReactiveWindow()
        {
            this.WhenAnyValue(x => x.ViewModel)
                .BindTo(this, x => x.DataContext);
        }
    }
}
@tlycken

Thanks a lot! 👍

Now I just have to figure out how best to make use of it ;)

@bradphelan

The only hacky thing is that generics in XAML kinda suck. So it's best to create a non generic base and then inherit your main control fromthat.

///        public class BradViewBase : ReactiveUserControl<BradViewModel> { }
///        public partial class BradView : BradViewBase
///        {
///            public BradView()
///            {
///                InitializeComponent();
///            }
///        }

see the doc at the top of the class. Let me know if you have any troubles. We use it in about 30 production views so it's been battle tested :)

@bradphelan

And disregard the class

    public class LensUserControl<ViewModelT> : ReactiveUserControl<ILens<ViewModelT>>
    {
        public override void DesignUnsafeConstruct(){
            ViewModel = Lens.Empty<ViewModelT>();
        }
    }

You don't need that.

@tlycken

Thanks for the updates :)

Unfortunately, I can't even get the class to build. I assume it's because I'm using it in a project for Windows Phone 8.1, and it's targeted at another platform?

I can make some simple changes (remove unused usings that don't compile, import some new classes that R# suggests, remove the argument to [ContentProperty] on the class) to get most of the errors to disappear, but there are still a few things I don't know how to handle:

// UniformGrid is not defined (line 147)
public UniformGrid DataContextHost { get; private set; }

and

// DesignerProperties isn't defined (line 173)
if (!System.ComponentModel.DesignerProperties.GetIsInDesignMode(this))
{
    DesignSafeConstruct();
}

and

// .Events is not defined (line 202)
Observable.Merge
    (control.Events().Loaded.Select(x => true)
    , control.Events().Unloaded.Select(x => false)
)

For the first one, can I just replace UniformGrid with Grid? If not, what do I do? For the second, I think I can just delete the entire check, since both the called methods are empty, but what do I do about the third?

Thanks a lot for all your help!

@bradphelan

Yeah, It's targeting a WPF usercontrol so you may find you need to modify for win 8.1 phone. UniformGrid is definately WPF. You could replace it with anything that will hold a child. There must be some kind of grid control in the standard toolbox you are using.

The DesignSafeConstruct pattern is something you can just remove if you want. It just a simple way to detect if the designer is running and not run the body of that function. Either find another detection mechanism for win 8.1 or get rid of it.

.Events is more tricky. This is a library that Paul write to make the C# event system more RX friendly. It comes from the ReactiveUI.Events library. However the implementation is pretty brute force and uses Cecil to generate extension methods for all known controls. However it might be that Paul didn't provide generators for the win 8.1 phone controls. Perhapps check with him.

@kentcb

I'm not sure I understand entirely the need for the shim base class (nor the AdditionalContent property). In my WPF project, I have a ReactiveUserControl defined as follows:

    public abstract class ReactiveUserControl<TViewModel> :
        UserControl, IViewFor<TViewModel>
        where TViewModel : class
    {
        public static readonly DependencyProperty ViewModelProperty =
            DependencyProperty.Register(
                "ViewModel",
                typeof(TViewModel),
                typeof(ReactiveUserControl<TViewModel>));

        public TViewModel ViewModel
        {
            get { return (TViewModel)this.GetValue(ViewModelProperty); }
            set { this.SetValue(ViewModelProperty, value); }
        }

        object IViewFor.ViewModel
        {
            get { return this.ViewModel; }
            set { this.ViewModel = (TViewModel)value; }
        }
    }

Which gets used like this:

<rxui:ReactiveUserControl
        x:Class="Namespace.Views.SomeView"
        x:TypeArguments="vms:SomeViewModel"
        ...

This approach has worked fine for me and I intended creating a PR for my ReactiveUserControl until I saw yours. Can you explain why my approach is inadequate? The only thing I can think of is Blend/designer support (which I don't use).

@bradphelan

The reason for the AddtionalContent is to set the DataContext for the child controls to be the viewmodel. However if you set the DataContext of the ReactiveUserControl itself to be the viewmodel then it changes the datacontext for users of the final user control.

@bradphelan

I created the shim class because I never got good results with using generics in WPF/XAML. If it works for you then cool 👍

@kentcb

Superseded by #943

@kentcb kentcb closed this Sep 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment