[MenuButton] Accessibility fixes and allow custom button content #4093

adamint · 2025-08-29T02:35:09Z

Pull Request

📖 Description

I really like this component, but it's currently unusable in Aspire because it's not keyboard-accessible. To make it accessible, the menu button needs to properly handle tab (tab on button menu is open focuses first menu item, tab when focus is on a menu item will focus the next focusable dom element outside the menu + close the menu), and shift+tab (shift+tab when focus is on the button will close the menu if open, or otherwise will focus the button if a menu item is focused).

Additionally, the button content was not customizable (no render fragment could be provided).

This PR fixes both problems. I added a new parameter ButtonContent that can be provided instead of Text.

The accessibility changes were implemented client-side because of the focusing logic requiring DOM access. I also had to fix the FluentAnchoredRegion's FocusableElement.findNextFocusableElement function to support focusing fluent components that have focusable children (since they themselves are not focusable), which I reused in the FluentMenuButton. Please refer to this before and after:

Before (see keys pressed on screen for context)

Grabacion.de.pantalla.2025-08-28.a.la.s.8.29.16.p.m.mov

After

Grabacion.de.pantalla.2025-08-28.a.la.s.8.30.51.p.m.mov

🎫 Issues

Fixes #4090

👩‍💻 Reviewer Notes

N/A

📑 Test Plan

I added a couple tests for the ButtonContent parameter, but as most of the accessibility changes are in the component JS, you will need to test manually using the MenuButton sample page.

✅ Checklist

General

I have added tests for my changes.
I have tested my changes.
I have updated the project documentation to reflect my changes.
I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

I have added a new component
I have added Unit Tests for my new component
I have modified an existing component
I have validated the Unit Tests for an existing component

⏭ Next Steps

src/Core/Components/MenuButton/FluentMenuButton.razor.cs

dvoituron · 2025-08-29T07:56:40Z

