-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Clear PopupFlyout target on flyout close #15466
Conversation
You can test this PR using the following package version. |
ebfdf49
to
44fa44f
Compare
44fa44f
to
344b3ec
Compare
You can test this PR using the following package version. |
I don't think you can do this. |
Keep in mind https://github.com/AvaloniaUI/Avalonia/blob/master/src/Avalonia.Controls/Button.cs#L675-L692 and this usage is still valid, because in the event Target is already set. usages of |
So we have a different issue here too. Target is from UWP Flyouts and PlacementTarget is from WPF Popups. We really shouldn't have both and one needs to be obsoleted. Both properties should really behave the same too. (Havnt looked at code to see how PlacementTarget handles this). |
@robloo it's a bit simpler here. Flyout doesn't have PlacementTarget property. It's an argument of |
Double checked, WinUI behaves this way as I expected:
On a Button with ContextFlyout |
You can test this PR using the following package version. |
* Clear PopupFlyout target on flyout close * Clear Target after OnClose is called
What does the pull request do?
Flyouts hold a strong reference to target object: #15465 leaking memory. Luckily when the same menu is opened on another target, the memory is released. Still, this is a bug.
What is the current behavior?
What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
The Target in PopupFlyoutBase will be cleared on popup close. I am not sure if this changes any behaviour or not.
Checklist
Breaking changes
Not sure if this changes flyouts behaviour or not.
Obsoletions / Deprecations
Fixed issues
Fixes #15465