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

Close popup if combobox is not visible #6404

Merged
merged 4 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/Avalonia.Controls/ComboBox.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Linq;
using System.Reactive.Disposables;
using Avalonia.Controls.Generators;
using Avalonia.Controls.Mixins;
using Avalonia.Controls.Presenters;
using Avalonia.Controls.Primitives;
using Avalonia.Controls.Shapes;
Expand Down Expand Up @@ -80,7 +82,7 @@ public class ComboBox : SelectingItemsControl
private bool _isDropDownOpen;
private Popup _popup;
private object _selectionBoxItem;
private IDisposable _subscriptionsOnOpen;
private readonly CompositeDisposable _subscriptionsOnOpen = new CompositeDisposable();

/// <summary>
/// Initializes static members of the <see cref="ComboBox"/> class.
Expand Down Expand Up @@ -291,6 +293,7 @@ protected override void OnApplyTemplate(TemplateAppliedEventArgs e)

_popup = e.NameScope.Get<Popup>("PART_Popup");
_popup.Opened += PopupOpened;
_popup.Closed += PopupClosed;
Copy link
Member

Choose a reason for hiding this comment

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

Huh, wow, looks like we weren't subscribing to the Closed event, meaning that PopupClosed was never called!

}

internal void ItemFocused(ComboBoxItem dropDownItem)
Expand All @@ -303,8 +306,7 @@ internal void ItemFocused(ComboBoxItem dropDownItem)

private void PopupClosed(object sender, EventArgs e)
{
_subscriptionsOnOpen?.Dispose();
_subscriptionsOnOpen = null;
_subscriptionsOnOpen.Clear();

if (CanFocus(this))
{
Expand All @@ -316,20 +318,34 @@ private void PopupOpened(object sender, EventArgs e)
{
TryFocusSelectedItem();

_subscriptionsOnOpen?.Dispose();
_subscriptionsOnOpen = null;
_subscriptionsOnOpen.Clear();

var toplevel = this.GetVisualRoot() as TopLevel;
if (toplevel != null)
{
_subscriptionsOnOpen = toplevel.AddDisposableHandler(PointerWheelChangedEvent, (s, ev) =>
toplevel.AddDisposableHandler(PointerWheelChangedEvent, (s, ev) =>
{
//eat wheel scroll event outside dropdown popup while it's open
if (IsDropDownOpen && (ev.Source as IVisual).GetVisualRoot() == toplevel)
{
ev.Handled = true;
}
}, Interactivity.RoutingStrategies.Tunnel);
}, Interactivity.RoutingStrategies.Tunnel).DisposeWith(_subscriptionsOnOpen);
}

this.GetObservable(IsVisibleProperty).Subscribe(IsVisibleChanged).DisposeWith(_subscriptionsOnOpen);

foreach (var parent in this.GetVisualAncestors().OfType<IControl>())
{
parent.GetObservable(IsVisibleProperty).Subscribe(IsVisibleChanged).DisposeWith(_subscriptionsOnOpen);
}
}

private void IsVisibleChanged(bool isVisible)
{
if (!isVisible && IsDropDownOpen)
{
IsDropDownOpen = false;
}
}

Expand Down
38 changes: 38 additions & 0 deletions src/Avalonia.Controls/Mixins/DisposableMixin.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System;
using System.Reactive.Disposables;

namespace Avalonia.Controls.Mixins
{
/// <summary>
/// Extension methods associated with the IDisposable interface.
/// </summary>
public static class DisposableMixin
{
/// <summary>
/// Ensures the provided disposable is disposed with the specified <see cref="CompositeDisposable"/>.
/// </summary>
/// <typeparam name="T">
/// The type of the disposable.
/// </typeparam>
/// <param name="item">
/// The disposable we are going to want to be disposed by the CompositeDisposable.
/// </param>
/// <param name="compositeDisposable">
/// The <see cref="CompositeDisposable"/> to which <paramref name="item"/> will be added.
/// </param>
/// <returns>
/// The disposable.
/// </returns>
public static T DisposeWith<T>(this T item, CompositeDisposable compositeDisposable)
where T : IDisposable
{
if (compositeDisposable is null)
{
throw new ArgumentNullException(nameof(compositeDisposable));
}

compositeDisposable.Add(item);
return item;
}
}
}
41 changes: 16 additions & 25 deletions src/Avalonia.Controls/Primitives/Popup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.ComponentModel;
using System.Linq;
using System.Reactive.Disposables;
using Avalonia.Controls.Mixins;
using Avalonia.Controls.Diagnostics;
using Avalonia.Controls.Presenters;
using Avalonia.Controls.Primitives.PopupPositioning;
Expand Down Expand Up @@ -393,18 +394,8 @@ public void Open()

var handlerCleanup = new CompositeDisposable(5);

void DeferCleanup(IDisposable? disposable)
{
if (disposable is null)
{
return;
}

handlerCleanup.Add(disposable);
}

DeferCleanup(popupHost.BindConstraints(this, WidthProperty, MinWidthProperty, MaxWidthProperty,
HeightProperty, MinHeightProperty, MaxHeightProperty, TopmostProperty));
popupHost.BindConstraints(this, WidthProperty, MinWidthProperty, MaxWidthProperty,
HeightProperty, MinHeightProperty, MaxHeightProperty, TopmostProperty).DisposeWith(handlerCleanup);

popupHost.SetChild(Child);
((ISetLogicalParent)popupHost).SetParent(this);
Expand All @@ -418,33 +409,33 @@ void DeferCleanup(IDisposable? disposable)
PlacementConstraintAdjustment,
PlacementRect);

