From 3d1fabed156b05ae4a19b3443db8931314b4fcd6 Mon Sep 17 00:00:00 2001 From: Nathan Nguyen <146415969+NathanDrake2406@users.noreply.github.com> Date: Thu, 19 Mar 2026 05:29:47 +1100 Subject: [PATCH] Make FlyoutBase.IsOpen a public StyledProperty (#20920) * Make FlyoutBase.IsOpen a public StyledProperty with two-way binding support Convert IsOpen from a DirectProperty with a protected setter to a StyledProperty with a public setter and TwoWay default binding mode. This enables MVVM scenarios where a ViewModel can control flyout visibility through data binding. The implementation mirrors the established Popup.IsOpen pattern: - Reentrancy guard (BeginIgnoringIsOpen scope) prevents recursive property change notifications when internal code syncs the property - SetCurrentValue preserves active bindings and styles (enforced by analyzer AVP1012) - _isOpen field tracks actual open state independently of the property value, since the property system sets the value before the change handler fires - _lastPlacementTarget enables re-opening at the last known target when IsOpen is set to true via binding - IsOpen reverts to false when no target is available or opening is cancelled, and reverts to true when closing is cancelled, keeping the property honest Fixes #18716 * ci: retrigger checks * Add API suppression for FlyoutBase.IsOpenProperty type change Suppress CP0002 for the intentional binary breaking change from DirectProperty to StyledProperty. * Pre-register owning control as flyout placement target When Button.Flyout or SplitButton.Flyout is set, the owning control now registers itself as the default placement target via an internal SetDefaultPlacementTarget method. This allows IsOpen = true to work on first use without a prior ShowAt call, addressing review feedback from MrJul. * Remove TwoWay default binding mode from IsOpenProperty Follow Avalonia convention: Popup.IsOpen and ToolTip.IsOpen use the default OneWay binding mode. TwoWay is reserved for input controls. Users opt in with Mode=TwoWay when needed. --- api/Avalonia.nupkg.xml | 12 + src/Avalonia.Controls/Button.cs | 3 + src/Avalonia.Controls/Flyouts/FlyoutBase.cs | 19 +- .../Flyouts/PopupFlyoutBase.cs | 111 +++++++-- .../SplitButton/SplitButton.cs | 3 + .../FlyoutTests.cs | 220 +++++++++++++++++- 6 files changed, 346 insertions(+), 22 deletions(-) diff --git a/api/Avalonia.nupkg.xml b/api/Avalonia.nupkg.xml index 63e8234919..d215b13118 100644 --- a/api/Avalonia.nupkg.xml +++ b/api/Avalonia.nupkg.xml @@ -5545,4 +5545,16 @@ baseline/Avalonia/lib/netstandard2.0/Avalonia.Base.dll current/Avalonia/lib/netstandard2.0/Avalonia.Base.dll + + CP0002 + F:Avalonia.Controls.Primitives.FlyoutBase.IsOpenProperty + baseline/Avalonia/lib/net10.0/Avalonia.Controls.dll + current/Avalonia/lib/net10.0/Avalonia.Controls.dll + + + CP0002 + F:Avalonia.Controls.Primitives.FlyoutBase.IsOpenProperty + baseline/Avalonia/lib/net8.0/Avalonia.Controls.dll + current/Avalonia/lib/net8.0/Avalonia.Controls.dll + diff --git a/src/Avalonia.Controls/Button.cs b/src/Avalonia.Controls/Button.cs index ceadae432c..c094db57ec 100644 --- a/src/Avalonia.Controls/Button.cs +++ b/src/Avalonia.Controls/Button.cs @@ -543,10 +543,13 @@ namespace Avalonia.Controls oldFlyout.Hide(); } + (oldFlyout as PopupFlyoutBase)?.SetDefaultPlacementTarget(null); + // Must unregister events here while a reference to the old flyout still exists UnregisterFlyoutEvents(oldFlyout); RegisterFlyoutEvents(newFlyout); + (newFlyout as PopupFlyoutBase)?.SetDefaultPlacementTarget(this); UpdatePseudoClasses(); } } diff --git a/src/Avalonia.Controls/Flyouts/FlyoutBase.cs b/src/Avalonia.Controls/Flyouts/FlyoutBase.cs index b5328ccab8..b7de300e84 100644 --- a/src/Avalonia.Controls/Flyouts/FlyoutBase.cs +++ b/src/Avalonia.Controls/Flyouts/FlyoutBase.cs @@ -7,9 +7,8 @@ namespace Avalonia.Controls.Primitives /// /// Defines the property /// - public static readonly DirectProperty IsOpenProperty = - AvaloniaProperty.RegisterDirect(nameof(IsOpen), - x => x.IsOpen); + public static readonly StyledProperty IsOpenProperty = + AvaloniaProperty.Register(nameof(IsOpen)); /// /// Defines the property @@ -23,19 +22,23 @@ namespace Avalonia.Controls.Primitives public static readonly AttachedProperty AttachedFlyoutProperty = AvaloniaProperty.RegisterAttached("AttachedFlyout", null); - private bool _isOpen; private Control? _target; public event EventHandler? Opened; public event EventHandler? Closed; - + /// - /// Gets whether this Flyout is currently Open + /// Gets or sets whether this Flyout is currently open. /// + /// + /// Setting this property to true will show the flyout at the last known + /// placement target. If no target has been set via , + /// setting this to true will have no effect. + /// public bool IsOpen { - get => _isOpen; - protected set => SetAndRaise(IsOpenProperty, ref _isOpen, value); + get => GetValue(IsOpenProperty); + set => SetValue(IsOpenProperty, value); } /// diff --git a/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs b/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs index 19d1f52850..cea69524f1 100644 --- a/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs +++ b/src/Avalonia.Controls/Flyouts/PopupFlyoutBase.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.ComponentModel; using System.Linq; using Avalonia.Controls.Diagnostics; @@ -67,9 +67,14 @@ namespace Avalonia.Controls.Primitives private PixelRect? _enlargePopupRectScreenPixelRect; private IDisposable? _transientDisposable; private Action? _popupHostChangedHandler; + private bool _isOpen; + private bool _ignoreIsOpenChanged; + private Control? _lastPlacementTarget; static PopupFlyoutBase() { + IsOpenProperty.Changed.AddClassHandler( + (x, e) => x.IsOpenChanged((AvaloniaPropertyChangedEventArgs)e)); Control.ContextFlyoutProperty.Changed.Subscribe(OnContextFlyoutPropertyChanged); } @@ -136,9 +141,9 @@ namespace Avalonia.Controls.Primitives /// through to the parent window. /// /// - /// Clicks outside the popup cause the popup to close. When - /// is set to false, these clicks will be - /// handled by the popup and not be registered by the parent window. When set to true, + /// Clicks outside the popup cause the popup to close. When + /// is set to false, these clicks will be + /// handled by the popup and not be registered by the parent window. When set to true, /// the events will be passed through to the parent window. /// public bool OverlayDismissEventPassThrough @@ -175,6 +180,16 @@ namespace Avalonia.Controls.Primitives public event EventHandler? Closing; public event EventHandler? Opening; + /// + /// Pre-registers a control as the default placement target for this flyout. + /// Used by owning controls (e.g. ) so that setting + /// to true works on first use. + /// + internal void SetDefaultPlacementTarget(Control? target) + { + _lastPlacementTarget = target; + } + /// /// Shows the Flyout at the given Control /// @@ -205,7 +220,7 @@ namespace Avalonia.Controls.Primitives /// True, if action was handled protected virtual bool HideCore(bool canCancel = true) { - if (!IsOpen) + if (!_isOpen) { return false; } @@ -218,7 +233,11 @@ namespace Avalonia.Controls.Primitives } } - IsOpen = false; + _isOpen = false; + using (BeginIgnoringIsOpen()) + { + SetCurrentValue(IsOpenProperty, false); + } Popup.IsOpen = false; Popup.PlacementTarget = null; @@ -251,7 +270,9 @@ namespace Avalonia.Controls.Primitives throw new ArgumentNullException(nameof(placementTarget)); } - if (IsOpen) + _lastPlacementTarget = placementTarget; + + if (_isOpen) { if (placementTarget == Target) { @@ -280,7 +301,12 @@ namespace Avalonia.Controls.Primitives } PositionPopup(showAtPointer); - IsOpen = Popup.IsOpen = true; + _isOpen = true; + using (BeginIgnoringIsOpen()) + { + SetCurrentValue(IsOpenProperty, true); + } + Popup.IsOpen = true; OnOpened(); placementTarget.DetachedFromVisualTree += PlacementTarget_DetachedFromVisualTree; @@ -310,6 +336,7 @@ namespace Avalonia.Controls.Primitives private void PlacementTarget_DetachedFromVisualTree(object? sender, VisualTreeAttachmentEventArgs e) { _ = HideCore(false); + _lastPlacementTarget = null; } private void HandleTransientDismiss(RawInputEventArgs args) @@ -318,7 +345,7 @@ namespace Avalonia.Controls.Primitives { // In ShowMode = TransientWithDismissOnPointerMoveAway, the Flyout is kept // shown as long as the pointer is within a certain px distance from the - // flyout itself. I'm not sure what WinUI uses, but I'm defaulting to + // flyout itself. I'm not sure what WinUI uses, but I'm defaulting to // 100px, which seems about right // enlargedPopupRect is the Flyout bounds enlarged 100px // For windowed popups, enlargedPopupRect is in screen coordinates, @@ -348,7 +375,7 @@ namespace Avalonia.Controls.Primitives // As long as the pointer stays within the enlargedPopupRect // the flyout stays open. If it leaves, close it // Despite working in screen coordinates, leaving the TopLevel - // window will not close this (as pointer events stop), which + // window will not close this (as pointer events stop), which // does match UWP var pt = eventRoot.PointToScreen(pArgs.Position); if (!_enlargePopupRectScreenPixelRect?.Contains(pt) ?? false) @@ -401,14 +428,18 @@ namespace Avalonia.Controls.Primitives private void OnPopupOpened(object? sender, EventArgs e) { - IsOpen = true; + _isOpen = true; + using (BeginIgnoringIsOpen()) + { + SetCurrentValue(IsOpenProperty, true); + } _popupHostChangedHandler?.Invoke(Popup.Host); } private void OnPopupClosing(object? sender, CancelEventArgs e) { - if (IsOpen) + if (_isOpen) { e.Cancel = CancelClosing(); } @@ -425,7 +456,7 @@ namespace Avalonia.Controls.Primitives private void OnPlacementTargetOrPopupKeyUp(object? sender, KeyEventArgs e) { if (!e.Handled - && IsOpen + && _isOpen && Target?.ContextFlyout == this) { var keymap = Application.Current!.PlatformSettings?.HotkeyConfiguration; @@ -437,6 +468,60 @@ namespace Avalonia.Controls.Primitives } } + private void IsOpenChanged(AvaloniaPropertyChangedEventArgs e) + { + if (_ignoreIsOpenChanged) + { + return; + } + + if (e.NewValue.Value) + { + if (_lastPlacementTarget != null && ShowAtCore(_lastPlacementTarget)) + { + return; + } + + // No target, or opening was cancelled — revert so IsOpen stays honest + using (BeginIgnoringIsOpen()) + { + SetCurrentValue(IsOpenProperty, false); + } + } + else + { + if (!HideCore()) + { + // Closing was cancelled — revert so IsOpen stays honest + using (BeginIgnoringIsOpen()) + { + SetCurrentValue(IsOpenProperty, true); + } + } + } + } + + private IgnoreIsOpenScope BeginIgnoringIsOpen() + { + return new IgnoreIsOpenScope(this); + } + + private readonly struct IgnoreIsOpenScope : IDisposable + { + private readonly PopupFlyoutBase _owner; + + public IgnoreIsOpenScope(PopupFlyoutBase owner) + { + _owner = owner; + _owner._ignoreIsOpenChanged = true; + } + + public void Dispose() + { + _owner._ignoreIsOpenChanged = false; + } + } + private void PositionPopup(bool showAtPointer) { Size sz; diff --git a/src/Avalonia.Controls/SplitButton/SplitButton.cs b/src/Avalonia.Controls/SplitButton/SplitButton.cs index 28225a5ad1..390f679191 100644 --- a/src/Avalonia.Controls/SplitButton/SplitButton.cs +++ b/src/Avalonia.Controls/SplitButton/SplitButton.cs @@ -331,10 +331,13 @@ namespace Avalonia.Controls oldFlyout.Hide(); } + (oldFlyout as PopupFlyoutBase)?.SetDefaultPlacementTarget(null); + // Must unregister events here while a reference to the old flyout still exists UnregisterFlyoutEvents(oldFlyout); RegisterFlyoutEvents(newFlyout); + (newFlyout as PopupFlyoutBase)?.SetDefaultPlacementTarget(this); UpdatePseudoClasses(); } diff --git a/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs b/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs index 4d8a90f5e7..77c3eb0874 100644 --- a/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs +++ b/tests/Avalonia.Controls.UnitTests/FlyoutTests.cs @@ -3,6 +3,7 @@ using System.ComponentModel; using System.Linq; using Avalonia.Controls.Primitives; +using Avalonia.Data; using Avalonia.Input; using Avalonia.Markup.Xaml; using Avalonia.Media; @@ -646,6 +647,203 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void IsOpen_SetFalse_Closes_Flyout() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + window.Show(); + + var flyout = new TestFlyout(); + bool closedFired = false; + flyout.Closed += (s, e) => closedFired = true; + + flyout.ShowAt(window); + Assert.True(flyout.IsOpen); + + flyout.IsOpen = false; + + Assert.False(flyout.IsOpen); + Assert.False(flyout.Popup.IsOpen); + Assert.True(closedFired); + } + } + + [Fact] + public void IsOpen_SetTrue_Reopens_At_Last_Target() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + window.Show(); + + var flyout = new TestFlyout(); + flyout.ShowAt(window); + Assert.True(flyout.IsOpen); + + flyout.Hide(); + Assert.False(flyout.IsOpen); + + flyout.IsOpen = true; + + Assert.True(flyout.IsOpen); + Assert.True(flyout.Popup.IsOpen); + Assert.Equal(window, flyout.Popup.PlacementTarget); + } + } + + [Fact] + public void IsOpen_SetTrue_Without_Previous_Target_Reverts_To_False() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + window.Show(); + + var flyout = new TestFlyout(); + + flyout.IsOpen = true; + + Assert.False(flyout.IsOpen); + Assert.False(flyout.Popup.IsOpen); + } + } + + [Fact] + public void IsOpen_SetTrue_Opens_At_Button_Flyout_Owner() + { + using (CreateServicesWithFocus()) + { + var button = new Button(); + var window = PreparedWindow(button); + window.Show(); + + var flyout = new TestFlyout(); + button.Flyout = flyout; + + flyout.IsOpen = true; + + Assert.True(flyout.IsOpen); + Assert.True(flyout.Popup.IsOpen); + Assert.Equal(button, flyout.Popup.PlacementTarget); + } + } + + [Fact] + public void IsOpen_Button_Flyout_Removed_Clears_Target() + { + using (CreateServicesWithFocus()) + { + var button = new Button(); + var window = PreparedWindow(button); + window.Show(); + + var flyout = new TestFlyout(); + button.Flyout = flyout; + + button.Flyout = null; + + flyout.IsOpen = true; + + Assert.False(flyout.IsOpen); + } + } + + [Fact] + public void IsOpen_TwoWay_Binding_Syncs_With_Source() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + window.Show(); + + var viewModel = new FlyoutViewModel(); + var flyout = new TestFlyout(); + flyout.Bind(FlyoutBase.IsOpenProperty, new Binding(nameof(FlyoutViewModel.IsOpen)) + { + Source = viewModel, + Mode = BindingMode.TwoWay + }); + + Assert.False(viewModel.IsOpen); + + flyout.ShowAt(window); + Assert.True(viewModel.IsOpen); + + flyout.Hide(); + Assert.False(viewModel.IsOpen); + } + } + + [Fact] + public void IsOpen_SetFalse_Cancelled_Closing_Reverts_To_True() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + window.Show(); + + var flyout = new TestFlyout(); + flyout.Closing += (s, e) => e.Cancel = true; + + flyout.ShowAt(window); + Assert.True(flyout.IsOpen); + + flyout.IsOpen = false; + + Assert.True(flyout.IsOpen); + Assert.True(flyout.Popup.IsOpen); + } + } + + [Fact] + public void IsOpen_SetTrue_After_Target_Detached_Reverts_To_False() + { + using (CreateServicesWithFocus()) + { + var target = new Button(); + var window = PreparedWindow(target); + window.Show(); + + var flyout = new TestFlyout(); + flyout.ShowAt(target); + Assert.True(flyout.IsOpen); + + // Detach the target from the visual tree + window.Content = null; + Assert.False(flyout.IsOpen); + + flyout.IsOpen = true; + + Assert.False(flyout.IsOpen); + } + } + + [Fact] + public void IsOpen_SetTrue_Cancelled_Opening_Reverts_To_False() + { + using (CreateServicesWithFocus()) + { + var window = PreparedWindow(); + window.Show(); + + var flyout = new TestFlyout(); + flyout.ShowAt(window); + flyout.Hide(); + + flyout.Opening += (s, e) => + { + if (e is CancelEventArgs cancelArgs) + cancelArgs.Cancel = true; + }; + + flyout.IsOpen = true; + + Assert.False(flyout.IsOpen); + } + } + private IDisposable CreateServicesWithFocus() { return UnitTestApplication.Start(TestServices.StyledWindow.With(windowingPlatform: @@ -677,11 +875,31 @@ namespace Avalonia.Controls.UnitTests new PointerPointProperties(RawInputModifiers.None, PointerUpdateKind.LeftButtonPressed), KeyModifiers.None); } - + public class TestFlyout : Flyout { public new Popup Popup => base.Popup; } + + private class FlyoutViewModel : INotifyPropertyChanged + { + private bool _isOpen; + + public bool IsOpen + { + get => _isOpen; + set + { + if (_isOpen != value) + { + _isOpen = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(IsOpen))); + } + } + } + + public event PropertyChangedEventHandler? PropertyChanged; + } } public class OverlayPopupFlyoutTests : FlyoutTests