Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
11 changes: 7 additions & 4 deletions src/Aspire.Dashboard/Components/Controls/AspireMenu.razor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
/// </summary>
/// <remarks>
/// 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.
/// </remarks>
[Parameter]
public bool RestoreFocusOnItemClick { get; set; }
public bool RestoreFocusOnItemClick { get; set; } = true;
Comment thread
adamint marked this conversation as resolved.
Comment thread
mitchdenny marked this conversation as resolved.

protected override void OnParametersSet()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -61,18 +61,15 @@ 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<MenuButtonItem>
{
new()
{
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;
Expand All @@ -88,24 +85,20 @@ 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();
});

cut.Find($"#{anchor}").Click();
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<string>(argument)));
}

[Fact]
public void ClickItem_RestoreFocusOnItemClickFalse_DoesNotFocusAnchor()
public void ClickItem_MenuButtonWithFocusRestorationDisabled_DoesNotFocusAnchor()
{
FluentUISetupHelpers.AddCommonDashboardServices(this);
FluentUISetupHelpers.SetupFluentUIComponents(this);
Expand Down Expand Up @@ -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();
});

Expand Down
Original file line number Diff line number Diff line change
@@ -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<DashboardServerFixture>
{
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<string?>("() => document.activeElement?.id"), clearButtonId, StringComparison.Ordinal),
"Focus should return to the menu button after a menu item is selected.");
});
}
}
Loading