Skip to content

Commit af6c13f

Browse files
crisbetojelbourn
authored andcommitted
fix(menu): internal focus state out of sync if item is focused programmatically (#17762)
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 fe151e6 commit af6c13f

File tree

5 files changed

+79
-0
lines changed

5 files changed

+79
-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);
@@ -2454,6 +2485,7 @@ class MenuWithCheckboxItems {
24542485
class SimpleMenuWithRepeater {
24552486
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
24562487
@ViewChild(MatMenu) menu: MatMenu;
2488+
@ViewChildren(MatMenuItem) itemInstances: QueryList<MatMenuItem>;
24572489
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
24582490
}
24592491

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
@@ -943,6 +943,37 @@ describe('MatMenu', () => {
943943
flush();
944944
}));
945945

946+
it('should sync the focus order when an item is focused programmatically', fakeAsync(() => {
947+
const fixture = createComponent(SimpleMenuWithRepeater);
948+
949+
// Add some more items to work with.
950+
for (let i = 0; i < 5; i++) {
951+
fixture.componentInstance.items.push({label: `Extra ${i}`, disabled: false});
952+
}
953+
954+
fixture.detectChanges();
955+
fixture.componentInstance.trigger.openMenu();
956+
fixture.detectChanges();
957+
tick(500);
958+
959+
const menuPanel = document.querySelector('.mat-menu-panel')!;
960+
const items = menuPanel.querySelectorAll('.mat-menu-panel [mat-menu-item]');
961+
962+
expect(document.activeElement).toBe(items[0], 'Expected first item to be focused on open');
963+
964+
fixture.componentInstance.itemInstances.toArray()[3].focus();
965+
fixture.detectChanges();
966+
967+
expect(document.activeElement).toBe(items[3], 'Expected fourth item to be focused');
968+
969+
dispatchKeyboardEvent(menuPanel, 'keydown', DOWN_ARROW);
970+
fixture.detectChanges();
971+
tick();
972+
973+
expect(document.activeElement).toBe(items[4], 'Expected fifth item to be focused');
974+
flush();
975+
}));
976+
946977
it('should open submenus when the menu is inside an OnPush component', fakeAsync(() => {
947978
const fixture = createComponent(LazyMenuWithOnPush);
948979
fixture.detectChanges();
@@ -2442,6 +2473,7 @@ class MenuWithCheckboxItems {
24422473
class SimpleMenuWithRepeater {
24432474
@ViewChild(MatMenuTrigger) trigger: MatMenuTrigger;
24442475
@ViewChild(MatMenu) menu: MatMenu;
2476+
@ViewChildren(MatMenuItem) itemInstances: QueryList<MatMenuItem>;
24452477
items = [{label: 'Pizza', disabled: false}, {label: 'Pasta', disabled: false}];
24462478
}
24472479

src/material/menu/menu.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,14 @@ 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+
// If a user manually (programatically) focuses a menu item, we need to reflect that focus
256+
// change back to the key manager. Note that we don't need to unsubscribe here because _focused
257+
// is internal and we know that it gets completed on destroy.
258+
this._directDescendantItems.changes.pipe(
259+
startWith(this._directDescendantItems),
260+
switchMap(items => merge<MatMenuItem>(...items.map((item: MatMenuItem) => item._focused)))
261+
).subscribe(focusedItem => this._keyManager.updateActiveItem(focusedItem));
254262
}
255263

256264
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)