fix: Combobox not updating when using Bind() generated ViewModelViewHost (#1906)

Author: glennawatson

src/ReactiveUI.Tests/Platforms/xaml/ActivationForViewFetcherTest.cs

@@ -59,19 +59,20 @@ public void IsHitTestVisibleActivatesFrameworkElement()
 
             uc.RaiseEvent(loaded);
 
-            // IsHitTestVisible still false
-            Array.Empty<bool>().AssertAreEqual(activated);
+            // Loaded has happened.
+            new[] { true }.AssertAreEqual(activated);
 
             uc.IsHitTestVisible = true;
 
-            // IsHitTestVisible true
+            // IsHitTestVisible true, we don't want the event to repeat unnecessarily.
             new[] { true }.AssertAreEqual(activated);
 
             var unloaded = new RoutedEventArgs();
             unloaded.RoutedEvent = FrameworkElement.UnloadedEvent;
 
             uc.RaiseEvent(unloaded);
 
+            // We had both a loaded/hit test visible change/unloaded happen.
             new[] { true, false }.AssertAreEqual(activated);
         }
 

src/ReactiveUI.Wpf/TransitioningContentControl.cs

@@ -250,20 +250,12 @@ private void OnTransitionCompleted(object sender, EventArgs e)
         {
             AbortTransition();
 
-            var handler = TransitionCompleted;
-            if (handler != null)
-            {
-                handler(this, new RoutedEventArgs());
-            }
+            TransitionCompleted?.Invoke(this, new RoutedEventArgs());
         }
 
         private void RaiseTransitionStarted()
         {
-            var handler = TransitionStarted;
-            if (handler != null)
-            {
-                handler(this, new RoutedEventArgs());
-            }
+            TransitionStarted?.Invoke(this, new RoutedEventArgs());
         }
 
         private void QueueTransition(object oldContent, object newContent)
@@ -282,8 +274,7 @@ private void QueueTransition(object oldContent, object newContent)
                     {
                         // Wire up the completion transition.
                         var transitionInName = Transition + "Transition_" + TransitionPartType.In;
-                        var transitionIn = GetTransitionStoryboardByName(transitionInName);
-                        CompletingTransition = transitionIn;
+                        CompletingTransition = GetTransitionStoryboardByName(transitionInName);
 
                         // Wire up the first transition to start the second transition when it's complete.
                         startingTransitionName = Transition + "Transition_" + TransitionPartType.Out;
@@ -294,8 +285,7 @@ private void QueueTransition(object oldContent, object newContent)
                     else
                     {
                         startingTransitionName = Transition + "Transition_" + TransitionPart;
-                        var transitionIn = GetTransitionStoryboardByName(startingTransitionName);
-                        CompletingTransition = transitionIn;
+                        CompletingTransition = GetTransitionStoryboardByName(startingTransitionName);
                     }
 
                     // Start the transition.

src/ReactiveUI/Platforms/windows-common/ActivationForViewFetcher.cs

@@ -40,21 +40,30 @@ public IObservable<bool> GetActivationForView(IActivatable view)
 #if WINDOWS_UWP
             var viewLoaded = WindowsObservable.FromEventPattern<FrameworkElement, object>(
                 x => fe.Loading += x,
-                x => fe.Loading -= x).Select(_ => true);
+                x => fe.Loading -= x)
+                .Select(_ => true);
+
+            var hitTestVisible = fe.WhenAnyValue(x => x.IsHitTestVisible);
 #else
             var viewLoaded = Observable.FromEventPattern<RoutedEventHandler, RoutedEventArgs>(
                 x => fe.Loaded += x,
-                x => fe.Loaded -= x).Select(_ => true);
+                x => fe.Loaded -= x)
+                .Select(_ => true);
+
+            var hitTestVisible = Observable.FromEventPattern<DependencyPropertyChangedEventHandler, DependencyPropertyChangedEventArgs>(
+                x => fe.IsHitTestVisibleChanged += x,
+                x => fe.IsHitTestVisibleChanged -= x)
+                .Select(x => (bool)x.EventArgs.NewValue);
 #endif
 
             var viewUnloaded = Observable.FromEventPattern<RoutedEventHandler, RoutedEventArgs>(
                 x => fe.Unloaded += x,
-                x => fe.Unloaded -= x).Select(_ => false);
+                x => fe.Unloaded -= x)
+                .Select(_ => false);
 
             return viewLoaded
                 .Merge(viewUnloaded)
