From d242e1b5dddc2654019c8c493bb4aa9b80a360fe Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 4 Aug 2020 16:20:17 +0200 Subject: [PATCH 1/3] Revert "Fix shared size group for fluent sub menu" This reverts commit 85c09761ba8dbb52bafa9d5486801ad7223b70cb. --- src/Avalonia.Controls/MenuItem.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 789e4f8926..912abc6de3 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -97,6 +97,7 @@ namespace Avalonia.Controls private ICommand _command; private bool _commandCanExecute = true; private Popup _popup; + private IDisposable _gridHack; /// /// Initializes static members of the class. @@ -322,6 +323,9 @@ namespace Avalonia.Controls { Command.CanExecuteChanged -= CanExecuteChanged; } + + _gridHack?.Dispose(); + _gridHack = null; } protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) @@ -341,7 +345,9 @@ namespace Avalonia.Controls // the WPF codebase: // // https://github.com/dotnet/wpf/blob/89537909bdf36bc918e88b37751add46a8980bb0/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/MenuItem.cs#L2126-L2141 - SetValue(DefinitionBase.PrivateSharedSizeScopeProperty, parent.GetValue(DefinitionBase.PrivateSharedSizeScopeProperty)); + _gridHack = Bind( + DefinitionBase.PrivateSharedSizeScopeProperty, + parent.GetBindingObservable(DefinitionBase.PrivateSharedSizeScopeProperty)); } } From 922c46bbc4d125a005164c942cc1af70729afd6a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 5 Aug 2020 14:38:30 +0200 Subject: [PATCH 2/3] Dispose the grid hack in the correct place. The grid hack was initiated when attached to the visual tree but the dispose had been placed in `OnDetachedFromLogicalTree` by mistake. Place it in `OnDetachedFromVisualTree`. --- src/Avalonia.Controls/MenuItem.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 912abc6de3..2a22896de1 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -323,9 +323,6 @@ namespace Avalonia.Controls { Command.CanExecuteChanged -= CanExecuteChanged; } - - _gridHack?.Dispose(); - _gridHack = null; } protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) @@ -351,6 +348,14 @@ namespace Avalonia.Controls } } + protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) + { + base.OnDetachedFromVisualTree(e); + + _gridHack?.Dispose(); + _gridHack = null; + } + /// /// Called when the is clicked. /// From db67fa2cf6c4918a99006528567e2db1c2e0ea26 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 5 Aug 2020 23:57:45 +0200 Subject: [PATCH 3/3] Tweak the MenuItem Grid hack. To try and fix menu layout in the fluent theme. --- src/Avalonia.Controls/MenuItem.cs | 59 ++++++++++++++----------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/src/Avalonia.Controls/MenuItem.cs b/src/Avalonia.Controls/MenuItem.cs index 2a22896de1..bfa2e81745 100644 --- a/src/Avalonia.Controls/MenuItem.cs +++ b/src/Avalonia.Controls/MenuItem.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reactive.Linq; using System.Windows.Input; using Avalonia.Controls.Generators; using Avalonia.Controls.Mixins; @@ -97,7 +98,6 @@ namespace Avalonia.Controls private ICommand _command; private bool _commandCanExecute = true; private Popup _popup; - private IDisposable _gridHack; /// /// Initializes static members of the class. @@ -119,6 +119,32 @@ namespace Avalonia.Controls public MenuItem() { + // HACK: This nasty but it's all WPF's fault. Grid uses an inherited attached + // property to store SharedSizeGroup state, except property inheritance is done + // down the logical tree. In this case, the control which is setting + // Grid.IsSharedSizeScope="True" is not in the logical tree. Instead of fixing + // the way Grid stores shared size state, the developers of WPF just created a + // binding of the internal state of the visual parent to the menu item. We don't + // have much choice but to do the same for now unless we want to refactor Grid, + // which I honestly am not brave enough to do right now. Here's the same hack in + // the WPF codebase: + // + // https://github.com/dotnet/wpf/blob/89537909bdf36bc918e88b37751add46a8980bb0/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/MenuItem.cs#L2126-L2141 + // + // In addition to the hack from WPF, we also make sure to return null when we have + // no parent. If we don't do this, inheritance falls back to the logical tree, + // causing the shared size scope in the parent MenuItem to be used, breaking + // menu layout. + + var parentSharedSizeScope = this.GetObservable(VisualParentProperty) + .SelectMany(x => + { + var parent = x as Control; + return parent?.GetObservable(DefinitionBase.PrivateSharedSizeScopeProperty) ?? + Observable.Return(null); + }); + + this.Bind(DefinitionBase.PrivateSharedSizeScopeProperty, parentSharedSizeScope); } /// @@ -325,37 +351,6 @@ namespace Avalonia.Controls } } - protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e) - { - base.OnAttachedToVisualTree(e); - - if (this.GetVisualParent() is IControl parent) - { - // HACK: This nasty but it's all WPF's fault. Grid uses an inherited attached - // property to store SharedSizeGroup state, except property inheritance is done - // down the logical tree. In this case, the control which is setting - // Grid.IsSharedSizeScope="True" is not in the logical tree. Instead of fixing - // the way Grid stores shared size state, the developers of WPF just created a - // binding of the internal state of the visual parent to the menu item. We don't - // have much choice but to do the same for now unless we want to refactor Grid, - // which I honestly am not brave enough to do right now. Here's the same hack in - // the WPF codebase: - // - // https://github.com/dotnet/wpf/blob/89537909bdf36bc918e88b37751add46a8980bb0/src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Controls/MenuItem.cs#L2126-L2141 - _gridHack = Bind( - DefinitionBase.PrivateSharedSizeScopeProperty, - parent.GetBindingObservable(DefinitionBase.PrivateSharedSizeScopeProperty)); - } - } - - protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e) - { - base.OnDetachedFromVisualTree(e); - - _gridHack?.Dispose(); - _gridHack = null; - } - /// /// Called when the is clicked. ///