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

Fix overlay popup focus issues #17326

Merged
merged 3 commits into from
Oct 24, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Avalonia.Base/Input/IInputRoot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ public interface IInputRoot : IInputElement
/// <summary>
/// Gets or sets the keyboard navigation handler.
/// </summary>
IKeyboardNavigationHandler KeyboardNavigationHandler { get; }
IKeyboardNavigationHandler? KeyboardNavigationHandler { get; }

/// <summary>
/// Gets focus manager of the root.
Expand Down
51 changes: 48 additions & 3 deletions src/Avalonia.Controls/Primitives/OverlayPopupHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
using System.Collections.Generic;
using Avalonia.Controls.Primitives.PopupPositioning;
using Avalonia.Diagnostics;
using Avalonia.Input;
using Avalonia.Interactivity;
using Avalonia.Media;
using Avalonia.Metadata;
using Avalonia.VisualTree;
using Avalonia.Platform;

namespace Avalonia.Controls.Primitives
{
public class OverlayPopupHost : ContentControl, IPopupHost, IManagedPopupPositionerPopup
public class OverlayPopupHost : ContentControl, IPopupHost, IManagedPopupPositionerPopup, IInputRoot
{
/// <summary>
/// Defines the <see cref="Transform"/> property.
Expand All @@ -19,15 +20,21 @@ public class OverlayPopupHost : ContentControl, IPopupHost, IManagedPopupPositio

private readonly OverlayLayer _overlayLayer;
private readonly ManagedPopupPositioner _positioner;
private readonly IKeyboardNavigationHandler? _keyboardNavigationHandler;
private Point _lastRequestedPosition;
private PopupPositionRequest? _popupPositionRequest;
private Size _popupSize;
private bool _needsUpdate;

static OverlayPopupHost()
=> KeyboardNavigation.TabNavigationProperty.OverrideDefaultValue<OverlayPopupHost>(KeyboardNavigationMode.Cycle);

public OverlayPopupHost(OverlayLayer overlayLayer)
{
_overlayLayer = overlayLayer;
_positioner = new ManagedPopupPositioner(this);
_keyboardNavigationHandler = AvaloniaLocator.Current.GetService<IKeyboardNavigationHandler>();
_keyboardNavigationHandler?.SetOwner(this);
}

/// <inheritdoc />
Expand All @@ -53,6 +60,38 @@ bool IPopupHost.Topmost
set { /* Not currently supported in overlay popups */ }
}

private IInputRoot? InputRoot
=> TopLevel.GetTopLevel(this);

IKeyboardNavigationHandler? IInputRoot.KeyboardNavigationHandler
=> _keyboardNavigationHandler;

IFocusManager? IInputRoot.FocusManager
=> InputRoot?.FocusManager;

IPlatformSettings? IInputRoot.PlatformSettings
=> InputRoot?.PlatformSettings;

IInputElement? IInputRoot.PointerOverElement
{
get => InputRoot?.PointerOverElement;
set
{
if (InputRoot is { } inputRoot)
inputRoot.PointerOverElement = value;
}
}

bool IInputRoot.ShowAccessKeys
{
get => InputRoot?.ShowAccessKeys ?? false;
set
{
if (InputRoot is { } inputRoot)
inputRoot.ShowAccessKeys = value;
}
}

/// <inheritdoc />
internal override Interactive? InteractiveParent => Parent as Interactive;

Expand All @@ -63,6 +102,12 @@ bool IPopupHost.Topmost
public void Show()
{
_overlayLayer.Children.Add(this);

if (Content is Visual { IsAttachedToVisualTree: false })
{
// We need to force a measure pass so any descendants are built, for focus to work.
UpdateLayout();
}
}

/// <inheritdoc />
Expand Down Expand Up @@ -147,7 +192,7 @@ void IManagedPopupPositionerPopup.MoveAndResize(Point devicePoint, Size virtualS
double IManagedPopupPositionerPopup.Scaling => 1;

// TODO12: mark PrivateAPI or internal.
[Unstable("PopupHost is consireded an internal API. Use Popup or any Popup-based controls (Flyout, Tooltip) instead.")]
[Unstable("PopupHost is considered an internal API. Use Popup or any Popup-based controls (Flyout, Tooltip) instead.")]
public static IPopupHost CreatePopupHost(Visual target, IAvaloniaDependencyResolver? dependencyResolver)
{
if (TopLevel.GetTopLevel(target) is { } topLevel && topLevel.PlatformImpl?.CreatePopup() is { } popupImpl)
Expand Down
4 changes: 2 additions & 2 deletions src/Avalonia.Controls/TopLevel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -471,12 +471,12 @@ void UpdatePlatformImpl()
/// <summary>
/// Gets the access key handler for the window.
/// </summary>
internal IAccessKeyHandler AccessKeyHandler => _accessKeyHandler!;
internal IAccessKeyHandler? AccessKeyHandler => _accessKeyHandler;

/// <summary>
/// Gets or sets the keyboard navigation handler for the window.
/// </summary>
IKeyboardNavigationHandler IInputRoot.KeyboardNavigationHandler => _keyboardNavigationHandler!;
IKeyboardNavigationHandler? IInputRoot.KeyboardNavigationHandler => _keyboardNavigationHandler;

/// <inheritdoc/>
IInputElement? IInputRoot.PointerOverElement
Expand Down
19 changes: 12 additions & 7 deletions tests/Avalonia.Controls.UnitTests/FlyoutTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ namespace Avalonia.Controls.UnitTests
{
public class FlyoutTests
{
protected bool UseOverlayPopups { get; set; }

[Fact]
public void Opening_Raises_Single_Opening_Event()
{
Expand Down Expand Up @@ -373,10 +375,10 @@ public void ShowMode_Standard_Attemps_Focus_Flyout_Content()
window.Show();

button.Focus();
Assert.True(window.FocusManager.GetFocusedElement() == button);
Assert.Same(button, window.FocusManager!.GetFocusedElement());
button.Flyout.ShowAt(button);
Assert.False(button.IsFocused);
Assert.True(window.FocusManager.GetFocusedElement() == flyoutTextBox);
Assert.Same(flyoutTextBox, window.FocusManager!.GetFocusedElement());
}
}

Expand Down Expand Up @@ -640,14 +642,11 @@ public void Setting_FlyoutPresenterClasses_Sets_Classes_On_FlyoutPresenter()
}
}

private static IDisposable CreateServicesWithFocus()
private IDisposable CreateServicesWithFocus()
{
return UnitTestApplication.Start(TestServices.StyledWindow.With(windowingPlatform:
new MockWindowingPlatform(null,
x =>
{
return MockWindowingPlatform.CreatePopupMock(x).Object;
}),
x => UseOverlayPopups ? null : MockWindowingPlatform.CreatePopupMock(x).Object),
focusManager: new FocusManager(),
keyboardDevice: () => new KeyboardDevice()));
}
Expand Down Expand Up @@ -681,4 +680,10 @@ public class TestFlyout : Flyout
public new Popup Popup => base.Popup;
}
}

public class OverlayPopupFlyoutTests : FlyoutTests
{
public OverlayPopupFlyoutTests()
=> UseOverlayPopups = true;
}
}
43 changes: 18 additions & 25 deletions tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -633,46 +633,39 @@ public void Focusable_Controls_In_Popup_Should_Get_Focus()
{
using (CreateServicesWithFocus())
{
var window = PreparedWindow();
var window = PreparedWindow(new Panel { Children = { new Slider() }});

var tb = new TextBox();
var b = new Button();
var p = new Popup
var textBox = new TextBox();
var button = new Button();
var popup = new Popup
{
PlacementTarget = window,
Child = new StackPanel
{
Children =
{
tb,
b
textBox,
button
}
}
};
((ISetLogicalParent)p).SetParent(p.PlacementTarget);
window.Show();

p.Open();
((ISetLogicalParent)popup).SetParent(popup.PlacementTarget);
window.Show();
popup.Open();

if(p.Host is OverlayPopupHost host)
{
//Need to measure/arrange for visual children to show up
//in OverlayPopupHost
host.Measure(Size.Infinity);
host.Arrange(new Rect(host.DesiredSize));
}
button.Focus();

tb.Focus();
var inputRoot = Assert.IsAssignableFrom<IInputRoot>(popup.Host);

var focusManager = TopLevel.GetTopLevel(tb)!.FocusManager;
tb = Assert.IsType<TextBox>(focusManager.GetFocusedElement());
var focusManager = inputRoot.FocusManager!;
Assert.Same(button, focusManager.GetFocusedElement());

//Ensure focus remains in the popup
var nextFocus = KeyboardNavigationHandler.GetNext(tb, NavigationDirection.Next);

Assert.True(nextFocus == b);
inputRoot.KeyboardNavigationHandler!.Move(focusManager.GetFocusedElement()!, NavigationDirection.Next);
Assert.Same(textBox, focusManager.GetFocusedElement());

p.Close();
popup.Close();
}
}

Expand Down Expand Up @@ -1248,7 +1241,6 @@ public void Custom_Placement_Callback_Is_Executed()
}
}


private static PopupRoot CreateRoot(TopLevel popupParent, IPopupImpl impl = null)
{
impl ??= popupParent.PlatformImpl.CreatePopup();
Expand Down Expand Up @@ -1279,7 +1271,8 @@ private IDisposable CreateServicesWithFocus()
return UnitTestApplication.Start(TestServices.StyledWindow.With(
windowingPlatform: CreateMockWindowingPlatform(),
focusManager: new FocusManager(),
keyboardDevice: () => new KeyboardDevice()));
keyboardDevice: () => new KeyboardDevice(),
keyboardNavigation: () => new KeyboardNavigationHandler()));
}


Expand Down
Loading