Skip to content

Commit 629fe71

Browse files
authored
fix(aria/menu): defer menu item focus in case menus in cdk overlay (#33258)
When menus are placed in a `CdkConnectedOverlay`, the menu trigger tries to focus the first menu item even before the menus are rendered and registered to the menu trigger.
1 parent 905aeb5 commit 629fe71

12 files changed

Lines changed: 268 additions & 3 deletions

File tree

goldens/aria/private/index.api.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,8 @@ export class MenuTriggerPattern<V> {
497497
first?: boolean;
498498
last?: boolean;
499499
}): void;
500+
readonly pendingFocus: WritableSignalLike<"first" | "last" | undefined>;
501+
pendingFocusEffect(): void;
500502
readonly role: () => string;
501503
readonly tabIndex: SignalLike<-1 | 0>;
502504
}

src/aria/menu/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ ng_project(
2525
),
2626
deps = [
2727
":menu",
28+
"//:node_modules/@angular/common",
2829
"//:node_modules/@angular/core",
2930
"//:node_modules/@angular/platform-browser",
3031
"//:node_modules/axe-core",
3132
"//src/aria/private/testing",
33+
"//src/cdk/overlay",
3234
"//src/cdk/testing/private",
3335
],
3436
)

src/aria/menu/menu-trigger.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ export class MenuTrigger<V> {
8989

9090
constructor() {
9191
effect(() => this.menu()?.parent.set(this));
92+
effect(() => this._pattern.pendingFocusEffect());
9293
}
9394

9495
/** Opens the menu focusing on the first menu item. */

src/aria/menu/menu.spec.ts

Lines changed: 151 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
1-
import {Component, DebugElement, ChangeDetectionStrategy, signal} from '@angular/core';
1+
import {
2+
Component,
3+
DebugElement,
4+
ChangeDetectionStrategy,
5+
signal,
6+
ViewChild,
7+
inject,
8+
ChangeDetectorRef,
9+
} from '@angular/core';
10+
import {CommonModule} from '@angular/common';
11+
import {OverlayModule} from '@angular/cdk/overlay';
212
import {ComponentFixture, TestBed} from '@angular/core/testing';
313
import {By} from '@angular/platform-browser';
414
import {provideFakeDirectionality} from '@angular/cdk/testing/private';
@@ -741,6 +751,96 @@ describe('Menu Trigger Pattern', () => {
741751
});
742752
});
743753