src/Core/Components/MenuButton/FluentMenuButton.razor.cs


    protected override void OnParametersSet()
    {
+        if (Text is not null && ButtonContent is not null)


I don't think you need to test this. But you must add to the Text property that this property will be ignored if ButtonContent is defined.

You cannot force the developer to have a content. The dev can create a MenuButton without content.
So, just add in the Text property comment: this property will be ignored if ButtonContent is defined

This is not correct with the new comment. You must remove this test: the dev can set a value in Text and ButtonContent. And the component will render only the ButtonContent (ignoring the Text prarameter).

src/Core/Components/MenuButton/FluentMenuButton.razor.js

…ire-icon

…sible-no-require-icon' into adamint/fluent-menu-button-accessible-no-require-icon

src/Core/Components/MenuButton/FluentMenuButton.razor.cs

src/Core/Components/MenuButton/FluentMenuButton.razor.js

…ire-icon

vnbaaij · 2025-09-08T12:25:04Z

@adamint Can you please finish this PR and fix the comments. Thanks.

…sible-no-require-icon' into adamint/fluent-menu-button-accessible-no-require-icon

adamint · 2025-09-09T19:36:54Z

@vnbaaij updated based on comments.

…ire-icon

adamint · 2025-09-09T21:40:53Z

Ok, I realized that this issue is not unique to just menu buttons and was happening to all menus. I have reverted the focus changes to the FluentMenuButton, and instead implemented them in FluentMenu so that we properly handle tab and escape in the FluentDataGrid and other uses of FluentMenu as well. I added an example of a fluent data grid that uses menus in its cell headers, which I used to help test the changes in this PR and which used to fail.

Grabacion.de.pantalla.2025-09-09.a.la.s.5.37.40.p.m.mov

src/Core/Components/Menu/FluentMenu.razor.cs

…sible-no-require-icon' into adamint/fluent-menu-button-accessible-no-require-icon

src/Core/Components/Menu/FluentMenu.razor.cs

src/Core/Components/Menu/FluentMenu.razor.js

src/Core/Components/Menu/FluentMenu.razor.cs

dvoituron · 2025-09-10T07:40:33Z

src/Core/Components/MenuButton/FluentMenuButton.razor.cs


    protected override void OnParametersSet()
    {
+        if (Text is not null && ButtonContent is not null)


You cannot force the developer to have a content. The dev can create a MenuButton without content.
So, just add in the Text property comment: this property will be ignored if ButtonContent is defined

…ire-icon

vnbaaij · 2025-09-29T12:13:58Z

Hi @adamint . Can we work to get this merged in. Please look at the latest open remarks. Thanks.

…ire-icon

dvoituron · 2025-09-30T13:23:00Z

src/Core/Components/MenuButton/FluentMenuButton.razor.cs


    protected override void OnParametersSet()
    {
+        if (Text is not null && ButtonContent is not null)


This is not correct with the new comment. You must remove this test: the dev can set a value in Text and ButtonContent. And the component will render only the ButtonContent (ignoring the Text prarameter).

…ire-icon

vnbaaij · 2025-10-02T16:41:11Z

@adamint, just one more small change requested...

…ire-icon

dvoituron

@vnbaaij will update the PR comments in another PR.

…4093) (#4221) * Remove check on Text and ButtonContent being provided, change xml doc wording, * Remove redundant test

adamint added 2 commits August 28, 2025 19:55

Make FluentMenuButton fully accessible

3f4c58f

Allow providing a render fragment for the button content

46cc752

adamint requested review from dvoituron and vnbaaij as code owners August 29, 2025 02:35

adamint mentioned this pull request Aug 29, 2025

Model selector options are not accessible using the keyboard: A11y_Aspire Dashboard_Copilot_Keyboard dotnet/aspire#10297

Open

fix bug

b23ce0c

dvoituron requested changes Aug 29, 2025

View reviewed changes

vnbaaij and others added 3 commits August 29, 2025 11:56

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

4e9c061

…ire-icon

apply pr feedback

0be2cd8

Merge remote-tracking branch 'origin/adamint/fluent-menu-button-acces…

5da3b6f

…sible-no-require-icon' into adamint/fluent-menu-button-accessible-no-require-icon

adamint requested a review from dvoituron August 29, 2025 21:21

dvoituron requested changes Aug 30, 2025

View reviewed changes

src/Core/Components/MenuButton/FluentMenuButton.razor.cs Outdated Show resolved Hide resolved

src/Core/Components/MenuButton/FluentMenuButton.razor.js Outdated Show resolved Hide resolved

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

23f626d

…ire-icon

address pr comments

e728b21

adamint requested a review from dvoituron September 9, 2025 19:36

Merge remote-tracking branch 'origin/adamint/fluent-menu-button-acces…

4ff91b0

…sible-no-require-icon' into adamint/fluent-menu-button-accessible-no-require-icon

adamint changed the title ~~Fix: make fluent menu accessible, allow custom button content~~ Fix: make fluent menu button accessible, allow custom button content Sep 9, 2025

add a data grid example using HeaderCellAsButtonWithMenu

b1764e8

adamint marked this pull request as draft September 9, 2025 20:51

adamint added 4 commits September 9, 2025 17:26

move logic up to fluent menu

ef15208

undo the menu button focus changes

3aa8de2

undo redundant changes in menu button

cb35129

close menus on esc

3cf24e3

adamint changed the title ~~Fix: make fluent menu button accessible, allow custom button content~~ Fix: make fluent menu accessible, allow custom button content Sep 9, 2025

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

7e9c6c1

…ire-icon

adamint marked this pull request as ready for review September 9, 2025 21:36

adamint commented Sep 9, 2025

View reviewed changes

src/Core/Components/Menu/FluentMenu.razor.cs Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/adamint/fluent-menu-button-acces…

910c9d7

…sible-no-require-icon' into adamint/fluent-menu-button-accessible-no-require-icon

adamint mentioned this pull request Sep 9, 2025

Sort and Resize buttons are not accessible using the keyboard keys: A11y_Aspire Dashboard_Resources_Keyboard. dotnet/aspire#10118

Open

vnbaaij changed the title ~~Fix: make fluent menu accessible, allow custom button content~~ [MenuButton] Accessibility fixes and allow custom button content Sep 11, 2025

vnbaaij requested changes Sep 11, 2025

View reviewed changes

src/Core/Components/Menu/FluentMenu.razor.cs Outdated Show resolved Hide resolved

src/Core/Components/Menu/FluentMenu.razor.js Outdated Show resolved Hide resolved

fix comment, test modules, remove redundant initializer

3c3e9ea

adamint requested a review from vnbaaij September 11, 2025 16:38

adamint mentioned this pull request Sep 11, 2025

fix: shift-tab does not work in FluentPopover, and tab/shift-tab on the popover anchor while popover is open does not close it #4127

Open

vnbaaij approved these changes Sep 11, 2025

View reviewed changes

vnbaaij added this to the v4.12.2 milestone Sep 12, 2025

dvoituron requested changes Sep 12, 2025

View reviewed changes

dvoituron and others added 2 commits September 12, 2025 15:36

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

31e61a8

…ire-icon

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

afc6e37

…ire-icon

vnbaaij and others added 2 commits September 29, 2025 15:51

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

5b4f95f

…ire-icon

remove unused method, add open parameter directly to initialize method

823a7f6

adamint requested a review from dvoituron September 29, 2025 20:10

adamint and others added 2 commits September 29, 2025 16:25

reinitialize after render

3423ed9

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

43f3151

…ire-icon

vnbaaij approved these changes Sep 30, 2025

View reviewed changes

vnbaaij enabled auto-merge (squash) September 30, 2025 12:49

dvoituron requested changes Sep 30, 2025

View reviewed changes

vnbaaij added 2 commits September 30, 2025 15:27

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

67743df

…ire-icon

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

0076144

…ire-icon

vnbaaij added 2 commits October 7, 2025 06:55

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

a2e801e

…ire-icon

Merge branch 'dev' into adamint/fluent-menu-button-accessible-no-requ…

11d7623

…ire-icon

dvoituron approved these changes Oct 7, 2025

View reviewed changes

vnbaaij disabled auto-merge October 7, 2025 13:46

vnbaaij merged commit 4933136 into microsoft:dev Oct 7, 2025
2 checks passed

dvoituron pushed a commit that referenced this pull request Oct 7, 2025

[MenuButton] Finalize work on a11y fixes and custom button content (#…

5ffe060

…4093) (#4221) * Remove check on Text and ButtonContent being provided, change xml doc wording, * Remove redundant test

[MenuButton] Accessibility fixes and allow custom button content #4093

[MenuButton] Accessibility fixes and allow custom button content #4093

Uh oh!

Conversation

adamint commented Aug 29, 2025

Pull Request

📖 Description

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

Component-specific

⏭ Next Steps

Uh oh!

Uh oh!

dvoituron Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

adamint Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

dvoituron Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

dvoituron Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vnbaaij commented Sep 8, 2025

Uh oh!

adamint commented Sep 9, 2025

Uh oh!

adamint commented Sep 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dvoituron Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

vnbaaij commented Sep 29, 2025

Uh oh!

dvoituron Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

vnbaaij commented Oct 2, 2025

Uh oh!

dvoituron left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants