Skip to content

Commit 531f468

Browse files
committed
fix(menu): internal focus state out of sync if item is focused programmatically
Fixes the internal key manager of the menu being out of sync if one of the menu items is focused via its `focus` method. Fixes #17761.
1 parent 5428e93 commit 531f468

File tree

5 files changed

+78
-0
lines changed

5 files changed

+78
-0
lines changed

src/material-experimental/mdc-menu/menu.spec.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -817,6 +817,37 @@ describe('MDC-based MatMenu', () => {
817817
flush();
818818
}));
819819

820+
it('should sync the focus order when an item is focused programmatically', fakeAsync(() => {
821+
const fixture = createComponent(SimpleMenuWithRepeater);
822+
823+
// Add some more items to work with.
824+
for (let i = 0; i < 5; i++) {
825+
fixture.componentInstance.items.push({label: `Extra ${i}`, disabled: false});
826+
}
827+
828+
fixture.detectChanges();
829+
fixture.componentInstance.trigger.openMenu();
830+
fixture.detectChanges();
831+
tick(500);
832+
833+
const menuPanel = document.querySelector('.mat-mdc-menu-panel')!;
834+
const items = menuPanel.querySelectorAll('.mat-mdc-menu-panel [mat-menu-item]');
835+
836+
expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open');
837+
838+
fixture.componentInstance.itemInstances.toArray()[3].focus();
839+
fixture.detectChanges();
840+
841+
expect(document.activeElement).toBe(items[3], 'Expected fourth item to be focused');
842+
843+
dispatchKeyboardEvent(menuPanel, 'keydown', DOWN_ARROW);
844+
fixture.detectChanges();
845+
tick();
846+
847+
expect(document.activeElement).toBe(items[4], 'Expected fifth item to be focused');
848+
flush();
849+
}));
850+
820851
it('should focus the menu panel if all items are disabled', fakeAsync(() => {
821852
const fixture = createComponent(SimpleMenuWithRepeater, [], [FakeIcon]);
822853
fixture.componentInstance.items.forEach(item => item.disabled = true);
@@ -2443,5 +2474,6 @@ class MenuWithCheckboxItems {
24432474
class SimpleMenuWithRepeater {
24442475
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
24452476
@ViewChild(MatMenu) menu: MatMenu;
2477+
@ViewChildren(MatMenuItem) itemInstances: QueryList<MatMenuItem>;
24462478
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
24472479
}

src/material/menu/menu-item.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ export class MatMenuItem extends _MatMenuItemMixinBase
6666
/** Stream that emits when the menu item is hovered. */
6767
readonly _hovered: Subject<MatMenuItem> = new Subject<MatMenuItem>();
6868

69+
/** Stream that emits when the menu item is focused. */
70+
readonly _focused = new Subject<MatMenuItem>();
71+
6972
/** Whether the menu item is highlighted. */
7073
_highlighted: boolean = false;
7174

@@ -102,6 +105,8 @@ export class MatMenuItem extends _MatMenuItemMixinBase
102105
} else {
103106
this._getHostElement().focus(options);
104107
}
108+
109+
this._focused.next(this);
105110
}
106111

107112
ngOnDestroy() {
@@ -114,6 +119,7 @@ export class MatMenuItem extends _MatMenuItemMixinBase
114119
}
115120

116121
this._hovered.complete();
122+
this._focused.complete();
117123
}
118124

119125
/** Used to set the `tabindex`. */

src/material/menu/menu.spec.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,37 @@ describe('MatMenu', () => {
933933
flush();
934934
}));
935935

936+
it('should sync the focus order when an item is focused programmatically', fakeAsync(() => {
937+
const fixture = createComponent(SimpleMenuWithRepeater);
938+
939+
// Add some more items to work with.
940+
for (let i = 0; i < 5; i++) {
941+
fixture.componentInstance.items.push({label: `Extra ${i}`, disabled: false});
942+
}
943+
944+
fixture.detectChanges();
945+
fixture.componentInstance.trigger.openMenu();
946+
fixture.detectChanges();
947+
tick(500);
948+
949+
const menuPanel = document.querySelector('.mat-menu-panel')!;
950+
const items = menuPanel.querySelectorAll('.mat-menu-panel [mat-menu-item]');
951+
952+
expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open');
953+
954+
fixture.componentInstance.itemInstances.toArray()[3].focus();
955+
fixture.detectChanges();
956+
957+
expect(document.activeElement).toBe(items[3], 'Expected fourth item to be focused');
958+
959+
dispatchKeyboardEvent(menuPanel, 'keydown', DOWN_ARROW);
960+
fixture.detectChanges();
961+
tick();
962+
963+
expect(document.activeElement).toBe(items[4], 'Expected fifth item to be focused');
964+
flush();
965+
}));
966+
936967
it('should open submenus when the menu is inside an OnPush component', fakeAsync(() => {
937968
const fixture = createComponent(LazyMenuWithOnPush);
938969
fixture.detectChanges();
@@ -2432,6 +2463,7 @@ class MenuWithCheckboxItems {
24322463
class SimpleMenuWithRepeater {
24332464
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
24342465
@ViewChild(MatMenu) menu: MatMenu;
2466+
@ViewChildren(MatMenuItem) itemInstances: QueryList<MatMenuItem>;
24352467
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
24362468
}
24372469

src/material/menu/menu.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,13 @@ export class _MatMenuBase implements AfterContentInit, MatMenuPanel<MatMenuItem>
251251
this._updateDirectDescendants();
252252
this._keyManager = new FocusKeyManager(this._directDescendantItems).withWrap().withTypeAhead();
253253
this._tabSubscription = this._keyManager.tabOut.subscribe(() => this.closed.emit('tab'));
254+
255+
// We don't need to unsubscribe here because _focused is
256+
// internal and we know that it gets completed on destroy.
257+
this._directDescendantItems.changes.pipe(
258+
startWith(this._directDescendantItems),
259+
switchMap(items => merge<MatMenuItem>(...items.map((item: MatMenuItem) => item._focused)))
260+
).subscribe(focusedItem => this._keyManager.updateActiveItem(focusedItem));
254261
}
255262

256263
ngOnDestroy() {

tools/public_api_guard/material/menu.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ export interface MatMenuDefaultOptions {
9191
}
9292

9393
export declare class MatMenuItem extends _MatMenuItemMixinBase implements FocusableOption, CanDisable, CanDisableRipple, OnDestroy {
94+
readonly _focused: Subject<MatMenuItem>;
9495
_highlighted: boolean;
9596
readonly _hovered: Subject<MatMenuItem>;
9697
_parentMenu?: MatMenuPanel<MatMenuItem> | undefined;

0 commit comments

Comments
 (0)