Update so it works to remove a single event vs. only one when hooked …

…to the same delegate.
picoe · Jun 27, 2024 · f9b4a85 · f9b4a85
1 parent 19baf47
commit f9b4a85
Show file tree

Hide file tree

Showing 3 changed files with 210 additions and 37 deletions.
diff --git a/src/Eto/Forms/Binding/Binding.helpers.cs b/src/Eto/Forms/Binding/Binding.helpers.cs
@@ -189,13 +189,18 @@ public static IndirectBinding<TValue> Property<TValue>(string propertyName, bool
 	/// <param name="obj">INotifyPropertyChanged object to attach the event handler to</param>
 	/// <param name="propertyName">Name of the property to trigger the changed event.</param>
 	/// <param name="eh">Event handler delegate to trigger when the specified property changes</param>
-	/// <seealso cref="RemovePropertyEvent"/>
-	public static void AddPropertyEvent(object obj, string propertyName, EventHandler<EventArgs> eh)
+	/// <returns>Object to pass to RemovePropertyEvent to unregister the event</returns>
+	/// <seealso cref="RemovePropertyEvent(object,EventHandler{EventArgs})"/>
+	/// <seealso cref="RemovePropertyEvent(object,string,EventHandler{EventArgs})"/>
+	public static object AddPropertyEvent(object obj, string propertyName, EventHandler<EventArgs> eh)
 	{
-		if (obj is not INotifyPropertyChanged notifyObject)
-			return;
-
-		notifyObject.PropertyChanged += (s, e) => OnPropertyChanged(s, e, propertyName, eh);
+		if (obj is INotifyPropertyChanged notifyObject)
+		{
+			var helper = new PropertyNotifyHelper(notifyObject, propertyName);
+			helper.Changed += eh;
+			return helper;
+		}
+		return null;
 	}
 
 	/// <summary>
@@ -209,45 +214,99 @@ public static void AddPropertyEvent(object obj, string propertyName, EventHandle
 	/// <param name="obj">INotifyPropertyChanged object to attach the event handler to</param>
 	/// <param name="propertyExpression">Expression to the property to trigger the changed event.</param>
 	/// <param name="eh">Event handler delegate to trigger when the specified property changes</param>
-	/// <seealso cref="RemovePropertyEvent"/>
-	public static void AddPropertyEvent<T, TProperty>(T obj, Expression<Func<T, TProperty>> propertyExpression, EventHandler<EventArgs> eh)
+	/// <returns>Object to pass to RemovePropertyEvent to unregister the event</returns>
+	/// <seealso cref="RemovePropertyEvent(object,EventHandler{EventArgs})"/>
+	/// <seealso cref="RemovePropertyEvent{T,TProperty}(T,Expression{Func{T, TProperty}},EventHandler{EventArgs})"/>
+	public static object AddPropertyEvent<T, TProperty>(T obj, Expression<Func<T, TProperty>> propertyExpression, EventHandler<EventArgs> eh)
 	{
 		var propertyInfo = propertyExpression.GetMemberInfo();
-		if (propertyInfo == null)
-			return;
-
-		AddPropertyEvent(obj, propertyInfo.Member.Name, eh);
+		if (propertyInfo != null)
+		{
+			return AddPropertyEvent(obj, propertyInfo.Member.Name, eh);
+		}
+		return null;
 	}
 
 	/// <summary>
 	/// Removes an event handler previously attached with the AddPropertyEvent method.
 	/// </summary>
-	/// <param name="obj">INotifyPropertyChanged object to remove the event handler from</param>
+	/// <remarks>
+	/// Note that if you pass the object that the event is attached to instea of the object returned from AddPropertyEvent,
+	/// then this will unsubscribe from all property handlers that point to the same delegate specified by <paramref name="eh"/>
+	/// </remarks>
+	/// <param name="obj">Object returned from AddPropertyEvent to unsubscribe, or the object the event is subscribed to</param>
 	/// <param name="eh">Event handler delegate to remove</param>
 	/// <seealso cref="AddPropertyEvent(object,string,EventHandler{EventArgs})"/>
 	public static void RemovePropertyEvent(object obj, EventHandler<EventArgs> eh)
 	{
-		if (obj is not INotifyPropertyChanged notifyObject)
-			return;
-
-		var propertyChangedField = GetPropertyChangedField(notifyObject);
-		if (propertyChangedField == null)
-			return;
-
-		var propertyChangedDelegates = ((Delegate)propertyChangedField.GetValue(notifyObject))?.GetInvocationList().OfType<PropertyChangedEventHandler>() ?? Enumerable.Empty<PropertyChangedEventHandler>();
-		var delegateToRemove = propertyChangedDelegates.FirstOrDefault(d => d.Target?.GetType().GetField("eh")?.GetValue(d.Target)?.Equals(eh) ?? false);
-		if (delegateToRemove == null)
-			return;
+		if (obj is PropertyNotifyHelper helper)
+		{
+			helper.Changed -= eh;
+			helper.Unregister(obj);
+		}
+		else if (obj is INotifyPropertyChanged notifyObject)
+		{
+			var propertyChangedField = GetPropertyChangedField(notifyObject);
+			if (propertyChangedField == null)
+				return;
 
-		notifyObject.PropertyChanged -= delegateToRemove;
+			var propertyChangedDelegates = ((Delegate)propertyChangedField.GetValue(notifyObject))?.GetInvocationList().OfType<PropertyChangedEventHandler>() ?? Enumerable.Empty<PropertyChangedEventHandler>();
+			foreach (var del in propertyChangedDelegates)
+			{
+				// find ones hooked up to the PropertyNotifyHelper, regardless of property
+				if (del.Target is PropertyNotifyHelper h && h.IsHookedTo(eh))
+				{
+					h.Changed -= eh;
+					h.Unregister(obj);
+				}
+			}
+		}
 	}
-
-	static void OnPropertyChanged(object sender, PropertyChangedEventArgs args, string propertyName, EventHandler<EventArgs> handler)
+
+	/// <summary>
+	/// Removes an event handler previously attached with the AddPropertyEvent method.
+	/// </summary>
+	/// <param name="obj">Object returned from AddPropertyEvent to unsubscribe</param>
+	/// <param name="propertyExpression">Expression for the property to remove the event handler for</param>
+	/// <param name="eh">Event handler delegate to remove</param>
+	/// <seealso cref="AddPropertyEvent{T,TProperty}(T,Expression{Func{T, TProperty}},EventHandler{EventArgs})"/>
+	public static void RemovePropertyEvent<T, TProperty>(T obj, Expression<Func<T, TProperty>> propertyExpression, EventHandler<EventArgs> eh)
 	{
-		if (args.PropertyName != propertyName)
-			return;
+		var propertyInfo = propertyExpression.GetMemberInfo();
+		if (propertyInfo != null)
+		{
+			RemovePropertyEvent(obj, propertyInfo.Member.Name, eh);
+		}
+	}
+
+	/// <summary>
+	/// Removes an event handler previously attached with the AddPropertyEvent method.
+	/// </summary>
+	/// <param name="obj">Object returned from AddPropertyEvent to unsubscribe</param>
+	/// <param name="propertyName">Name of the property to remove the event handler for</param>
+	/// <param name="eh">Event handler delegate to remove</param>
+	/// <seealso cref="AddPropertyEvent(object,string,EventHandler{EventArgs})"/>
+	public static void RemovePropertyEvent(object obj, string propertyName, EventHandler<EventArgs> eh)
+	{
+		if (obj is INotifyPropertyChanged notifyObject)
+		{
+			// this only works when PropertyChanged is a simple public event
+			// if it is implemented explicitly via the interface this won't work
+			var propertyChangedField = GetPropertyChangedField(notifyObject);
+			if (propertyChangedField == null)
+				return;
 
-		handler?.Invoke(sender, EventArgs.Empty);
+			var propertyChangedDelegates = ((Delegate)propertyChangedField.GetValue(notifyObject))?.GetInvocationList().OfType<PropertyChangedEventHandler>() ?? Enumerable.Empty<PropertyChangedEventHandler>();
+			foreach (var del in propertyChangedDelegates)
+			{
+				// find ones hooked up to the PropertyNotifyHelper
+				if (del.Target is PropertyNotifyHelper h && h.PropertyName == propertyName && h.IsHookedTo(eh))
+				{
+					h.Changed -= eh;
+					h.Unregister(obj);
+				}
+			}
+		}
 	}
 
 	static FieldInfo GetPropertyChangedField(object obj)

diff --git a/src/Eto/Forms/Binding/PropertyNotifyHelper.cs b/src/Eto/Forms/Binding/PropertyNotifyHelper.cs
@@ -0,0 +1,49 @@
+namespace Eto.Forms;
+
+/// <summary>
+/// Helper to turn a property changed event to an EventHandler for binding
+/// </summary>
+/// <remarks>
+/// Use <see cref="Binding.AddPropertyEvent"/> and <see cref="Binding.RemovePropertyEvent(object,string,EventHandler{EventArgs})"/> to access
+/// this functionality.
+/// </remarks>
+class PropertyNotifyHelper
+{
+	public string PropertyName { get; private set; }
+
+	public event EventHandler<EventArgs> Changed;
+
+	public PropertyNotifyHelper(INotifyPropertyChanged obj, string propertyName)
+	{
+		PropertyName = propertyName;
+		obj.PropertyChanged += obj_PropertyChanged;
+	}
+
+	public void Unregister(object obj)
+	{
+		var notifyObject = obj as INotifyPropertyChanged;
+		if (notifyObject != null)
+			notifyObject.PropertyChanged -= obj_PropertyChanged;
+	}
+
+	void obj_PropertyChanged(object sender, PropertyChangedEventArgs e)
+	{
+		if (e.PropertyName == PropertyName)
+		{
+			if (Changed != null)
+				Changed(sender, EventArgs.Empty);
+		}
+	}
+
+	public bool IsHookedTo(EventHandler<EventArgs> eh)
+	{
+		foreach (var invocation in Changed.GetInvocationList())
+		{
+			if (invocation == (Delegate)eh)
+			{
+				return true;
+			}
+		}
+		return false;
+	}
+}
diff --git a/test/Eto.Test/UnitTests/Forms/Bindings/BindingHelpersTests.cs b/test/Eto.Test/UnitTests/Forms/Bindings/BindingHelpersTests.cs
@@ -86,7 +86,7 @@ public void RemovingAPropertyEventShouldWork()
 		void Handler(object sender, EventArgs e) => propertyValueChanged = true;
 
 		var bindObject = new BindObject { BoolProperty = true };
-		Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, Handler);
+		var obj = Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, Handler);
 
 		// Act
 		Binding.RemovePropertyEvent(bindObject, Handler);
@@ -107,9 +107,9 @@ public void RemovingAPropertyEventShouldKeepOtherEvents()
 		void StringHandler(object sender, EventArgs e) => stringPropertyValueChanged = true;
 
 		var bindObject = new BindObject { BoolProperty = true, IntProperty = 3, StringProperty = "Test1" };
-		Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, BoolHandler);
-		Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, IntHandler);
-		Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, StringHandler);
+		var boolObj = Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, BoolHandler);
+		var intObj = Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, IntHandler);
+		var stringObj = Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, StringHandler);
 
 		// Act
 		Binding.RemovePropertyEvent(bindObject, IntHandler);
@@ -134,9 +134,9 @@ public void RemovingAllPropertyEventsShouldWork()
 		void StringHandler(object sender, EventArgs e) => stringPropertyValueChanged = true;
 
 		var bindObject = new BindObject { BoolProperty = true, IntProperty = 3, StringProperty = "Test1" };
-		Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, BoolHandler);
-		Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, IntHandler);
-		Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, StringHandler);
+		var boolObj = Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, BoolHandler);
+		var intObj = Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, IntHandler);
+		var stringObj = Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, StringHandler);
 
 		// Act
 		Binding.RemovePropertyEvent(bindObject, StringHandler);
@@ -151,4 +151,69 @@ public void RemovingAllPropertyEventsShouldWork()
 		Assert.That(intPropertyValueChanged, Is.False);
 		Assert.That(stringPropertyValueChanged, Is.False);
 	}
