Merge pull request picoe#2259 from cwensley/curtis/mac-grid-contextmenu-without-header

Mac: Fix crash when setting context menu without header

Author: cwensley

src/Eto.Mac/Forms/Controls/GridHandler.cs

@@ -415,7 +415,7 @@ public bool ShowHeader
 			{
 				if (value && Control.HeaderView == null)
 				{
-					Control.HeaderView = headerView = new EtoTableHeaderView { Handler = this };
+					Control.HeaderView = headerView = new EtoTableHeaderView { Handler = this, Menu = ContextMenu.ToNS() };
 				}
 				else if (!value && Control.HeaderView != null)
 				{
@@ -437,7 +437,8 @@ public virtual ContextMenu ContextMenu
 			{
 				Widget.Properties.Set(GridHandler.ContextMenu_Key, value);
 				Control.Menu = value.ToNS();
-				Control.HeaderView.Menu = value.ToNS();
+				if (Control.HeaderView != null)
+					Control.HeaderView.Menu = value.ToNS();
 			}
 		}
 

src/Eto/Forms/Binding/BindableBinding.cs

@@ -78,13 +78,13 @@ public override DualBinding<TValue> Bind(DirectBinding<TValue> sourceBinding, Du
 			{
 				GettingNullValue = defaultControlValue,
 				SettingNullValue = defaultContextValue,
-				DataItem = contextBinding.DataValue
+				GetDataItem = () => contextBinding.DataValue // don't actually store the data context object in the binding
 			};
+			// don't trigger the value changes when we are currently changing the context
+			valueBinding.Changing += (sender, e) => e.Cancel = control.IsDataContextChanging;
+			contextBinding.DataValueChanged += (sender, e) => valueBinding.TriggerDataValueChanged();
+
 			DualBinding<TValue> binding = Bind(sourceBinding: valueBinding, mode: mode);
-			contextBinding.DataValueChanged += delegate
-			{
-				((ObjectBinding<object, TValue>)binding.Source).DataItem = contextBinding.DataValue;
-			};
 			control.Bindings.Add(contextBinding);
 			return binding;
 		}

src/Eto/Forms/Binding/BindableWidget.cs

@@ -99,7 +99,11 @@ internal protected set
 				if (Properties.TrySet(Parent_Key, value))
 				{
 					if (!HasDataContext && !(DataContext is null))
+					{
+						IsDataContextChanging = true;
 						OnDataContextChanged(EventArgs.Empty);
+						IsDataContextChanging = false;
+					}
 				}
 			}
 		}
@@ -193,12 +197,17 @@ public object DataContext
 			set
 			{
 				if (Properties.TrySet(DataContext_Key, value))
+				{
+					IsDataContextChanging = true;
 					OnDataContextChanged(EventArgs.Empty);
+					IsDataContextChanging = false;
+				}
 			}
 		}
 
 		internal bool HasDataContext => Properties.ContainsKey(DataContext_Key);
 
+		static readonly  object IsDataContextChanging_Key = new object();
 		static readonly  object Bindings_Key = new object();
 
 		/// <summary>
@@ -213,6 +222,22 @@ internal void TriggerDataContextChanged()
 			if (!HasDataContext)
 				OnDataContextChanged(EventArgs.Empty);
 		}
+		
+		/// <summary>
+		/// Gets a value indicating that the <see cref="DataContext"/> property is changing.
+		/// </summary>
+		/// <remarks>
+		/// This can be used to determine when to allow certain logic during the update of the data context.
+		/// 
+		/// It is used to disable binding setters on the model when the data context changes so that a binding
+		/// does not cause the view model to be updated when the state hasn't been fully set yet.
+		/// </remarks>
+		/// <value><c>true</c> if the DataContext is currently changing, <c>false</c> otherwise.</value>
+		public bool IsDataContextChanging
+		{
+			get => Properties.Get<bool?>(IsDataContextChanging_Key) ?? (Parent as IBindable)?.IsDataContextChanging ?? false;
+			set => Properties.Set(IsDataContextChanging_Key, value);
+		}
 
 		/// <summary>
 		/// Unbinds any bindings in the <see cref="Bindings"/> collection and removes the bindings

src/Eto/Forms/Binding/IBindable.cs

@@ -16,6 +16,18 @@ public interface IBindable
 		/// For example, a Control may return the data context of a parent, if it is not set explicitly.
 		/// </remarks>
 		object DataContext { get; set; }
+		
+		/// <summary>
+		/// Gets a value indicating that the <see cref="DataContext"/> property is changing.
+		/// </summary>
+		/// <remarks>
+		/// This can be used to determine when to allow certain logic during the update of the data context.
+		/// 
+		/// It is used to disable binding setters on the model when the data context changes so that a binding
+		/// does not cause the view model to be updated when the state hasn't been fully set yet.
+		/// </remarks>
+		/// <value><c>true</c> if the DataContext is currently changing, <c>false</c> otherwise.</value>
+		bool IsDataContextChanging { get; }
 
 		/// <summary>
 		/// Event to handle when the <see cref="DataContext"/> has changed