754+
describe('CDK Overlay Menu Pattern', () => {
755+
let fixture: ComponentFixture<CdkOverlayMenuExample>;
756+
757+
const focusin = (element: Element) => {
758+
element.dispatchEvent(new FocusEvent('focusin', {bubbles: true}));
759+
fixture.detectChanges();
760+
};
761+
762+
const keydown = async (element: Element, key: string, modifierKeys: {} = {}) => {
763+
focusin(element);
764+
element.dispatchEvent(
765+
new KeyboardEvent('keydown', {
766+
key,
767+
bubbles: true,
768+
...modifierKeys,
769+
}),
770+
);
771+
fixture.detectChanges();
772+
await waitForMicrotasks();
773+
fixture.detectChanges();
774+
};
775+
776+
const click = async (element: Element, eventInit?: PointerEventInit) => {
777+
focusin(element);
778+
element.dispatchEvent(new PointerEvent('click', {bubbles: true, ...eventInit}));
779+
fixture.detectChanges();
780+
await waitForMicrotasks();
781+
fixture.detectChanges();
782+
};
783+
784+
function setupMenu() {
785+
fixture = TestBed.createComponent(CdkOverlayMenuExample);
786+
fixture.detectChanges();
787+
}
788+
789+
function getTrigger(): HTMLElement {
790+
return fixture.debugElement.query(By.directive(MenuTrigger)).nativeElement as HTMLElement;
791+
}
792+
793+
function getItem(text: string): HTMLElement | null {
794+
const items = fixture.debugElement
795+
.queryAll(By.directive(MenuItem))
796+
.map((debugEl: DebugElement) => debugEl.nativeElement as HTMLElement);
797+
return items.find(item => item.textContent?.trim() === text) || null;
798+
}
799+
800+
beforeEach(() => setupMenu());
801+
802+
it('should focus the first item when opened via arrow down', async () => {
803+
await keydown(getTrigger(), 'ArrowDown');
804+
expect(document.activeElement).toBe(getItem('Apple'));
805+
});
806+
807+
it('should focus the first item when opened via enter', async () => {
808+
await keydown(getTrigger(), 'Enter');
809+
expect(document.activeElement).toBe(getItem('Apple'));
810+
});
811+
812+
it('should focus the first item when opened via space', async () => {
813+
await keydown(getTrigger(), ' ');
814+
expect(document.activeElement).toBe(getItem('Apple'));
815+
});
816+
817+
it('should focus the first item when opened via click', async () => {
818+
await click(getTrigger());
819+
expect(document.activeElement).toBe(getItem('Apple'));
820+
});
821+
822+
it('should focus the first item stably when opened, closed via escape, and opened again', async () => {
823+
const trigger = getTrigger();
824+
825+
// First open
826+
await keydown(trigger, 'Enter');
827+
expect(document.activeElement).toBe(getItem('Apple'));
828+
829+
// Close via escape
830+
await keydown(getItem('Apple')!, 'Escape');
831+
expect(trigger.getAttribute('aria-expanded')).toBe('false');
832+
expect(document.activeElement).toBe(trigger);
833+
834+
// Explicitly clear cached menu before second open
835+
fixture.componentInstance.clearMenu();
836+
fixture.detectChanges();
837+
838+
// Second open
839+
await keydown(trigger, 'Enter');
840+
expect(document.activeElement).toBe(getItem('Apple'));
841+
});
842+
});
843+
744844
describe('Menu Bar Pattern', () => {
745845
let fixture: ComponentFixture<MenuBarExample>;
746846

@@ -1254,3 +1354,53 @@ class MenuWithDuplicateValues {}
12541354
changeDetection: ChangeDetectionStrategy.Eager,
12551355
})
12561356
class MenuItemOutsideMenu {}
1357+
1358+
@Component({
1359+
template: `
1360+
<ng-container *ngTemplateOutlet="menuTemplate"></ng-container>
1361+
1362+
<ng-template #menuTemplate>
1363+
<button
1364+
ngMenuTrigger
1365+
#menuTrigger="ngMenuTrigger"
1366+
[menu]="myMenu"
1367+
cdkOverlayOrigin
1368+
#origin="cdkOverlayOrigin"
1369+
>
1370+
Open Menu
1371+
</button>
1372+
1373+
<ng-template
1374+
cdkConnectedOverlay
1375+
[cdkConnectedOverlayOrigin]="origin"
1376+
[cdkConnectedOverlayOpen]="menuTrigger.expanded()"
1377+
>
1378+
<div ngMenu #overlayMenu="ngMenu">
1379+
<ng-template ngMenuContent>
1380+
<div ngMenuItem value="Apple" searchTerm="Apple">Apple</div>
1381+
<div ngMenuItem value="Banana" searchTerm="Banana">Banana</div>
1382+
</ng-template>
1383+
</div>
1384+
</ng-template>
1385+
</ng-template>
1386+
`,
1387+
imports: [CommonModule, OverlayModule, Menu, MenuTrigger, MenuItem, MenuContent],
1388+
changeDetection: ChangeDetectionStrategy.Eager,
1389+
})
1390+
class CdkOverlayMenuExample {
1391+
@ViewChild('overlayMenu') _myMenu!: Menu<any>;
1392+
private _cachedMenu?: Menu<any>;
1393+
private readonly _cdr = inject(ChangeDetectorRef);
1394+
1395+
get myMenu() {
1396+
if (this._myMenu) {
1397+
this._cachedMenu = this._myMenu;
1398+
}
1399+
return this._cachedMenu;
1400+
}
1401+
1402+
clearMenu() {
1403+
this._cachedMenu = undefined;
1404+
this._cdr.markForCheck();
1405+
}
1406+
}

src/aria/private/menu/menu.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,19 @@ function getMenuTriggerPattern(opts?: {textDirection: 'ltr' | 'rtl'}) {
4646
menu: submenu,
4747
disabled: signal(false),
4848
});
49+
50+
const originalOnClick = trigger.onClick.bind(trigger);
51+
trigger.onClick = () => {
52+
originalOnClick();
53+
trigger.pendingFocusEffect();
54+
};
55+
56+
const originalOnKeydown = trigger.onKeydown.bind(trigger);
57+
trigger.onKeydown = (event: KeyboardEvent) => {
58+
originalOnKeydown(event);
59+
trigger.pendingFocusEffect();
60+
};
61+
4962
return trigger;
5063
}
5164