DeferCleanup(SubscribeToEventHandler<IPopupHost, EventHandler<TemplateAppliedEventArgs>>(popupHost, RootTemplateApplied,
SubscribeToEventHandler<IPopupHost, EventHandler<TemplateAppliedEventArgs>>(popupHost, RootTemplateApplied,
(x, handler) => x.TemplateApplied += handler,
(x, handler) => x.TemplateApplied -= handler));
(x, handler) => x.TemplateApplied -= handler).DisposeWith(handlerCleanup);

if (topLevel is Window window)
{
DeferCleanup(SubscribeToEventHandler<Window, EventHandler>(window, WindowDeactivated,
SubscribeToEventHandler<Window, EventHandler>(window, WindowDeactivated,
(x, handler) => x.Deactivated += handler,
(x, handler) => x.Deactivated -= handler));
(x, handler) => x.Deactivated -= handler).DisposeWith(handlerCleanup);

DeferCleanup(SubscribeToEventHandler<IWindowImpl, Action>(window.PlatformImpl, WindowLostFocus,
SubscribeToEventHandler<IWindowImpl, Action>(window.PlatformImpl, WindowLostFocus,
(x, handler) => x.LostFocus += handler,
(x, handler) => x.LostFocus -= handler));
(x, handler) => x.LostFocus -= handler).DisposeWith(handlerCleanup);
}
else
{
var parentPopupRoot = topLevel as PopupRoot;

if (parentPopupRoot?.Parent is Popup popup)
{
DeferCleanup(SubscribeToEventHandler<Popup, EventHandler<EventArgs>>(popup, ParentClosed,
SubscribeToEventHandler<Popup, EventHandler<EventArgs>>(popup, ParentClosed,
(x, handler) => x.Closed += handler,
(x, handler) => x.Closed -= handler));
(x, handler) => x.Closed -= handler).DisposeWith(handlerCleanup);
}
}

DeferCleanup(InputManager.Instance?.Process.Subscribe(ListenForNonClientClick));
InputManager.Instance?.Process.Subscribe(ListenForNonClientClick).DisposeWith(handlerCleanup);

var cleanupPopup = Disposable.Create((popupHost, handlerCleanup), state =>
{
Expand All @@ -466,17 +457,17 @@ void DeferCleanup(IDisposable? disposable)
dismissLayer.IsVisible = true;
dismissLayer.InputPassThroughElement = _overlayInputPassThroughElement;

DeferCleanup(Disposable.Create(() =>
Disposable.Create(() =>
{
dismissLayer.IsVisible = false;
dismissLayer.InputPassThroughElement = null;
}));
}).DisposeWith(handlerCleanup);

DeferCleanup(SubscribeToEventHandler<LightDismissOverlayLayer, EventHandler<PointerPressedEventArgs>>(
SubscribeToEventHandler<LightDismissOverlayLayer, EventHandler<PointerPressedEventArgs>>(
dismissLayer,
PointerPressedDismissOverlay,
(x, handler) => x.PointerPressed += handler,
(x, handler) => x.PointerPressed -= handler));
(x, handler) => x.PointerPressed -= handler).DisposeWith(handlerCleanup);
}
}

Expand Down