Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix FlyoutsControl memory leak and exception #1681

Merged
merged 5 commits into from
Nov 29, 2014

Conversation

punker76
Copy link
Member

Closes #1638 Update FlyoutsControl.cs

i'm using a <Controls:FlyoutsControl ItemsSource="{Binding Flyouts}" /> binded to an >ObservableCollection but when i remove items from Flyout Collection i get a Value can't be null >error.

Following the recent conversation with thoemmi in Gitter i changed FlyoutControl to check for null before >advancing

this pr uses PropertyChangeNotifier from this nice solution

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.

@punker76 punker76 added this to the v1.0 milestone Nov 28, 2014
///
/// Complete implementation can be found here: http://agsmith.wordpress.com/2008/04/07/propertydescriptor-addvaluechanged-alternative/
/// </summary>
public sealed class PropertyChangeNotifier : DependencyObject, IDisposable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be internal?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe, it's not only for mahapps...

punker76 added a commit that referenced this pull request Nov 29, 2014
fix FlyoutsControl memory leak and exception
@punker76 punker76 merged commit 944c7ed into master Nov 29, 2014
@punker76 punker76 deleted the punker76-flyoutcontrol-memory-leak branch December 1, 2014 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants