Fix overlay popup focus issues (#17326)

* Add failing focus tests for flyouts inside overlay popups

* Implement IKeyboardNavigationHandler on OverlayPopupHost

* Layout OverlayPopupHost content for focus to work

src/Avalonia.Base/Input/IInputRoot.cs

@@ -12,7 +12,7 @@ namespace Avalonia.Input
         /// <summary>
         /// Gets or sets the keyboard navigation handler.
         /// </summary>
-        IKeyboardNavigationHandler KeyboardNavigationHandler { get; }
+        IKeyboardNavigationHandler? KeyboardNavigationHandler { get; }
 
         /// <summary>
         /// Gets focus manager of the root.

src/Avalonia.Controls/Primitives/OverlayPopupHost.cs

@@ -2,14 +2,15 @@ using System;
 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.
@@ -19,15 +20,21 @@ namespace Avalonia.Controls.Primitives
 
         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 />
@@ -53,6 +60,38 @@ namespace Avalonia.Controls.Primitives
             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;
 
@@ -63,6 +102,12 @@ namespace Avalonia.Controls.Primitives
         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 />
@@ -147,7 +192,7 @@ namespace Avalonia.Controls.Primitives
         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)

src/Avalonia.Controls/TopLevel.cs

@@ -471,12 +471,12 @@ namespace Avalonia.Controls
         /// <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

tests/Avalonia.Controls.UnitTests/FlyoutTests.cs

@@ -20,6 +20,8 @@ namespace Avalonia.Controls.UnitTests
 {
     public class FlyoutTests
     {
+        protected bool UseOverlayPopups { get; set; }
+
         [Fact]
         public void Opening_Raises_Single_Opening_Event()
         {
@@ -373,10 +375,10 @@ namespace Avalonia.Controls.UnitTests
                 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());
             }
         }
 
@@ -640,14 +642,11 @@ namespace Avalonia.Controls.UnitTests
             }
         }
 
-        private static IDisposable CreateServicesWithFocus()
+        private IDisposable CreateServicesWithFocus()
         {
             return UnitTestApplication.Start(TestServices.StyledWindow.With(windowingPlatform:
                 new MockWindowingPlatform(null,
-                    x =>
+                    x => UseOverlayPopups ? null : MockWindowingPlatform.CreatePopupMock(x).Object),
-                    {
-                        return MockWindowingPlatform.CreatePopupMock(x).Object;
-                    }),
                     focusManager: new FocusManager(),
                     keyboardDevice: () => new KeyboardDevice()));
         }
@@ -681,4 +680,10 @@ namespace Avalonia.Controls.UnitTests
             public new Popup Popup => base.Popup;
         }
     }
+
+    public class OverlayPopupFlyoutTests : FlyoutTests
+    {
+        public OverlayPopupFlyoutTests()
+            => UseOverlayPopups = true;
+    }
 }

tests/Avalonia.Controls.UnitTests/Primitives/PopupTests.cs

@@ -633,46 +633,39 @@ namespace Avalonia.Controls.UnitTests.Primitives
         {
             using (CreateServicesWithFocus())
             {
-                var window = PreparedWindow();
+                var window = PreparedWindow(new Panel { Children = { new Slider() }});
 
-                var tb = new TextBox();
+                var textBox = new TextBox();
-                var b = new Button();
+                var button = new Button();
-                var p = new Popup
+                var popup = new Popup
                 {
                     PlacementTarget = window,
                     Child = new StackPanel
                     {
                         Children =
                         {
-                            tb,
+                            textBox,
-                            b
+                            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)
+                button.Focus();
-                {
-                    //Need to measure/arrange for visual children to show up
-                    //in OverlayPopupHost
-                    host.Measure(Size.Infinity);
-                    host.Arrange(new Rect(host.DesiredSize));
-                }
 
-                tb.Focus();
+                var inputRoot = Assert.IsAssignableFrom<IInputRoot>(popup.Host);
 
-                var focusManager = TopLevel.GetTopLevel(tb)!.FocusManager;
+                var focusManager = inputRoot.FocusManager!;
-                tb = Assert.IsType<TextBox>(focusManager.GetFocusedElement());
+                Assert.Same(button, focusManager.GetFocusedElement());
 
                 //Ensure focus remains in the popup
-                var nextFocus = KeyboardNavigationHandler.GetNext(tb, NavigationDirection.Next);
+                inputRoot.KeyboardNavigationHandler!.Move(focusManager.GetFocusedElement()!, NavigationDirection.Next);
-
+                Assert.Same(textBox, focusManager.GetFocusedElement());
-                Assert.True(nextFocus == b);
 
-                p.Close();
+                popup.Close();
             }
         }
 
@@ -1248,7 +1241,6 @@ namespace Avalonia.Controls.UnitTests.Primitives
             }
         }
 
-
         private static PopupRoot CreateRoot(TopLevel popupParent, IPopupImpl impl = null)
         {
             impl ??= popupParent.PlatformImpl.CreatePopup();
@@ -1279,7 +1271,8 @@ namespace Avalonia.Controls.UnitTests.Primitives
             return UnitTestApplication.Start(TestServices.StyledWindow.With(
                 windowingPlatform: CreateMockWindowingPlatform(),
                 focusManager: new FocusManager(),
-                keyboardDevice: () => new KeyboardDevice()));
+                keyboardDevice: () => new KeyboardDevice(),
+                keyboardNavigation: () => new KeyboardNavigationHandler()));
         }