src/Eto/Forms/Binding/ObjectBinding.cs

@@ -103,22 +103,66 @@ internal override object InternalValue
 		public IndirectBinding<TValue> InnerBinding { get; private set; }
 
 		/// <summary>
-		/// Gets the object to get/set the values using the <see cref="InnerBinding"/>
+		/// Gets or sets the object to get/set the values using the <see cref="InnerBinding"/>
 		/// </summary>
+		/// <remarks>
+		/// This uses <see cref="GetDataItem"/> if set, otherwise it will use the set value. 
+		/// Setting the value explicitly will set <see cref="GetDataItem"/> to null.
+		/// </remarks>
 		public T DataItem
 		{
-			get { return dataItem; }
+			get
+			{
+				if (GetDataItem != null)
+					return GetDataItem();
+				return dataItem;
+			}
 			set
 			{
 				var hasValueChanged = dataValueChangedHandled;
 				if (hasValueChanged)
 					RemoveEvent(DataValueChangedEvent);
 				dataItem = value;
+				GetDataItem = null;
 				OnDataValueChanged(EventArgs.Empty);
 				if (hasValueChanged)
 					HandleEvent(DataValueChangedEvent);
 			}
 		}
+		
+		/// <summary>
+		/// Gets or sets a delegate used to get the current <see cref="DataItem"/> value instead of storing the value in this binding.
+		/// </summary>
+		/// <remarks>
+		/// This is used so that all bindings using a DataContext will be updated with the correct value as soon as the DataContext is set.
+		/// For example, if updating one binding would trigger the setter of a secondary binding that hasn't been updated with the new value yet
+		/// it would inadvertently change the DataContext even though it is now actually null.
+		/// 
+		/// Use the <see cref="TriggerDataValueChanged"/> method to specify that the value returned by this delegate has changed,
+		/// which will cause the binding to update all of its dependants.
+		/// 
+		/// Note if this is set, then it will always be used to retrieve the DataItem. If you set the DataItem directly, this will be set to null
+		/// and will no longer be used..
+		/// </remarks>
+		/// <value>The delegate used to get the current data item</value>
+		public Func<T> GetDataItem { get; set; }
+
+		/// <summary>
+		/// Triggers the <see cref="DirectBinding{T}.DataValueChanged"/> event.
+		/// </summary>
+		/// <remarks>
+		/// When using the <see cref="GetDataItem"/> delegate to retrieve the value of the DataItem, you can call this method to trigger
+		/// that its value has been changed.
+		/// </remarks>
+		public void TriggerDataValueChanged()
+		{
+			var hasValueChanged = dataValueChangedHandled;
+			if (hasValueChanged)
+				RemoveEvent(DataValueChangedEvent);
+			OnDataValueChanged(EventArgs.Empty);
+			if (hasValueChanged)
+				HandleEvent(DataValueChangedEvent);
+		}
 
 		/// <summary>
 		/// Gets or sets the default value to use when setting the value for this binding when input value is null

src/Eto/Forms/Command.cs

@@ -10,7 +10,7 @@ namespace Eto.Forms
 	/// </summary>
 	public class CheckCommand : Command, IValueCommand<bool>
 	{
-#region Events
+		#region Events
 
 		/// <summary>
 		/// Occurs when the <see cref="Checked"/> value has changed.
@@ -27,9 +27,9 @@ protected virtual void OnCheckedChanged(EventArgs e)
 				CheckedChanged(this, e);
 		}
 
-#endregion
+		#endregion
 
-#region Properties
+		#region Properties
 
 		bool ischecked;
 
@@ -50,7 +50,7 @@ public bool Checked
 			}
 		}
 
-#endregion
+		#endregion
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="Eto.Forms.CheckCommand"/> class.
@@ -157,7 +157,7 @@ public override ToolItem CreateToolItem()
 	/// </remarks>
 	public class Command : IBindable, ICommand
 	{
-#region Events
+		#region Events
 
 		/// <summary>
 		/// Occurs when the <see cref="Enabled"/> property is changed.
@@ -189,9 +189,9 @@ protected virtual void OnExecuted(EventArgs e)
 				Executed(this, e);
 		}
 
-#endregion
+		#endregion
 
-#region Properties
+		#region Properties
 
 		/// <summary>
 		/// Gets or sets the ID of the command
@@ -261,7 +261,7 @@ public virtual bool Enabled
 		/// <value>The command shortcut.</value>
 		public Keys Shortcut { get; set; }
 
-#endregion
+		#endregion
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="Eto.Forms.Command"/> class.
@@ -329,8 +329,8 @@ public static implicit operator ToolItem(Command command)
 		/// Gets the dictionary of properties for this widget
 		/// </summary>
 		public PropertyStore Properties
