diff --git a/src/Avalonia.Base/Utilities/WeakEventHandlerManager.cs b/src/Avalonia.Base/Utilities/WeakEventHandlerManager.cs index 37e25d0fac..aca08f5259 100644 --- a/src/Avalonia.Base/Utilities/WeakEventHandlerManager.cs +++ b/src/Avalonia.Base/Utilities/WeakEventHandlerManager.cs @@ -183,11 +183,16 @@ namespace Avalonia.Utilities for (int c = 0; c < _count; c++) { var r = _data[c]; + + TSubscriber target = null; + + r.Subscriber?.TryGetTarget(out target); + //Mark current index as first empty - if (r.Subscriber == null && empty == -1) + if (target == null && empty == -1) empty = c; //If current element isn't null and we have an empty one - if (r.Subscriber != null && empty != -1) + if (target != null && empty != -1) { _data[c] = default; _data[empty] = r; diff --git a/src/Avalonia.Controls/AutoCompleteBox.cs b/src/Avalonia.Controls/AutoCompleteBox.cs index 6deddef0d0..bf177d64cd 100644 --- a/src/Avalonia.Controls/AutoCompleteBox.cs +++ b/src/Avalonia.Controls/AutoCompleteBox.cs @@ -683,7 +683,7 @@ namespace Avalonia.Controls added.Add(e.NewValue); } - OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, added, removed)); + OnSelectionChanged(new SelectionChangedEventArgs(SelectionChangedEvent, removed, added)); } /// diff --git a/src/Avalonia.Controls/Calendar/DatePicker.cs b/src/Avalonia.Controls/Calendar/DatePicker.cs index 46a1f5557b..ea06bdb394 100644 --- a/src/Avalonia.Controls/Calendar/DatePicker.cs +++ b/src/Avalonia.Controls/Calendar/DatePicker.cs @@ -788,7 +788,7 @@ namespace Avalonia.Controls removedItems.Add(removedDate.Value); } - handler(this, new SelectionChangedEventArgs(SelectingItemsControl.SelectionChangedEvent, addedItems, removedItems)); + handler(this, new SelectionChangedEventArgs(SelectingItemsControl.SelectionChangedEvent, removedItems, addedItems)); } } private void OnCalendarClosed(EventArgs e) diff --git a/src/Avalonia.Controls/Calendar/SelectedDatesCollection.cs b/src/Avalonia.Controls/Calendar/SelectedDatesCollection.cs index e6e0cf7c4f..3573c67057 100644 --- a/src/Avalonia.Controls/Calendar/SelectedDatesCollection.cs +++ b/src/Avalonia.Controls/Calendar/SelectedDatesCollection.cs @@ -49,7 +49,7 @@ namespace Avalonia.Controls.Primitives private void InvokeCollectionChanged(System.Collections.IList removedItems, System.Collections.IList addedItems) { - _owner.OnSelectedDatesCollectionChanged(new SelectionChangedEventArgs(null, addedItems, removedItems)); + _owner.OnSelectedDatesCollectionChanged(new SelectionChangedEventArgs(null, removedItems, addedItems)); } /// @@ -119,7 +119,7 @@ namespace Avalonia.Controls.Primitives } } - _owner.OnSelectedDatesCollectionChanged(new SelectionChangedEventArgs(null, _addedItems, _owner.RemovedItems)); + _owner.OnSelectedDatesCollectionChanged(new SelectionChangedEventArgs(null, _owner.RemovedItems, _addedItems)); _owner.RemovedItems.Clear(); _owner.UpdateMonths(); _isRangeAdded = false; diff --git a/src/Avalonia.Controls/Primitives/Popup.cs b/src/Avalonia.Controls/Primitives/Popup.cs index 77febf9384..9dd481cf40 100644 --- a/src/Avalonia.Controls/Primitives/Popup.cs +++ b/src/Avalonia.Controls/Primitives/Popup.cs @@ -2,12 +2,10 @@ // Licensed under the MIT license. See licence.md file in the project root for full license information. using System; -using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Reactive.Disposables; using Avalonia.Controls.Presenters; -using Avalonia.Data; using Avalonia.Input; using Avalonia.Input.Raw; using Avalonia.Interactivity; @@ -15,6 +13,8 @@ using Avalonia.LogicalTree; using Avalonia.Metadata; using Avalonia.VisualTree; +#nullable enable + namespace Avalonia.Controls.Primitives { /// @@ -25,8 +25,8 @@ namespace Avalonia.Controls.Primitives /// /// Defines the property. /// - public static readonly StyledProperty ChildProperty = - AvaloniaProperty.Register(nameof(Child)); + public static readonly StyledProperty ChildProperty = + AvaloniaProperty.Register(nameof(Child)); /// /// Defines the property. @@ -43,11 +43,13 @@ namespace Avalonia.Controls.Primitives public static readonly StyledProperty PlacementModeProperty = AvaloniaProperty.Register(nameof(PlacementMode), defaultValue: PlacementMode.Bottom); +#pragma warning disable 618 /// /// Defines the property. /// public static readonly StyledProperty ObeyScreenEdgesProperty = AvaloniaProperty.Register(nameof(ObeyScreenEdges), true); +#pragma warning restore 618 /// /// Defines the property. @@ -64,8 +66,8 @@ namespace Avalonia.Controls.Primitives /// /// Defines the property. /// - public static readonly StyledProperty PlacementTargetProperty = - AvaloniaProperty.Register(nameof(PlacementTarget)); + public static readonly StyledProperty PlacementTargetProperty = + AvaloniaProperty.Register(nameof(PlacementTarget)); /// /// Defines the property. @@ -80,12 +82,8 @@ namespace Avalonia.Controls.Primitives AvaloniaProperty.Register(nameof(Topmost)); private bool _isOpen; - private IPopupHost _popupHost; - private TopLevel _topLevel; - private IDisposable _nonClientListener; - private IDisposable _presenterSubscription; - bool _ignoreIsOpenChanged = false; - private List _bindings = new List(); + private bool _ignoreIsOpenChanged; + private PopupOpenState? _openState; /// /// Initializes static members of the class. @@ -94,31 +92,26 @@ namespace Avalonia.Controls.Primitives { IsHitTestVisibleProperty.OverrideDefaultValue(false); ChildProperty.Changed.AddClassHandler((x, e) => x.ChildChanged(e)); - IsOpenProperty.Changed.AddClassHandler((x, e) => x.IsOpenChanged(e)); - } - - public Popup() - { - + IsOpenProperty.Changed.AddClassHandler((x, e) => x.IsOpenChanged((AvaloniaPropertyChangedEventArgs)e)); } /// /// Raised when the popup closes. /// - public event EventHandler Closed; + public event EventHandler? Closed; /// /// Raised when the popup opens. /// - public event EventHandler Opened; + public event EventHandler? Opened; - public IPopupHost Host => _popupHost; + public IPopupHost? Host => _openState?.PopupHost; /// /// Gets or sets the control to display in the popup. /// [Content] - public Control Child + public Control? Child { get { return GetValue(ChildProperty); } set { SetValue(ChildProperty, value); } @@ -131,7 +124,7 @@ namespace Avalonia.Controls.Primitives /// This property allows a client to customize the behaviour of the popup by injecting /// a specialized dependency resolver into the 's constructor. /// - public IAvaloniaDependencyResolver DependencyResolver + public IAvaloniaDependencyResolver? DependencyResolver { get; set; @@ -183,7 +176,7 @@ namespace Avalonia.Controls.Primitives /// /// Gets or sets the control that is used to determine the popup's position. /// - public Control PlacementTarget + public Control? PlacementTarget { get { return GetValue(PlacementTargetProperty); } set { SetValue(PlacementTargetProperty, value); } @@ -211,7 +204,7 @@ namespace Avalonia.Controls.Primitives /// /// Gets the root of the popup window. /// - IVisual IVisualTreeHost.Root => _popupHost?.HostedVisualTreeRoot; + IVisual? IVisualTreeHost.Root => _openState?.PopupHost.HostedVisualTreeRoot; /// /// Opens the popup. @@ -219,50 +212,91 @@ namespace Avalonia.Controls.Primitives public void Open() { // Popup is currently open - if (_topLevel != null) + if (_openState != null) + { return; - CloseCurrent(); + } + var placementTarget = PlacementTarget ?? this.GetLogicalAncestors().OfType().FirstOrDefault(); + if (placementTarget == null) + { throw new InvalidOperationException("Popup has no logical parent and PlacementTarget is null"); + } - _topLevel = placementTarget.GetVisualRoot() as TopLevel; + var topLevel = placementTarget.VisualRoot as TopLevel; - if (_topLevel == null) + if (topLevel == null) { throw new InvalidOperationException( "Attempted to open a popup not attached to a TopLevel"); } - _popupHost = OverlayPopupHost.CreatePopupHost(placementTarget, DependencyResolver); + var popupHost = OverlayPopupHost.CreatePopupHost(placementTarget, DependencyResolver); + + var handlerCleanup = new CompositeDisposable(5); - _bindings.Add(_popupHost.BindConstraints(this, WidthProperty, MinWidthProperty, MaxWidthProperty, + void DeferCleanup(IDisposable? disposable) + { + if (disposable is null) + { + return; + } + + handlerCleanup.Add(disposable); + } + + DeferCleanup(popupHost.BindConstraints(this, WidthProperty, MinWidthProperty, MaxWidthProperty, HeightProperty, MinHeightProperty, MaxHeightProperty, TopmostProperty)); - _popupHost.SetChild(Child); - ((ISetLogicalParent)_popupHost).SetParent(this); - _popupHost.ConfigurePosition(placementTarget, - PlacementMode, new Point(HorizontalOffset, VerticalOffset)); - _popupHost.TemplateApplied += RootTemplateApplied; - - var window = _topLevel as Window; - if (window != null) + popupHost.SetChild(Child); + ((ISetLogicalParent)popupHost).SetParent(this); + + popupHost.ConfigurePosition( + placementTarget, + PlacementMode, + new Point(HorizontalOffset, VerticalOffset)); + + DeferCleanup(SubscribeToEventHandler>(popupHost, RootTemplateApplied, + (x, handler) => x.TemplateApplied += handler, + (x, handler) => x.TemplateApplied -= handler)); + + if (topLevel is Window window) { - window.Deactivated += WindowDeactivated; + DeferCleanup(SubscribeToEventHandler(window, WindowDeactivated, + (x, handler) => x.Deactivated += handler, + (x, handler) => x.Deactivated -= handler)); } else { - var parentPopuproot = _topLevel as PopupRoot; - if (parentPopuproot?.Parent is Popup popup) + var parentPopupRoot = topLevel as PopupRoot; + + if (parentPopupRoot?.Parent is Popup popup) { - popup.Closed += ParentClosed; + DeferCleanup(SubscribeToEventHandler(popup, ParentClosed, + (x, handler) => x.Closed += handler, + (x, handler) => x.Closed -= handler)); } } - _topLevel.AddHandler(PointerPressedEvent, PointerPressedOutside, RoutingStrategies.Tunnel); - _nonClientListener = InputManager.Instance?.Process.Subscribe(ListenForNonClientClick); - - _popupHost.Show(); + DeferCleanup(topLevel.AddHandler(PointerPressedEvent, PointerPressedOutside, RoutingStrategies.Tunnel)); + + DeferCleanup(InputManager.Instance?.Process.Subscribe(ListenForNonClientClick)); + + var cleanupPopup = Disposable.Create((popupHost, handlerCleanup), state => + { + state.handlerCleanup.Dispose(); + + state.popupHost.SetChild(null); + state.popupHost.Hide(); + + ((ISetLogicalParent)state.popupHost).SetParent(null); + state.popupHost.Dispose(); + }); + + _openState = new PopupOpenState(topLevel, popupHost, cleanupPopup); + + popupHost.Show(); using (BeginIgnoringIsOpen()) { @@ -277,14 +311,19 @@ namespace Avalonia.Controls.Primitives /// public void Close() { - if (_popupHost != null) + if (_openState is null) { - _popupHost.TemplateApplied -= RootTemplateApplied; + using (BeginIgnoringIsOpen()) + { + IsOpen = false; + } + + return; } - _presenterSubscription?.Dispose(); + _openState.Dispose(); + _openState = null; - CloseCurrent(); using (BeginIgnoringIsOpen()) { IsOpen = false; @@ -293,41 +332,6 @@ namespace Avalonia.Controls.Primitives Closed?.Invoke(this, EventArgs.Empty); } - void CloseCurrent() - { - if (_topLevel != null) - { - _topLevel.RemoveHandler(PointerPressedEvent, PointerPressedOutside); - var window = _topLevel as Window; - if (window != null) - window.Deactivated -= WindowDeactivated; - else - { - var parentPopuproot = _topLevel as PopupRoot; - if (parentPopuproot?.Parent is Popup popup) - { - popup.Closed -= ParentClosed; - } - } - _nonClientListener?.Dispose(); - _nonClientListener = null; - - _topLevel = null; - } - if (_popupHost != null) - { - foreach(var b in _bindings) - b.Dispose(); - _bindings.Clear(); - _popupHost.SetChild(null); - _popupHost.Hide(); - ((ISetLogicalParent)_popupHost).SetParent(null); - _popupHost.Dispose(); - _popupHost = null; - } - - } - /// /// Measures the control. /// @@ -345,16 +349,22 @@ namespace Avalonia.Controls.Primitives Close(); } + private static IDisposable SubscribeToEventHandler(T target, TEventHandler handler, Action subscribe, Action unsubscribe) + { + subscribe(target, handler); + + return Disposable.Create((unsubscribe, target, handler), state => state.unsubscribe(state.target, state.handler)); + } /// /// Called when the property changes. /// /// The event args. - private void IsOpenChanged(AvaloniaPropertyChangedEventArgs e) + private void IsOpenChanged(AvaloniaPropertyChangedEventArgs e) { if (!_ignoreIsOpenChanged) { - if ((bool)e.NewValue) + if (e.NewValue.Value) { Open(); } @@ -373,7 +383,7 @@ namespace Avalonia.Controls.Primitives { LogicalChildren.Clear(); - ((ISetLogicalParent)e.OldValue)?.SetParent(null); + ((ISetLogicalParent?)e.OldValue)?.SetParent(null); if (e.NewValue != null) { @@ -394,34 +404,37 @@ namespace Avalonia.Controls.Primitives private void PointerPressedOutside(object sender, PointerPressedEventArgs e) { - if (!StaysOpen) + if (!StaysOpen && !IsChildOrThis((IVisual)e.Source)) { - if (!IsChildOrThis((IVisual)e.Source)) - { - Close(); - e.Handled = true; - } + Close(); + e.Handled = true; } } private void RootTemplateApplied(object sender, TemplateAppliedEventArgs e) { - _popupHost.TemplateApplied -= RootTemplateApplied; - - if (_presenterSubscription != null) + if (_openState is null) { - _presenterSubscription.Dispose(); - _presenterSubscription = null; + return; } + var popupHost = _openState.PopupHost; + + popupHost.TemplateApplied -= RootTemplateApplied; + + _openState.SetPresenterSubscription(null); + // If the Popup appears in a control template, then the child controls // that appear in the popup host need to have their TemplatedParent // properties set. - if (TemplatedParent != null) + if (TemplatedParent != null && popupHost.Presenter != null) { - _popupHost.Presenter?.ApplyTemplate(); - _popupHost.Presenter?.GetObservable(ContentPresenter.ChildProperty) + popupHost.Presenter.ApplyTemplate(); + + var presenterSubscription = popupHost.Presenter.GetObservable(ContentPresenter.ChildProperty) .Subscribe(SetTemplatedParentAndApplyChildTemplates); + + _openState.SetPresenterSubscription(presenterSubscription); } } @@ -440,7 +453,7 @@ namespace Avalonia.Controls.Primitives if (!(control is IPresenter) && control.TemplatedParent == templatedParent) { - foreach (IControl child in control.GetVisualChildren()) + foreach (IControl child in control.VisualChildren) { SetTemplatedParentAndApplyChildTemplates(child); } @@ -450,22 +463,41 @@ namespace Avalonia.Controls.Primitives private bool IsChildOrThis(IVisual child) { - IVisual root = child.GetVisualRoot(); - while (root is IHostedVisualTreeRoot hostedRoot ) + if (_openState is null) { - if (root == this._popupHost) + return false; + } + + var popupHost = _openState.PopupHost; + + IVisual? root = child.VisualRoot; + + while (root is IHostedVisualTreeRoot hostedRoot) + { + if (root == popupHost) + { return true; - root = hostedRoot.Host?.GetVisualRoot(); + } + + root = hostedRoot.Host?.VisualRoot; } + return false; } public bool IsInsidePopup(IVisual visual) { - return _popupHost != null && ((IVisual)_popupHost)?.IsVisualAncestorOf(visual) == true; + if (_openState is null) + { + return false; + } + + var popupHost = _openState.PopupHost; + + return popupHost != null && ((IVisual)popupHost).IsVisualAncestorOf(visual); } - public bool IsPointerOverPopup => ((IInputElement)_popupHost).IsPointerOver; + public bool IsPointerOverPopup => ((IInputElement?)_openState?.PopupHost)?.IsPointerOver ?? false; private void WindowDeactivated(object sender, EventArgs e) { @@ -503,5 +535,36 @@ namespace Avalonia.Controls.Primitives _owner._ignoreIsOpenChanged = false; } } + + private class PopupOpenState : IDisposable + { + private readonly IDisposable _cleanup; + private IDisposable? _presenterCleanup; + + public PopupOpenState(TopLevel topLevel, IPopupHost popupHost, IDisposable cleanup) + { + TopLevel = topLevel; + PopupHost = popupHost; + _cleanup = cleanup; + } + + public TopLevel TopLevel { get; } + + public IPopupHost PopupHost { get; } + + public void SetPresenterSubscription(IDisposable? presenterCleanup) + { + _presenterCleanup?.Dispose(); + + _presenterCleanup = presenterCleanup; + } + + public void Dispose() + { + _presenterCleanup?.Dispose(); + + _cleanup.Dispose(); + } + } } } diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 69da211aa4..6bc4e71508 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -939,8 +939,8 @@ namespace Avalonia.Controls.Primitives { var changed = new SelectionChangedEventArgs( SelectionChangedEvent, - added ?? Empty, - removed ?? Empty); + removed ?? Empty, + added ?? Empty); RaiseEvent(changed); } } @@ -1055,8 +1055,8 @@ namespace Avalonia.Controls.Primitives var e = new SelectionChangedEventArgs( SelectionChangedEvent, - added != -1 ? new[] { ElementAt(Items, added) } : Array.Empty(), - removed?.Select(x => ElementAt(Items, x)).ToArray() ?? Array.Empty()); + removed?.Select(x => ElementAt(Items, x)).ToArray() ?? Array.Empty(), + added != -1 ? new[] { ElementAt(Items, added) } : Array.Empty()); RaiseEvent(e); } diff --git a/src/Avalonia.Controls/Repeater/ItemsRepeaterElementIndexChangedEventArgs.cs b/src/Avalonia.Controls/Repeater/ItemsRepeaterElementIndexChangedEventArgs.cs index 7ca68140b2..bf1b80f947 100644 --- a/src/Avalonia.Controls/Repeater/ItemsRepeaterElementIndexChangedEventArgs.cs +++ b/src/Avalonia.Controls/Repeater/ItemsRepeaterElementIndexChangedEventArgs.cs @@ -12,11 +12,11 @@ namespace Avalonia.Controls /// public class ItemsRepeaterElementIndexChangedEventArgs : EventArgs { - internal ItemsRepeaterElementIndexChangedEventArgs(IControl element, int newIndex, int oldIndex) + internal ItemsRepeaterElementIndexChangedEventArgs(IControl element, int oldIndex, int newIndex) { Element = element; - NewIndex = newIndex; OldIndex = oldIndex; + NewIndex = newIndex; } /// diff --git a/src/Avalonia.Controls/SelectionChangedEventArgs.cs b/src/Avalonia.Controls/SelectionChangedEventArgs.cs index 267ddf036e..fb412a64a8 100644 --- a/src/Avalonia.Controls/SelectionChangedEventArgs.cs +++ b/src/Avalonia.Controls/SelectionChangedEventArgs.cs @@ -16,13 +16,13 @@ namespace Avalonia.Controls /// Initializes a new instance of the class. /// /// The event being raised. - /// The items added to the selection. /// The items removed from the selection. - public SelectionChangedEventArgs(RoutedEvent routedEvent, IList addedItems, IList removedItems) + /// The items added to the selection. + public SelectionChangedEventArgs(RoutedEvent routedEvent, IList removedItems, IList addedItems) : base(routedEvent) { - AddedItems = addedItems; RemovedItems = removedItems; + AddedItems = addedItems; } /// diff --git a/src/Avalonia.Controls/TreeView.cs b/src/Avalonia.Controls/TreeView.cs index 6c8d58a8dd..99e9ac2ecf 100644 --- a/src/Avalonia.Controls/TreeView.cs +++ b/src/Avalonia.Controls/TreeView.cs @@ -324,8 +324,8 @@ namespace Avalonia.Controls { var changed = new SelectionChangedEventArgs( SelectingItemsControl.SelectionChangedEvent, - added ?? Empty, - removed ?? Empty); + removed ?? Empty, + added ?? Empty); RaiseEvent(changed); } } diff --git a/tests/Avalonia.Controls.UnitTests/ComboBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ComboBoxTests.cs index 599e214b31..81908b5147 100644 --- a/tests/Avalonia.Controls.UnitTests/ComboBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ComboBoxTests.cs @@ -108,5 +108,36 @@ namespace Avalonia.Controls.UnitTests }; }); } + + [Fact] + public void Detaching_Closed_ComboBox_Keeps_Current_Focus() + { + using (UnitTestApplication.Start(TestServices.RealFocus)) + { + var target = new ComboBox + { + Items = new[] { new Canvas() }, + SelectedIndex = 0, + Template = GetTemplate(), + }; + + var other = new Control { Focusable = true }; + + StackPanel panel; + + var root = new TestRoot { Child = panel = new StackPanel { Children = { target, other } } }; + + target.ApplyTemplate(); + target.Presenter.ApplyTemplate(); + + other.Focus(); + + Assert.True(other.IsFocused); + + panel.Children.Remove(target); + + Assert.True(other.IsFocused); + } + } } } diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs index 7cb9fccee8..1f07482617 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs @@ -223,7 +223,33 @@ namespace Avalonia.Controls.UnitTests.Primitives } } - + [Fact] + public void Popup_Close_On_Closed_Popup_Should_Not_Raise_Closed_Event() + { + using (CreateServices()) + { + var window = PreparedWindow(); + var target = new Popup() { PlacementMode = PlacementMode.Pointer }; + + window.Content = target; + window.ApplyTemplate(); + + int closedCount = 0; + + target.Closed += (sender, args) => + { + closedCount++; + }; + + target.Close(); + target.Close(); + target.Close(); + target.Close(); + + Assert.Equal(0, closedCount); + } + } + [Fact] public void Templated_Control_With_Popup_In_Template_Should_Set_TemplatedParent() {