Skip to content

fix(DropdownMenu): a disabled trigger never opens the menu #7475

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.

Sign up for GitHub

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

Merged
cixzhang merged 4 commits into from
Oct 13, 2026
Merged

fix(DropdownMenu): a disabled trigger never opens the menu #7475

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
642b9ca
fix(DropdownMenu): a disabled trigger never opens the menu
cixzhang Oct 13, 2026
036a208
fix(DropdownMenu): a refused open clears the ArrowUp landing flag; th…
cixzhang Oct 13, 2026
4cb0a80
fix(DropdownMenu): bottom-sheet disabled custom-trigger regression te…
cixzhang Oct 13, 2026
935c63d
Merge remote-tracking branch 'origin/main' into fix/dropdownmenu-disa…
cixzhang Oct 13, 2026
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
7 changes: 7 additions & 0 deletions .changeset/dropdownmenu-disabled-trigger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] `DropdownMenu` and `MoreMenu`: a disabled trigger no longer opens the menu. A disabled trigger with a tooltip stays focusable (`aria-disabled`), and Button swallowed only Enter and Space, so ArrowDown, ArrowUp, a press, or a click could still open the menu; every open path now checks the trigger's availability first.

@astracat-bot astracat-bot Bot Oct 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"every open path now checks the trigger's availability first" covers only the anchored menu. In the bottom-sheet presentation the trigger opens through handleSheetTriggerClick, which has no availability check, so a caller-rendered aria-disabled control still opens the sheet. The built-in disabled trigger stays closed there only because Button's click guard runs first. Please narrow the wording, or add the same check to the sheet's click handler.


@cixzhang
12 changes: 11 additions & 1 deletion packages/core/src/DropdownMenu/DropdownMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1190,6 +1190,16 @@ function DropdownMenuPopover({
modality: 'keyboard' | 'pointer' = 'keyboard',
notifyClick = false,
): boolean => {
// An unavailable trigger never opens the menu. Button keeps an
// aria-disabled trigger focusable for its tooltip and swallows only
// Enter and Space, so the arrow keys, a press and a click would still
// arrive here; a custom trigger may be natively disabled instead.
if (
button.isDisabled === true ||

@astracat-bot astracat-bot Bot Oct 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: the selector on the next line already matches the built-in Button's disabled or aria-disabled state, so every menu suite stays green without this clause. Keeping it also stops an enabled renderTrigger control from opening the menu when button is passed alongside it with isDisabled, although the caller's control is meant to win. Consider dropping it here and in the bottom-sheet guard.

buttonRef.current?.matches(':disabled, [aria-disabled="true"]') === true

@astracat-bot astracat-bot Bot Oct 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clause is the only thing protecting caller-rendered renderTrigger controls (and loading or group-disabled built-in triggers), but no test covers it. With this line deleted, all 344 tests in the DropdownMenu, MoreMenu, ContextMenu and NavMenu suites still pass. Please add a custom-trigger case, for example with the trigger below, asserting that ArrowDown, a click and a mouse press each leave aria-expanded="false".

renderTrigger={props => (
  <button type="button" {...props} aria-disabled="true">
    More
  </button>
)}

) {
return false;

@astracat-bot astracat-bot Bot Oct 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This early return leaves focusLastOnOpenRef set when the refused key was ArrowUp, because handleButtonKeyDown sets it before calling here. Once the trigger is enabled, the next open that does not set the flag again lands focus on the last row instead of the first. That includes an AT activation (a detail 0 click, which this component treats as a keyboard open) and a controlled isMenuOpen open. In jsdom: ArrowUp while disabled, then enable, then either open → "Last". Without the earlier ArrowUp → "First". Clearing the flag on refusal fixes it:

focusLastOnOpenRef.current = false;
return false;

The existing rejected-show branch below has the same gap.

}
acceptedOpenRef.current = false;
notifyClickOnShowRef.current = notifyClick;
pendingControlledOpenRef.current = isControlled;
Expand All @@ -1205,7 +1215,7 @@ function DropdownMenuPopover({
}
return true;
},
[isControlled, popover],
[button.isDisabled, isControlled, popover],
);

const isMenuOpenNow = isControlled
Expand Down
16 changes: 16 additions & 0 deletions packages/core/src/MoreMenu/MoreMenu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,22 @@ describe('MoreMenu', () => {
expect(button).toHaveAttribute('aria-disabled', 'true');
});

it('keeps the menu closed from every opening key and press while disabled', async () => {
const user = userEvent.setup();
render(<MoreMenu items={defaultItems} isDisabled />);
const button = screen.getByRole('button', {name: 'More options'});
button.focus();
for (const key of ['{Enter}', ' ', '{ArrowDown}', '{ArrowUp}']) {
await user.keyboard(key);
expect(button).toHaveAttribute('aria-expanded', 'false');
}
await user.pointer([
{keys: '[MouseLeft>]', target: button},
{keys: '[/MouseLeft]'},
]);
expect(button).toHaveAttribute('aria-expanded', 'false');
});

it('supports custom label', () => {
render(<MoreMenu items={defaultItems} label="Row actions" />);
expect(
Expand Down
Loading