src/aria/private/menu/menu.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,9 @@ export class MenuTriggerPattern<V> {
638638
/** Whether the menu trigger has received interaction. */
639639
readonly hasBeenInteracted = signal(false);
640640

641+
/** The pending focus target when the menu is opened before the menu instance is available. */
642+
readonly pendingFocus = signal<'first' | 'last' | undefined>(undefined);
643+
641644
/** The role of the menu trigger. */
642645
readonly role = () => 'button';
643646

@@ -669,6 +672,20 @@ export class MenuTriggerPattern<V> {
669672
this.menu = this.inputs.menu;
670673
}
671674

675+
/** Flushes any pending focus when the menu instance becomes available. */
676+
pendingFocusEffect(): void {
677+
const menu = this.inputs.menu();
678+
const intent = this.pendingFocus();
679+
if (menu && intent) {
680+
if (intent === 'first') {
681+
menu.first();
682+
} else if (intent === 'last') {
683+
menu.last();
684+
}
685+
this.pendingFocus.set(undefined);
686+
}
687+
}
688+
672689
/** Handles keyboard events for the menu trigger. */
673690
onKeydown(event: KeyboardEvent) {
674691
if (!this.inputs.disabled()) {
@@ -708,15 +725,16 @@ export class MenuTriggerPattern<V> {
708725
this.expanded.set(true);
709726

710727
if (opts?.first) {
711-
this.inputs.menu()?.first();
728+
this.pendingFocus.set('first');
712729
} else if (opts?.last) {
713-
this.inputs.menu()?.last();
730+
this.pendingFocus.set('last');
714731
}
715732
}
716733

717734
/** Closes the menu. */
718735
close(opts: {refocus?: boolean} = {}) {
719736
this.expanded.set(false);
737+
this.pendingFocus.set(undefined);
720738
this.menu()?.listBehavior.unfocus();
721739

722740
if (opts.refocus) {

src/components-examples/aria/menu/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ ng_project(
1010
"**/*.css",
1111
]),
1212
deps = [
13+
"//:node_modules/@angular/common",
1314
"//:node_modules/@angular/core",
1415
"//src/aria/menu",
1516
"//src/cdk/a11y",

src/components-examples/aria/menu/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@ export {MenuTriggerExample} from './menu-trigger/menu-trigger-example';
33
export {MenuTriggerDisabledExample} from './menu-trigger-disabled/menu-trigger-disabled-example';
44
export {MenuStandaloneExample} from './menu-standalone/menu-standalone-example';
55
export {MenuStandaloneDisabledExample} from './menu-standalone-disabled/menu-standalone-disabled-example';
6+
export {MenuCdkOverlayExample} from './menu-cdk-overlay/menu-cdk-overlay-example';
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!-- Stamped out dynamically via *ngTemplateOutlet -->
2+
<ng-container *ngTemplateOutlet="menuTemplate"></ng-container>
3+
4+
<ng-template #menuTemplate>
5+
<div class="trigger-container">
6+
<button
7+
ngMenuTrigger
8+
#menuTrigger="ngMenuTrigger"
9+
[menu]="myMenu"
10+
cdkOverlayOrigin
11+
#origin="cdkOverlayOrigin"
12+
aria-label="Open menu"
13+
class="example-menu-trigger"
14+
>
15+
<span aria-hidden="true" class="material-symbols-outlined example-icon">menu</span>
16+
<span style="margin-left: 8px; font-size: 0.875rem;">Open Menu</span>
17+
</button>
18+
19+
<!-- Dynamic Popover Portal -->
20+
<ng-template
21+
cdkConnectedOverlay
22+
[cdkConnectedOverlayOrigin]="origin"
23+
[cdkConnectedOverlayOpen]="menuTrigger.expanded()"
24+
>
25+
<div ngMenu #myMenu="ngMenu" class="example-menu" [style.width]="'200px'">
26+
<ng-template ngMenuContent>
27+
<div ng-menu-item value="Item 1">
28+
<span ng-menu-item-icon>star</span>
29+
<span ng-menu-item-text>Item 1</span>
30+
</div>
31+
<div ng-menu-item value="Item 2">
32+
<span ng-menu-item-icon>settings</span>
33+
<span ng-menu-item-text>Item 2</span>
34+
</div>
35+
<div ng-menu-item value="Item 3">
36+
<span ng-menu-item-icon>help</span>
37+
<span ng-menu-item-text>Item 3</span>
38+
</div>
39+
</ng-template>
40+
</div>
41+
</ng-template>
42+
</div>
43+
</ng-template>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import {Component, ViewChild} from '@angular/core';
2+
import {CommonModule} from '@angular/common';
3+
import {OverlayModule} from '@angular/cdk/overlay';
4+
import {Menu, MenuTrigger, MenuContent} from '@angular/aria/menu';
5+
import {SimpleMenuItem, SimpleMenuItemIcon, SimpleMenuItemText} from '../simple-menu';
6+
7+
/**
8+
* @title Menu CDK overlay example
9+
*/
10+
@Component({
11+
selector: 'menu-cdk-overlay-example',
12+
templateUrl: 'menu-cdk-overlay-example.html',
13+
styleUrl: '../menu-example.css',
14+
imports: [
15+
CommonModule,
16+
OverlayModule,
17+
Menu,
18+
MenuTrigger,
19+
MenuContent,
20+
SimpleMenuItem,
21+
SimpleMenuItemIcon,
22+
SimpleMenuItemText,
23+
],
24+
})
25+
export class MenuCdkOverlayExample {
26+
@ViewChild('myMenu') myMenu!: Menu<any>;
27+
}

0 commit comments

Comments
 (0)