diff --git a/src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs b/src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs index 2934332ffcf..33b818f61ab 100644 --- a/src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs +++ b/src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs @@ -114,16 +114,19 @@ public async Task OpenAsync(int screenWidth, int screenHeight, int clientX, int private async Task HandleItemClicked(MenuButtonItem item) { - if (item.OnClick is {} onClick) - { - await onClick(); - } await SetOpenAsync(false); if (RestoreFocusOnItemClick && !string.IsNullOrEmpty(Anchor)) { await JS.InvokeVoidAsync("focusElement", Anchor); } + + // Item callbacks can move focus to a dialog or another control, so restore the + // menu trigger first to avoid stealing focus back after the callback completes. + if (item.OnClick is {} onClick) + { + await onClick(); + } } private async Task OnOpenChanged(bool open) diff --git a/src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs b/src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs index 511444e755a..1443388f52d 100644 --- a/src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs +++ b/src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs @@ -55,11 +55,10 @@ public partial class AspireMenuButton : FluentComponentBase /// Gets or sets a value indicating whether focus should return to this menu button after a menu item is clicked. /// /// - /// Use this for button-anchored menus because the underlying menu anchor is the element that opened the menu. - /// Do not use this behavior for cursor-positioned or context menus where the anchor is only used for positioning. + /// Focus restoration is enabled by default because the underlying menu anchor is the button that opened the menu. /// [Parameter] - public bool RestoreFocusOnItemClick { get; set; } + public bool RestoreFocusOnItemClick { get; set; } = true; protected override void OnParametersSet() { diff --git a/tests/Aspire.Dashboard.Components.Tests/Controls/AspireMenuTests.cs b/tests/Aspire.Dashboard.Components.Tests/Controls/AspireMenuTests.cs index 98fe00c07a8..dcc2e92c246 100644 --- a/tests/Aspire.Dashboard.Components.Tests/Controls/AspireMenuTests.cs +++ b/tests/Aspire.Dashboard.Components.Tests/Controls/AspireMenuTests.cs @@ -50,7 +50,7 @@ public async Task DisposeAsync_RemovesFluentMenuFromMenuProvider() } [Fact] - public void ClickItem_RestoreFocusOnItemClickTrue_FocusesAnchor() + public void ClickItem_MenuButton_FocusesAnchorBeforeOnClick() { FluentUISetupHelpers.AddCommonDashboardServices(this); FluentUISetupHelpers.SetupFluentUIComponents(this); @@ -61,7 +61,7 @@ public void ClickItem_RestoreFocusOnItemClickTrue_FocusesAnchor() var anchor = "view-options-button"; var itemClicked = false; var focusElementInvocationHandler = JSInterop.SetupVoid("focusElement", anchor); - var focusElementInvocationsDuringOnClick = -1; + focusElementInvocationHandler.SetVoidResult(); var items = new List { new() @@ -69,10 +69,7 @@ public void ClickItem_RestoreFocusOnItemClickTrue_FocusesAnchor() Text = "Show hidden resources", OnClick = () => { - focusElementInvocationsDuringOnClick = focusElementInvocationHandler.Invocations.Count; - Assert.True( - focusElementInvocationsDuringOnClick == 0, - $"Focus should not be restored until item OnClick completes. Actual focusElement invocations during OnClick: {focusElementInvocationsDuringOnClick}."); + Assert.Single(focusElementInvocationHandler.Invocations); itemClicked = true; return Task.CompletedTask; @@ -88,7 +85,6 @@ public void ClickItem_RestoreFocusOnItemClickTrue_FocusesAnchor() builder.AddAttribute(2, nameof(AspireMenuButton.MenuButtonId), anchor); builder.AddAttribute(3, nameof(AspireMenuButton.Title), "View options"); builder.AddAttribute(4, nameof(AspireMenuButton.Items), items); - builder.AddAttribute(5, nameof(AspireMenuButton.RestoreFocusOnItemClick), true); builder.CloseComponent(); }); @@ -96,16 +92,13 @@ public void ClickItem_RestoreFocusOnItemClickTrue_FocusesAnchor() cut.WaitForElement("fluent-menu-item").Click(); Assert.True(itemClicked); - Assert.True( - focusElementInvocationsDuringOnClick == 0, - $"Expected zero focusElement invocations during item OnClick, but captured {focusElementInvocationsDuringOnClick}."); var invocation = Assert.Single(focusElementInvocationHandler.Invocations); Assert.Collection(invocation.Arguments, argument => Assert.Equal(anchor, Assert.IsType(argument))); } [Fact] - public void ClickItem_RestoreFocusOnItemClickFalse_DoesNotFocusAnchor() + public void ClickItem_MenuButtonWithFocusRestorationDisabled_DoesNotFocusAnchor() { FluentUISetupHelpers.AddCommonDashboardServices(this); FluentUISetupHelpers.SetupFluentUIComponents(this); @@ -136,6 +129,7 @@ public void ClickItem_RestoreFocusOnItemClickFalse_DoesNotFocusAnchor() builder.AddAttribute(2, nameof(AspireMenuButton.MenuButtonId), anchor); builder.AddAttribute(3, nameof(AspireMenuButton.Title), "View options"); builder.AddAttribute(4, nameof(AspireMenuButton.Items), items); + builder.AddAttribute(5, nameof(AspireMenuButton.RestoreFocusOnItemClick), false); builder.CloseComponent(); }); diff --git a/tests/Aspire.Dashboard.Tests/Integration/Playwright/AspireMenuButtonFocusTests.cs b/tests/Aspire.Dashboard.Tests/Integration/Playwright/AspireMenuButtonFocusTests.cs new file mode 100644 index 00000000000..2228dafc390 --- /dev/null +++ b/tests/Aspire.Dashboard.Tests/Integration/Playwright/AspireMenuButtonFocusTests.cs @@ -0,0 +1,55 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Aspire.Dashboard.Resources; +using Aspire.Dashboard.Tests.Integration.Playwright.Infrastructure; +using Aspire.TestUtilities; +using Microsoft.AspNetCore.InternalTesting; +using Microsoft.Playwright; +using Xunit; + +namespace Aspire.Dashboard.Tests.Integration.Playwright; + +[RequiresFeature(TestFeature.Playwright)] +public class AspireMenuButtonFocusTests : PlaywrightTestsBase +{ + public AspireMenuButtonFocusTests(DashboardServerFixture dashboardServerFixture) + : base(dashboardServerFixture) + { + } + + [Fact] + [OuterloopTest("Resource-intensive Playwright browser test")] + public async Task MenuButton_ItemSelected_RestoresFocusToMenuButton() + { + await RunTestAsync(async page => + { + await page.GotoAsync("/structuredlogs").DefaultTimeout(); + + // The structured logs "Remove data" menu relies on the AspireMenuButton default for focus + // restoration rather than opting in explicitly, so it covers the menus that regressed in + // https://github.com/microsoft/aspire/issues/17656. Its items only clear the telemetry + // repository, so nothing else legitimately claims focus and the assertion stays meaningful. + // Focus after a menu closes is a browser-only behavior that bUnit can't observe. + // + // Match the fluent-button host rather than using a role locator: aria-expanded and the id + // used for focus restoration live on the host, while the role locator resolves to the inner + // shadow DOM control that only carries the aria-label. + var clearButton = page.Locator($"fluent-button[title='{ControlsStrings.ClearSignalsButtonTitle}'][aria-haspopup='true']").First; + await Assertions.Expect(clearButton).ToHaveAttributeAsync("aria-expanded", "false"); + + var clearButtonId = await clearButton.GetAttributeAsync("id"); + Assert.False(string.IsNullOrEmpty(clearButtonId)); + + await clearButton.ClickAsync(); + await Assertions.Expect(clearButton).ToHaveAttributeAsync("aria-expanded", "true"); + + await page.Locator("fluent-menu-item#clear-menu-all").ClickAsync(); + await Assertions.Expect(clearButton).ToHaveAttributeAsync("aria-expanded", "false"); + + await AsyncTestHelpers.AssertIsTrueRetryAsync( + async () => string.Equals(await page.EvaluateAsync("() => document.activeElement?.id"), clearButtonId, StringComparison.Ordinal), + "Focus should return to the menu button after a menu item is selected."); + }); + } +} diff --git a/tests/Aspire.Hosting.Azure.Tests/AzureCosmosDBEmulatorFunctionalTests.cs b/tests/Aspire.Hosting.Azure.Tests/AzureCosmosDBEmulatorFunctionalTests.cs index f035019581d..8269e38cfdb 100644 --- a/tests/Aspire.Hosting.Azure.Tests/AzureCosmosDBEmulatorFunctionalTests.cs +++ b/tests/Aspire.Hosting.Azure.Tests/AzureCosmosDBEmulatorFunctionalTests.cs @@ -21,6 +21,7 @@ public class AzureCosmosDBEmulatorFunctionalTests(ITestOutputHelper testOutputHe // [InlineData(true)] // "Using CosmosDB emulator in integration tests leads to flaky tests - https://github.com/microsoft/aspire/issues/5820" [InlineData(false)] [RequiresFeature(TestFeature.Docker)] + [ActiveIssue("https://github.com/microsoft/aspire/issues/18898")] public async Task VerifyWaitForOnCosmosDBEmulatorBlocksDependentResources(bool usePreview) { // Cosmos can be pretty slow to spin up, lets give it plenty of time.