+
+	[Test]
+	public void UnsubscribingToSameEventShouldntUnsubscribeAllWhenPassingProperty()
+	{
+		var numchanged = 0;
+		void Handler(object sender, EventArgs e) => numchanged++;
+
+		var bindObject = new BindObject { BoolProperty = true, IntProperty = 3, StringProperty = "Test1" };
+		Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, Handler);
+		Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, Handler);
+		Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, Handler);
+
+		// Act
+		Binding.RemovePropertyEvent(bindObject, obj => obj.BoolProperty, Handler);
+		bindObject.BoolProperty = false;
+		bindObject.IntProperty = 4;
+		bindObject.StringProperty = "Test2";
+
+		// Assert
+		Assert.That(numchanged, Is.EqualTo(2));
+	}
+
+	[Test]
+	public void UnsubscribingToSameEventWillUnsubscribeAll()
+	{
+		var numchanged = 0;
+		void Handler(object sender, EventArgs e) => numchanged++;
+
+		var bindObject = new BindObject { BoolProperty = true, IntProperty = 3, StringProperty = "Test1" };
+		Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, Handler);
+		Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, Handler);
+		Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, Handler);
+
+		// Act
+		Binding.RemovePropertyEvent(bindObject, Handler);
+		bindObject.BoolProperty = false;
+		bindObject.IntProperty = 4;
+		bindObject.StringProperty = "Test2";
+
+		// Assert
+		// ambiguous which one to remove, so it removed all -- need to specify property or return value from AddPropertyEvent
+		Assert.That(numchanged, Is.EqualTo(0));
+	}
+
+	[Test]
+	public void UnsubscribingToAReferencedEventShouldWork()
+	{
+		var numchanged = 0;
+		void Handler(object sender, EventArgs e) => numchanged++;
+
+		var bindObject = new BindObject { BoolProperty = true, IntProperty = 3, StringProperty = "Test1" };
+		var boolObj = Binding.AddPropertyEvent(bindObject, obj => obj.BoolProperty, Handler);
+		Binding.AddPropertyEvent(bindObject, obj => obj.IntProperty, Handler);
+		Binding.AddPropertyEvent(bindObject, obj => obj.StringProperty, Handler);
+
+		// Act
+		Binding.RemovePropertyEvent(boolObj, Handler);
+		bindObject.BoolProperty = false;
+		bindObject.IntProperty = 4;
+		bindObject.StringProperty = "Test2";
+
+		// Assert
+		Assert.That(numchanged, Is.EqualTo(2));
+	}
+
 }