-                .Select(b => b ? fe.WhenAnyValue(x => x.IsHitTestVisible).SkipWhile(x => !x) : Observables.False)
-                .Switch()
+                .Merge(hitTestVisible)
                 .DistinctUntilChanged();
         }
     }

src/ReactiveUI/Platforms/windows-common/AutoDataTemplateBindingHook.cs

@@ -38,7 +38,7 @@ public class AutoDataTemplateBindingHook : IPropertyBindingHook
 #else
             const string template = "<DataTemplate xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation' " +
                      "xmlns:xaml='clr-namespace:ReactiveUI;assembly=__ASSEMBLYNAME__'> " +
-                 "<xaml:ViewModelViewHost ViewModel=\"{Binding}\" VerticalContentAlignment=\"Stretch\" HorizontalContentAlignment=\"Stretch\" IsTabStop=\"False\" />" +
+                 "<xaml:ViewModelViewHost ViewModel=\"{Binding Mode=OneWay}\" VerticalContentAlignment=\"Stretch\" HorizontalContentAlignment=\"Stretch\" IsTabStop=\"False\" />" +
              "</DataTemplate>";
 
             var assemblyName = typeof(AutoDataTemplateBindingHook).Assembly.FullName;

src/ReactiveUI/Platforms/windows-common/ViewModelViewHost.cs

@@ -5,6 +5,7 @@
 
 using System;
 using System.Reactive;
+using System.Reactive.Disposables;
 using System.Reactive.Linq;
 using System.Reactive.Subjects;
 using System.Windows;
@@ -27,22 +28,22 @@ namespace ReactiveUI
     public class ViewModelViewHost : TransitioningContentControl, IViewFor, IEnableLogger, IDisposable
     {
         /// <summary>
-        /// The view model dependency property.
+        /// The default content dependency property.
         /// </summary>
-        public static readonly DependencyProperty ViewModelProperty =
-            DependencyProperty.Register("ViewModel", typeof(object), typeof(ViewModelViewHost), new PropertyMetadata(null, SomethingChanged));
+        public static readonly DependencyProperty DefaultContentProperty =
+            DependencyProperty.Register(nameof(DefaultContent), typeof(object), typeof(ViewModelViewHost), new PropertyMetadata(null));
 
         /// <summary>
-        /// The default content dependency property.
+        /// The view model dependency property.
         /// </summary>
-        public static readonly DependencyProperty DefaultContentProperty =
-            DependencyProperty.Register("DefaultContent", typeof(object), typeof(ViewModelViewHost), new PropertyMetadata(null, SomethingChanged));
+        public static readonly DependencyProperty ViewModelProperty =
+            DependencyProperty.Register(nameof(ViewModel), typeof(object), typeof(ViewModelViewHost), new PropertyMetadata(null, SomethingChanged));
 
         /// <summary>
         /// The view contract observable dependency property.
         /// </summary>
         public static readonly DependencyProperty ViewContractObservableProperty =
-            DependencyProperty.Register("ViewContractObservable", typeof(IObservable<string>), typeof(ViewModelViewHost), new PropertyMetadata(Observable<string>.Default));
+            DependencyProperty.Register(nameof(ViewContractObservable), typeof(IObservable<string>), typeof(ViewModelViewHost), new PropertyMetadata(Observable<string>.Default, SomethingChanged));
 
         private readonly Subject<Unit> _updateViewModel = new Subject<Unit>();
         private string _viewContract;
@@ -57,17 +58,14 @@ public ViewModelViewHost()
             DefaultStyleKey = typeof(ViewModelViewHost);
 #endif
 
-            if (ModeDetector.InUnitTestRunner()) // NB: InUnitTestRunner also returns true in Design Mode
+            if (ModeDetector.InUnitTestRunner())
             {
                 ViewContractObservable = Observable<string>.Never;
+
+                // NB: InUnitTestRunner also returns true in Design Mode
                 return;
             }
 
-            var vmAndContract = Observable.CombineLatest(
-                this.WhenAnyValue(x => x.ViewModel),
-                this.WhenAnyObservable(x => x.ViewContractObservable),
-                (vm, contract) => new { ViewModel = vm, Contract = contract, });
-
             var platform = Locator.Current.GetService<IPlatformOperations>();
             Func<string> platformGetter = () => default(string);
 
@@ -87,32 +85,15 @@ public ViewModelViewHost()
                 .StartWith(platformGetter())
                 .DistinctUntilChanged();
 
-            this.WhenActivated(d =>
-            {
-                d(vmAndContract.Subscribe(x =>
-                {
-                    if (x.ViewModel == null)
-                    {
-                        Content = DefaultContent;
-                        return;
-                    }
-
-                    var viewLocator = ViewLocator ?? ReactiveUI.ViewLocator.Current;
-                    var view = viewLocator.ResolveView(x.ViewModel, x.Contract) ?? viewLocator.ResolveView(x.ViewModel, null);
-
-                    if (view == null)
-                    {
-                        throw new Exception($"Couldn't find view for '{x.ViewModel}'.");
-                    }
-
-                    view.ViewModel = x.ViewModel;
-                    Content = view;
-                }));
-
-                d(this.WhenAnyObservable(x => x.ViewContractObservable)
-                    .ObserveOn(RxApp.MainThreadScheduler)
-                    .Subscribe(x => _viewContract = x));
-            });
+            var contractChanged = _updateViewModel.Select(_ => ViewContractObservable).Switch();
+            var viewModelChanged = _updateViewModel.Select(_ => ViewModel);
+
+            var vmAndContract = contractChanged.CombineLatest(viewModelChanged, (contract, vm) => new { ViewModel = vm, Contract = contract });
+
+            vmAndContract.Subscribe(x => ResolveViewForViewModel(x.ViewModel, x.Contract));
+            contractChanged
+                .ObserveOn(RxApp.MainThreadScheduler)
+                .Subscribe(x => _viewContract = x);
         }
 
         /// <summary>