-		{ 
-			get { return properties ?? (properties = new PropertyStore(this)); } 
+		{
+			get { return properties ?? (properties = new PropertyStore(this)); }
 		}
 
 		static readonly object Command_Key = new object();
@@ -378,7 +378,7 @@ event EventHandler ICommand.CanExecuteChanged
 		}
 
 		bool ICommand.CanExecute(object parameter)
-		{ 
+		{
 			return Enabled;
 		}
 
@@ -387,7 +387,7 @@ void ICommand.Execute(object parameter)
 			Execute();
 		}
 
-#region IBindable implementation
+		#region IBindable implementation
 
 		/// <summary>
 		/// Event to handle when the <see cref="DataContext"/> has changed
@@ -423,7 +423,7 @@ public IBindable Parent
 		{
 			get { return Properties.Get<IBindable>(Parent_Key); }
 			set
-			{ 
+			{
 				var old = Parent;
 				Properties.Set(Parent_Key, value, () =>
 				{
@@ -432,7 +432,11 @@ public IBindable Parent
 					if (value != null)
 						value.DataContextChanged += Value_DataContextChanged;
 					if (!Properties.ContainsKey(DataContext_Key))
+					{
+						IsDataContextChanging = true;
 						OnDataContextChanged(EventArgs.Empty);
+						IsDataContextChanging = false;
+					}
 				});
 			}
 		}
@@ -462,10 +466,36 @@ public object DataContext
 					return bindable != null ? bindable.DataContext : null;
 				});
 			}
-			set { Properties.Set(DataContext_Key, value, () => OnDataContextChanged(EventArgs.Empty)); }
+			set
+			{
+				if (Properties.TrySet(DataContext_Key, value))
+				{
+					IsDataContextChanging = true;
+					OnDataContextChanged(EventArgs.Empty);
+					IsDataContextChanging = false;
+				}
+			}
+		}
+
+		static readonly object IsDataContextChanging_Key = new object();
+
+		/// <summary>
+		/// Gets a value indicating that the <see cref="DataContext"/> property is changing.
+		/// </summary>
+		/// <remarks>
+		/// This can be used to determine when to allow certain logic during the update of the data context.
+		/// 
+		/// It is used to disable binding setters on the model when the data context changes so that a binding
+		/// does not cause the view model to be updated when the state hasn't been fully set yet.
+		/// </remarks>
+		/// <value><c>true</c> if the DataContext is currently changing, <c>false</c> otherwise.</value>
+		public bool IsDataContextChanging
+		{
+			get => Properties.Get<bool?>(IsDataContextChanging_Key) ?? (Parent as IBindable)?.IsDataContextChanging ?? false;
+			set => Properties.Set(IsDataContextChanging_Key, value);
 		}
 
-		static readonly  object Bindings_Key = new object();
+		static readonly object Bindings_Key = new object();
 
 		/// <summary>
 		/// Gets the collection of bindings that are attached to this widget
@@ -475,6 +505,6 @@ public BindingCollection Bindings
 			get { return Properties.Create(Bindings_Key, () => new BindingCollection()); }
 		}
 
-#endregion
+		#endregion
 	}
 }

test/Eto.Test/UnitTests/Forms/DataContextTests.cs

@@ -285,7 +285,7 @@ public void DataContextChangeShouldFireForControlInStackLayout()
 			Shown(form =>
 			{
 				c = new Panel();
-				c.DataContextChanged += (sender, e) => 
+				c.DataContextChanged += (sender, e) =>
 					dataContextChanged++;
 
 				form.Content = new StackLayout
@@ -400,6 +400,44 @@ public void DataContextWithEqualsShouldSet()
 				Assert.AreEqual(3, changed);
 			});
 		}
+
+		class DropDownViewModel
+		{
+			public string[] DataSource { get; set; }
+
+			public int SelectedIndex { get; set; }
+		}
+
+		[Test]
+		public void ChangingDataContextShouldNotSetValues() => Shown(
+			form =>
+			{
+				var dropDown = new DropDown();
+				dropDown.BindDataContext(c => c.DataStore, (DropDownViewModel m) => m.DataSource);
+				dropDown.BindDataContext(c => c.SelectedIndex, (DropDownViewModel m) => m.SelectedIndex);
+				return dropDown;
+			},
+			dropDown =>
+			{
+				var model1 = new DropDownViewModel
+				{
+					DataSource = new string[] { "Item 1", "Item 2", "Item 3" },
+					SelectedIndex = 0
+				};
+				dropDown.DataContext = model1;
+				Assert.AreEqual(0, dropDown.SelectedIndex, "#1");
+
+				var model2 = new DropDownViewModel
+				{
+					DataSource = new string[] { "Item 4", "Item 5", "Item 6" },
+					SelectedIndex = 1
+				};
+				dropDown.DataContext = model2;
+				Assert.AreEqual(1, dropDown.SelectedIndex, "#2");
+
+				Assert.AreEqual(0, model1.SelectedIndex, "#3 - Model 1 was changed");
+				Assert.AreEqual(1, model2.SelectedIndex, "#4 - Model 2 was changed");
+			});
 	}
 }