From a920bbd088c6baddfab7f22dcbd3a386f00c99da Mon Sep 17 00:00:00 2001 From: Michael Lamers Date: Sun, 5 Aug 2018 23:08:22 +0200 Subject: [PATCH] MvvmBehavior is less aggressive it takes into account that there might be a notify signal infrastructure set by other behaviors or the user and uses this signal when PropertyChanged gets triggered renamed MVVM to Mvvm --- src/net/Qml.Net.Tests/Qml/BaseQmlTests.cs | 6 +- ...orTests.cs => MvvmInteropBehaviorTests.cs} | 108 +++++++++++++++--- ...pBehavior.cs => MvvmQmlInteropBehavior.cs} | 75 +++++++++--- src/net/Qml.Net/QQmlApplicationEngine.cs | 2 +- 4 files changed, 160 insertions(+), 31 deletions(-) rename src/net/Qml.Net.Tests/Qml/{MVVMInteropBehaviorTests.cs => MvvmInteropBehaviorTests.cs} (55%) rename src/net/Qml.Net/Internal/Behaviors/{MVVMQmlInteropBehavior.cs => MvvmQmlInteropBehavior.cs} (51%) diff --git a/src/net/Qml.Net.Tests/Qml/BaseQmlTests.cs b/src/net/Qml.Net.Tests/Qml/BaseQmlTests.cs index 16b20cde..aabc65e9 100644 --- a/src/net/Qml.Net.Tests/Qml/BaseQmlTests.cs +++ b/src/net/Qml.Net.Tests/Qml/BaseQmlTests.cs @@ -119,14 +119,14 @@ namespace Qml.Net.Tests.Qml } } - public abstract class BaseQmlMVVMTestsWithInstance : BaseQmlTests where T : class, new() + public abstract class BaseQmlMvvmTestsWithInstance : BaseQmlTests where T : class, new() { protected readonly T Instance; - protected BaseQmlMVVMTestsWithInstance() + protected BaseQmlMvvmTestsWithInstance() { InteropBehaviors.ClearQmlInteropBehaviors(); - InteropBehaviors.RegisterQmlInteropBehavior(new MVVMQmlInteropBehavior()); + InteropBehaviors.RegisterQmlInteropBehavior(new MvvmQmlInteropBehavior()); RegisterType(); Instance = new T(); diff --git a/src/net/Qml.Net.Tests/Qml/MVVMInteropBehaviorTests.cs b/src/net/Qml.Net.Tests/Qml/MvvmInteropBehaviorTests.cs similarity index 55% rename from src/net/Qml.Net.Tests/Qml/MVVMInteropBehaviorTests.cs rename to src/net/Qml.Net.Tests/Qml/MvvmInteropBehaviorTests.cs index d8525fbc..38160974 100644 --- a/src/net/Qml.Net.Tests/Qml/MVVMInteropBehaviorTests.cs +++ b/src/net/Qml.Net.Tests/Qml/MvvmInteropBehaviorTests.cs @@ -1,12 +1,7 @@ -using System; -using System.Collections.Generic; -using System.ComponentModel; +using System.ComponentModel; using System.Runtime.CompilerServices; -using System.Text; using Xunit; using FluentAssertions; -using Qml.Net.Internal; -using Qml.Net.Internal.Behaviors; using Qml.Net.Internal.Qml; namespace Qml.Net.Tests.Qml @@ -25,7 +20,17 @@ namespace Qml.Net.Tests.Qml ViewModel.IntProperty = newValue; } - public bool? TestResult { get; set; } + public void ChangeCustomIntPropertyTo(int newValue) + { + ViewModel.CustomIntProperty = newValue; + } + + public void ChangeCustomMvvmStyleIntPropertyTo(int newValue) + { + ViewModel.CustomMvvmStyleIntProperty = newValue; + } + + public bool? TestResult { get; set; } = false; } public class ViewModel : INotifyPropertyChanged @@ -49,13 +54,10 @@ namespace Qml.Net.Tests.Qml } } - private int _intProperty = 0; + private int _intProperty; public int IntProperty { - get - { - return _intProperty; - } + get => _intProperty; set { if (!Equals(value, _intProperty)) @@ -65,6 +67,42 @@ namespace Qml.Net.Tests.Qml } } } + + private int _customIntProperty; + [NotifySignal("customIntPropertyChangedSignal")] + public int CustomIntProperty + { + get + { + return _customIntProperty; + } + set + { + if (!Equals(value, _customIntProperty)) + { + _customIntProperty = value; + FirePropertyChanged(); + } + } + } + + private int _customMvvmStyleIntProperty; + [NotifySignal] + public int CustomMvvmStyleIntProperty + { + get + { + return _customMvvmStyleIntProperty; + } + set + { + if (!Equals(value, _customMvvmStyleIntProperty)) + { + _customMvvmStyleIntProperty = value; + FirePropertyChanged(); + } + } + } private void FirePropertyChanged([CallerMemberName]string propertyName = "") { @@ -72,7 +110,7 @@ namespace Qml.Net.Tests.Qml } } - public class MVVMInteropBehaviorTests : BaseQmlMVVMTestsWithInstance + public class MvvmInteropBehaviorTests : BaseQmlMvvmTestsWithInstance { [Fact] public void Does_register_property_changed_signal() @@ -140,5 +178,49 @@ namespace Qml.Net.Tests.Qml Instance.TestResult.Should().Be(true); } + + [Fact] + public void Does_not_interfer_with_completely_custom_notify_signals() + { + NetTestHelper.RunQml(qmlApplicationEngine, + @" + import QtQuick 2.0 + import tests 1.0 + ViewModelContainer { + id: viewModelContainer + Component.onCompleted: function() { + var vm = viewModelContainer.viewModel + vm.customIntPropertyChangedSignal.connect(function() { + viewModelContainer.testResult = true + }) + viewModelContainer.changeCustomIntPropertyTo(3) + } + } + "); + + Instance.TestResult.Should().Be(true); + } + + [Fact] + public void Does_not_interfer_with_custom_notify_signals() + { + NetTestHelper.RunQml(qmlApplicationEngine, + @" + import QtQuick 2.0 + import tests 1.0 + ViewModelContainer { + id: viewModelContainer + Component.onCompleted: function() { + var vm = viewModelContainer.viewModel + vm.customMvvmStyleIntPropertyChanged.connect(function() { + viewModelContainer.testResult = true + }) + viewModelContainer.changeCustomMvvmStyleIntPropertyTo(3) + } + } + "); + + Instance.TestResult.Should().Be(true); + } } } diff --git a/src/net/Qml.Net/Internal/Behaviors/MVVMQmlInteropBehavior.cs b/src/net/Qml.Net/Internal/Behaviors/MvvmQmlInteropBehavior.cs similarity index 51% rename from src/net/Qml.Net/Internal/Behaviors/MVVMQmlInteropBehavior.cs rename to src/net/Qml.Net/Internal/Behaviors/MvvmQmlInteropBehavior.cs index 01183082..76c3fc85 100644 --- a/src/net/Qml.Net/Internal/Behaviors/MVVMQmlInteropBehavior.cs +++ b/src/net/Qml.Net/Internal/Behaviors/MvvmQmlInteropBehavior.cs @@ -5,8 +5,42 @@ using System.ComponentModel; namespace Qml.Net.Internal.Behaviors { - internal class MVVMQmlInteropBehavior : IQmlInteropBehavior + internal class MvvmQmlInteropBehavior : IQmlInteropBehavior { + private class MvvmPropertyInfo + { + public MvvmPropertyInfo(string name, string signalName) + { + Name = name; + SignalName = signalName; + } + + // ReSharper disable once MemberCanBePrivate.Local + // ReSharper disable once UnusedAutoPropertyAccessor.Local + public string Name { get; } + public string SignalName { get; } + } + + private class MvvmTypeInfo + { + private readonly Dictionary _propertyInfos = new Dictionary(); + + // ReSharper disable once UnusedMember.Local + public Type Type { get; set; } + + public void AddPropertyInfo(string propertyName, string signalName) + { + _propertyInfos.Add(propertyName, new MvvmPropertyInfo(propertyName, signalName)); + } + + public string GetPropertySignalName(string propertyName) + { + return !_propertyInfos.ContainsKey(propertyName) ? null : _propertyInfos[propertyName].SignalName; + } + } + + private static readonly Dictionary TypeInfos = new Dictionary(); + public bool IsApplicableFor(Type type) { return typeof(INotifyPropertyChanged).IsAssignableFrom(type); @@ -43,15 +77,23 @@ namespace Qml.Net.Internal.Behaviors private void PropertyChangedHandler(object sender, PropertyChangedEventArgs e) { - //fire signal according to the property that got changed - var signalName = CalculateSignalSignatureFromPropertyName(e.PropertyName); - if (signalName != null) + if (sender == null) { - sender.ActivateSignal(signalName); + return; + } + //fire signal according to the property that got changed + var type = sender.GetType(); + if (TypeInfos.TryGetValue(type, out var typeInfo)) + { + var signalName = typeInfo.GetPropertySignalName(e.PropertyName); + if (signalName != null) + { + sender.ActivateSignal(signalName); + } } } - private string CalculateSignalSignatureFromPropertyName(string propertyName) + private static string CalculateSignalNameFromPropertyName(string propertyName) { var result = $"{propertyName}Changed"; if (!char.IsLower(result[0])) @@ -67,12 +109,22 @@ namespace Qml.Net.Internal.Behaviors { return; } + var mvvmTypeInfo = new MvvmTypeInfo(); + TypeInfos.Add(forType, mvvmTypeInfo); for (var i = 0; i < netTypeInfo.PropertyCount; i++) { int? existingSignalIndex = null; var property = netTypeInfo.GetProperty(i); - var signalName = CalculateSignalSignatureFromPropertyName(property.Name); + if (property.NotifySignal != null) + { + //in this case some other behavior or the user has already set up a notify signal for this property + //we don't want to destroy that + mvvmTypeInfo.AddPropertyInfo(property.Name, property.NotifySignal.Name); + continue; + } + var signalName = CalculateSignalNameFromPropertyName(property.Name); + mvvmTypeInfo.AddPropertyInfo(property.Name, signalName); //check if this signal already has been registered for(var signalIndex = 0; signalIndex < netTypeInfo.SignalCount; signalIndex++) { @@ -85,13 +137,8 @@ namespace Qml.Net.Internal.Behaviors } if(existingSignalIndex.HasValue) { - //signal for this property is already existent - - //check if the property is linked to it - if(property.NotifySignal == null) - { - property.NotifySignal = netTypeInfo.GetSignal(existingSignalIndex.Value); - } + //signal for this property is already existent but not registered (we check that above) + property.NotifySignal = netTypeInfo.GetSignal(existingSignalIndex.Value); continue; } //create a new signal and link it to the property diff --git a/src/net/Qml.Net/QQmlApplicationEngine.cs b/src/net/Qml.Net/QQmlApplicationEngine.cs index 5d86f9c9..76fab75a 100644 --- a/src/net/Qml.Net/QQmlApplicationEngine.cs +++ b/src/net/Qml.Net/QQmlApplicationEngine.cs @@ -46,7 +46,7 @@ namespace Qml.Net /// public static void ActivateMVVMBehavior() { - InteropBehaviors.RegisterQmlInteropBehavior(new MVVMQmlInteropBehavior(), false); + InteropBehaviors.RegisterQmlInteropBehavior(new MvvmQmlInteropBehavior(), false); } public void AddImportPath(string path)