@@ -186,5 +167,26 @@ private static void SomethingChanged(DependencyObject dependencyObject, Dependen
         {
             ((ViewModelViewHost)dependencyObject)._updateViewModel.OnNext(Unit.Default);
         }
+
+        private void ResolveViewForViewModel(object viewModel, string contract)
+        {
+            if (viewModel == null)
+            {
+                Content = DefaultContent;
+                return;
+            }
+
+            var viewLocator = ViewLocator ?? ReactiveUI.ViewLocator.Current;
+            var viewInstance = viewLocator.ResolveView(viewModel, contract) ?? viewLocator.ResolveView(viewModel, null);
+
+            if (viewInstance == null)
+            {
+                throw new Exception($"The {nameof(ViewModelViewHost)} could not find a valid view for the view model of type {viewModel.GetType()} and value {viewModel}.");
+            }
+
+            viewInstance.ViewModel = viewModel;
+
+            Content = viewInstance;
+        }
     }
 }

src/ReactiveUI/View/DefaultViewLocator.cs

@@ -132,15 +132,13 @@ private static Type ToggleViewModelType(Type viewModelType)
                 if (viewModelType.Name.StartsWith("I", StringComparison.InvariantCulture))
                 {
                     var toggledTypeName = DeinterfaceifyTypeName(viewModelTypeName);
-                    var toggledType = Reflection.ReallyFindType(toggledTypeName, throwOnFailure: false);
-                    return toggledType;
+                    return Reflection.ReallyFindType(toggledTypeName, throwOnFailure: false);
                 }
             }
             else
             {
                 var toggledTypeName = InterfaceifyTypeName(viewModelTypeName);
-                var toggledType = Reflection.ReallyFindType(toggledTypeName, throwOnFailure: false);
-                return toggledType;
+                return Reflection.ReallyFindType(toggledTypeName, throwOnFailure: false);
             }
 
             return null;
@@ -215,6 +213,8 @@ private IViewFor AttemptViewResolution(string viewTypeName, string contract)
                     return null;
                 }
 
+                this.Log().Debug("Resolved service type '{0}'", viewType.FullName);
+
                 return view;
             }
             catch (Exception ex)