diff --git a/MahApps.Metro/Controls/Flyout.cs b/MahApps.Metro/Controls/Flyout.cs
index 28478dfb37..df1025aa42 100644
--- a/MahApps.Metro/Controls/Flyout.cs
+++ b/MahApps.Metro/Controls/Flyout.cs
@@ -57,6 +57,9 @@ public event RoutedEventHandler ClosingFinished
public static readonly DependencyProperty CloseButtonVisibilityProperty = DependencyProperty.Register("CloseButtonVisibility", typeof(Visibility), typeof(Flyout), new FrameworkPropertyMetadata(Visibility.Visible));
public static readonly DependencyProperty TitleVisibilityProperty = DependencyProperty.Register("TitleVisibility", typeof(Visibility), typeof(Flyout), new FrameworkPropertyMetadata(Visibility.Visible));
+ internal PropertyChangeNotifier IsOpenPropertyChangeNotifier { get; set; }
+ internal PropertyChangeNotifier ThemePropertyChangeNotifier { get; set; }
+
///
/// Gets/sets if the title is visible in this flyout.
///
diff --git a/MahApps.Metro/Controls/FlyoutsControl.cs b/MahApps.Metro/Controls/FlyoutsControl.cs
index 4f59520590..ad6c7486ec 100644
--- a/MahApps.Metro/Controls/FlyoutsControl.cs
+++ b/MahApps.Metro/Controls/FlyoutsControl.cs
@@ -1,8 +1,6 @@
using System;
using System.Collections;
using System.Collections.Generic;
-using System.Collections.Specialized;
-using System.ComponentModel;
using System.Linq;
using System.Windows;
using System.Windows.Controls;
@@ -54,60 +52,21 @@ protected override bool IsItemItsOwnContainerOverride(object item)
return item is Flyout;
}
- protected override void OnItemsChanged(NotifyCollectionChangedEventArgs e)
+ protected override void PrepareContainerForItemOverride(DependencyObject element, object item)
{
- base.OnItemsChanged(e);
-
- switch (e.Action)
- {
- case NotifyCollectionChangedAction.Add:
- this.AttachHandlers(this.GetFlyouts(e.NewItems));
- break;
- case NotifyCollectionChangedAction.Replace:
- this.AttachHandlers(this.GetFlyouts(e.NewItems));
- this.DetachHandlers(this.GetFlyouts(e.OldItems));
- break;
- case NotifyCollectionChangedAction.Remove:
- this.DetachHandlers(this.GetFlyouts(e.OldItems));
- break;
- case NotifyCollectionChangedAction.Reset:
- List flyouts = this.GetFlyouts(this.Items).ToList();
- this.DetachHandlers(flyouts);
- this.AttachHandlers(flyouts);
- break;
- }
+ base.PrepareContainerForItemOverride(element, item);
+ this.AttachHandlers((Flyout)element);
}
- private void AttachHandlers(IEnumerable items)
+ private void AttachHandlers(Flyout flyout)
{
- foreach (var item in items)
- {
- this.AttachHandlers(item);
- }
- }
+ var isOpenNotifier = new PropertyChangeNotifier(flyout, Flyout.IsOpenProperty);
+ isOpenNotifier.ValueChanged += FlyoutStatusChanged;
+ flyout.IsOpenPropertyChangeNotifier = isOpenNotifier;
- private void AttachHandlers(Flyout item)
- {
- var isOpenChanged = DependencyPropertyDescriptor.FromProperty(Flyout.IsOpenProperty, typeof(Flyout));
- var themeChanged = DependencyPropertyDescriptor.FromProperty(Flyout.ThemeProperty, typeof(Flyout));
- isOpenChanged.AddValueChanged(item, this.FlyoutStatusChanged);
- themeChanged.AddValueChanged(item, this.FlyoutStatusChanged);
- }
-
- private void DetachHandlers(IEnumerable items)
- {
- foreach (var item in items)
- {
- this.DetachHandlers(item);
- }
- }
-
- private void DetachHandlers(Flyout item)
- {
- var isOpenChanged = DependencyPropertyDescriptor.FromProperty(Flyout.IsOpenProperty, typeof(Flyout));
- var themeChanged = DependencyPropertyDescriptor.FromProperty(Flyout.ThemeProperty, typeof(Flyout));
- isOpenChanged.RemoveValueChanged(item, this.FlyoutStatusChanged);
- themeChanged.RemoveValueChanged(item, this.FlyoutStatusChanged);
+ var themeNotifier = new PropertyChangeNotifier(flyout, Flyout.ThemeProperty);
+ themeNotifier.ValueChanged += FlyoutStatusChanged;
+ flyout.ThemePropertyChangeNotifier = themeNotifier;
}
private void FlyoutStatusChanged(object sender, EventArgs e)
diff --git a/MahApps.Metro/Controls/PropertyChangeNotifier.cs b/MahApps.Metro/Controls/PropertyChangeNotifier.cs
new file mode 100644
index 0000000000..c2ce7e0c0b
--- /dev/null
+++ b/MahApps.Metro/Controls/PropertyChangeNotifier.cs
@@ -0,0 +1,109 @@
+using System;
+using System.ComponentModel;
+using System.Windows;
+using System.Windows.Data;
+
+namespace MahApps.Metro.Controls
+{
+ ///
+ /// AddValueChanged of dependency property descriptor results in memory leak as you already know.
+ /// So, as described here, you can create custom class PropertyChangeNotifier to listen
+ /// to any dependency property changes.
+ ///
+ /// This class takes advantage of the fact that bindings use weak references to manage associations
+ /// so the class will not root the object who property changes it is watching. It also uses a WeakReference
+ /// to maintain a reference to the object whose property it is watching without rooting that object.
+ /// In this way, you can maintain a collection of these objects so that you can unhook the property
+ /// change later without worrying about that collection rooting the object whose values you are watching.
+ ///
+ /// Complete implementation can be found here: http://agsmith.wordpress.com/2008/04/07/propertydescriptor-addvaluechanged-alternative/
+ ///
+ internal sealed class PropertyChangeNotifier : DependencyObject, IDisposable
+ {
+ private WeakReference _propertySource;
+
+ public PropertyChangeNotifier(DependencyObject propertySource, string path)
+ : this(propertySource, new PropertyPath(path))
+ {
+ }
+
+ public PropertyChangeNotifier(DependencyObject propertySource, DependencyProperty property)
+ : this(propertySource, new PropertyPath(property))
+ {
+ }
+
+ public PropertyChangeNotifier(DependencyObject propertySource, PropertyPath property)
+ {
+ if (null == propertySource)
+ {
+ throw new ArgumentNullException("propertySource");
+ }
+ if (null == property)
+ {
+ throw new ArgumentNullException("property");
+ }
+ this._propertySource = new WeakReference(propertySource);
+ var binding = new Binding();
+ binding.Path = property;
+ binding.Mode = BindingMode.OneWay;
+ binding.Source = propertySource;
+ BindingOperations.SetBinding(this, ValueProperty, binding);
+ }
+
+ public DependencyObject PropertySource
+ {
+ get
+ {
+ try
+ {
+ // note, it is possible that accessing the target property
+ // will result in an exception so i’ve wrapped this check
+ // in a try catch
+ return this._propertySource.IsAlive
+ ? this._propertySource.Target as DependencyObject
+ : null;
+ }
+ catch
+ {
+ return null;
+ }
+ }
+ }
+
+ ///
+ /// Identifies the dependency property
+ ///
+ public static readonly DependencyProperty ValueProperty
+ = DependencyProperty.Register("Value", typeof(object), typeof(PropertyChangeNotifier),
+ new FrameworkPropertyMetadata(null, new PropertyChangedCallback(OnPropertyChanged)));
+
+ ///
+ /// Returns/sets the value of the property
+ ///
+ ///
+ [Description("Returns/sets the value of the property")]
+ [Category("Behavior")]
+ [Bindable(true)]
+ public object Value
+ {
+ get { return (object)this.GetValue(ValueProperty); }
+ set { this.SetValue(ValueProperty, value); }
+ }
+
+ private static void OnPropertyChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
+ {
+ var notifier = (PropertyChangeNotifier)d;
+ if (null != notifier.ValueChanged)
+ {
+ notifier.ValueChanged(notifier.PropertySource, EventArgs.Empty);
+ }
+ }
+
+ public event EventHandler ValueChanged;
+
+ public void Dispose()
+ {
+ BindingOperations.ClearBinding(this, ValueProperty);
+ }
+ }
+}
\ No newline at end of file
diff --git a/MahApps.Metro/MahApps.Metro.NET45.csproj b/MahApps.Metro/MahApps.Metro.NET45.csproj
index c4314098e1..ed1ff51213 100644
--- a/MahApps.Metro/MahApps.Metro.NET45.csproj
+++ b/MahApps.Metro/MahApps.Metro.NET45.csproj
@@ -171,6 +171,7 @@
+
diff --git a/MahApps.Metro/MahApps.Metro.csproj b/MahApps.Metro/MahApps.Metro.csproj
index 9929ede8cf..406d81223e 100644
--- a/MahApps.Metro/MahApps.Metro.csproj
+++ b/MahApps.Metro/MahApps.Metro.csproj
@@ -135,6 +135,7 @@
+
diff --git a/Mahapps.Metro.Tests/FlyoutTest.cs b/Mahapps.Metro.Tests/FlyoutTest.cs
index 6414f65e2a..08f1bb8bc5 100644
--- a/Mahapps.Metro.Tests/FlyoutTest.cs
+++ b/Mahapps.Metro.Tests/FlyoutTest.cs
@@ -1,7 +1,9 @@
-using System.Threading.Tasks;
+using System;
+using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Media;
+using System.Windows.Threading;
using ExposedObject;
using MahApps.Metro.Tests.TestHelpers;
using MahApps.Metro;
@@ -161,14 +163,14 @@ public async Task RaisesIsOpenChangedEvent()
bool eventRaised = false;
- window.RightFlyout.IsOpenChanged += (sender, args) =>
- {
+ window.RightFlyout.IsOpenChanged += (sender, args) => {
eventRaised = true;
};
window.RightFlyout.IsOpen = true;
- Assert.True(eventRaised);
+ // IsOpen fires IsOpenChangedEvent with DispatcherPriority.Background
+ window.RightFlyout.Dispatcher.BeginInvoke(DispatcherPriority.Background, new Action(() => Assert.True(eventRaised)));
}
public class ColorTest
diff --git a/docs/release-notes/1.0.0.md b/docs/release-notes/1.0.0.md
index a6371d7d34..617ccc7f61 100644
--- a/docs/release-notes/1.0.0.md
+++ b/docs/release-notes/1.0.0.md
@@ -43,3 +43,4 @@ A migration guide for the breaking changes is available here: https://github.com
- Fixed `MetroProgressBar` System.Windows.Media.Animation Warning 6 #1664
- Fixed opening animation of dynamically created flyouts #1665 #1655
- Fixed `WindowSettings` SaveWindowPosition saves to position 0,0 if window not shown #1671 #1672
+- Fixed memory leak and exception with dynamically created Flyouts (mostly happens with Caliburn) #1681