Skip to content

Commit 2c33108

Browse files
committed
fix(menu): unable to open same sub-menu from different triggers and not picking up indirect descendant items
* Reworks the relationship between the menu items and the menu panel to allow for the items to be indirect descendants of the menu, while also allowing for `mat-menu` instances to be declared inside of other `mat-menu` instances without having the root `mat-menu` pick up all of the descendant items. * Adds the ability to pass in an array to the `ListKeyManager`, in addition to a `QueryList`. * Fixes not being able to re-use the same sub-menu between multiple sub-menu triggers. Fixes #9969. Fixes #9987.
1 parent 2046b7a commit 2c33108

File tree

7 files changed

+288
-55
lines changed

7 files changed

+288
-55
lines changed

src/cdk/a11y/key-manager/list-key-manager.ts

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,22 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
5252
// Buffer for the letters that the user has pressed when the typeahead option is turned on.
5353
private _pressedLetters: string[] = [];
5454

55-
constructor(private _items: QueryList<T>) {
56-
_items.changes.subscribe((newItems: QueryList<T>) => {
57-
if (this._activeItem) {
58-
const itemArray = newItems.toArray();
59-
const newIndex = itemArray.indexOf(this._activeItem);
60-
61-
if (newIndex > -1 && newIndex !== this._activeItemIndex) {
62-
this._activeItemIndex = newIndex;
55+
constructor(private _items: QueryList<T> | T[]) {
56+
// We allow for the items to be an array because, in some cases, the consumer may
57+
// not have access to a QueryList of the items they want to manage (e.g. when the
58+
// items aren't being collected via `ViewChildren` or `ContentChildren`).
59+
if (_items instanceof QueryList) {
60+
_items.changes.subscribe((newItems: QueryList<T>) => {
61+
if (this._activeItem) {
62+
const itemArray = newItems.toArray();
63+
const newIndex = itemArray.indexOf(this._activeItem);
64+
65+
if (newIndex > -1 && newIndex !== this._activeItemIndex) {
66+
this._activeItemIndex = newIndex;
67+
}
6368
}
64-
}
65-
});
69+
});
70+
}
6671
}
6772

6873
/**
@@ -132,7 +137,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
132137
filter(() => this._pressedLetters.length > 0),
133138
map(() => this._pressedLetters.join(''))
134139
).subscribe(inputString => {
135-
const items = this._items.toArray();
140+
const items = this._getItemsArray();
136141

137142
// Start at 1 because we want to start searching at the item immediately
138143
// following the current active item.
@@ -288,7 +293,7 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
288293
updateActiveItem(item: T): void;
289294

290295
updateActiveItem(item: any): void {
291-
const itemArray = this._items.toArray();
296+
const itemArray = this._getItemsArray();
292297
const index = typeof item === 'number' ? item : itemArray.indexOf(item);
293298

294299
this._activeItemIndex = index;
@@ -310,17 +315,18 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
310315
* currently active item and the new active item. It will calculate differently
311316
* depending on whether wrap mode is turned on.
312317
*/
313-
private _setActiveItemByDelta(delta: -1 | 1, items = this._items.toArray()): void {
314-
this._wrap ? this._setActiveInWrapMode(delta, items)
315-
: this._setActiveInDefaultMode(delta, items);
318+
private _setActiveItemByDelta(delta: -1 | 1): void {
319+
this._wrap ? this._setActiveInWrapMode(delta) : this._setActiveInDefaultMode(delta);
316320
}
317321

318322
/**
319323
* Sets the active item properly given "wrap" mode. In other words, it will continue to move
320324
* down the list until it finds an item that is not disabled, and it will wrap if it
321325
* encounters either end of the list.
322326
*/
323-
private _setActiveInWrapMode(delta: -1 | 1, items: T[]): void {
327+
private _setActiveInWrapMode(delta: -1 | 1): void {
328+
const items = this._getItemsArray();
329+
324330
for (let i = 1; i <= items.length; i++) {
325331
const index = (this._activeItemIndex + (delta * i) + items.length) % items.length;
326332
const item = items[index];
@@ -337,17 +343,18 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
337343
* continue to move down the list until it finds an item that is not disabled. If
338344
* it encounters either end of the list, it will stop and not wrap.
339345
*/
340-
private _setActiveInDefaultMode(delta: -1 | 1, items: T[]): void {
341-
this._setActiveItemByIndex(this._activeItemIndex + delta, delta, items);
346+
private _setActiveInDefaultMode(delta: -1 | 1): void {
347+
this._setActiveItemByIndex(this._activeItemIndex + delta, delta);
342348
}
343349

344350
/**
345351
* Sets the active item to the first enabled item starting at the index specified. If the
346352
* item is disabled, it will move in the fallbackDelta direction until it either
347353
* finds an enabled item or encounters the end of the list.
348354
*/
349-
private _setActiveItemByIndex(index: number, fallbackDelta: -1 | 1,
350-
items = this._items.toArray()): void {
355+
private _setActiveItemByIndex(index: number, fallbackDelta: -1 | 1): void {
356+
const items = this._getItemsArray();
357+
351358
if (!items[index]) {
352359
return;
353360
}
@@ -362,4 +369,9 @@ export class ListKeyManager<T extends ListKeyManagerOption> {
362369

363370
this.setActiveItem(index);
364371
}
372+
373+
/** Returns the items as an array. */
374+
private _getItemsArray(): T[] {
375+
return this._items instanceof QueryList ? this._items.toArray() : this._items;
376+
}
365377
}

src/lib/menu/menu-directive.ts

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import {
2525
OnDestroy,
2626
OnInit,
2727
Output,
28-
QueryList,
2928
TemplateRef,
29+
QueryList,
3030
ViewChild,
3131
ViewEncapsulation,
3232
} from '@angular/core';
@@ -36,7 +36,7 @@ import {matMenuAnimations} from './menu-animations';
3636
import {MatMenuContent} from './menu-content';
3737
import {throwMatMenuInvalidPositionX, throwMatMenuInvalidPositionY} from './menu-errors';
3838
import {MatMenuItem} from './menu-item';
39-
import {MatMenuPanel} from './menu-panel';
39+
import {MAT_MENU_PANEL, MatMenuPanel} from './menu-panel';
4040
import {MenuPositionX, MenuPositionY} from './menu-positions';
4141

4242

@@ -84,18 +84,28 @@ const MAT_MENU_BASE_ELEVATION = 2;
8484
styleUrls: ['menu.css'],
8585
changeDetection: ChangeDetectionStrategy.OnPush,
8686
encapsulation: ViewEncapsulation.None,
87+
preserveWhitespaces: false,
88+
exportAs: 'matMenu',
8789
animations: [
8890
matMenuAnimations.transformMenu,
8991
matMenuAnimations.fadeInItems
9092
],
91-
exportAs: 'matMenu'
93+
providers: [
94+
{provide: MAT_MENU_PANEL, useExisting: MatMenu}
95+
]
9296
})
93-
export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestroy {
97+
export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel<MatMenuItem>, OnDestroy {
9498
private _keyManager: FocusKeyManager<MatMenuItem>;
9599
private _xPosition: MenuPositionX = this._defaultOptions.xPosition;
96100
private _yPosition: MenuPositionY = this._defaultOptions.yPosition;
97101
private _previousElevation: string;
98102

103+
/** Menu items inside the current menu. */
104+
private _items: MatMenuItem[] = [];
105+
106+
/** Emits whenever the amount of menu items changes. */
107+
private _itemChanges = new Subject<MatMenuItem[]>();
108+
99109
/** Subscription to tab events on the menu panel */
100110
private _tabSubscription = Subscription.EMPTY;
101111

@@ -106,7 +116,10 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
106116
_panelAnimationState: 'void' | 'enter' = 'void';
107117

108118
/** Emits whenever an animation on the menu completes. */
109-
_animationDone = new Subject<void>();
119+
_animationDone = new Subject<AnimationEvent>();
120+
121+
/** Whether the menu is animating. */
122+
_isAnimating: boolean;
110123

111124
/** Parent menu of the current menu panel. */
112125
parentMenu: MatMenuPanel | undefined;
@@ -142,7 +155,11 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
142155
/** @docs-private */
143156
@ViewChild(TemplateRef) templateRef: TemplateRef<any>;
144157

145-
/** List of the items inside of a menu. */
158+
/**
159+
* List of the items inside of a menu.
160+
* @deprecated
161+
* @deletion-target 7.0.0
162+
*/
146163
@ContentChildren(MatMenuItem) items: QueryList<MatMenuItem>;
147164

148165
/**
@@ -218,7 +235,7 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
218235
}
219236

220237
ngAfterContentInit() {
221-
this._keyManager = new FocusKeyManager<MatMenuItem>(this.items).withWrap().withTypeAhead();
238+
this._keyManager = new FocusKeyManager<MatMenuItem>(this._items).withWrap().withTypeAhead();
222239
this._tabSubscription = this._keyManager.tabOut.subscribe(() => this.close.emit('tab'));
223240
}
224241

@@ -229,16 +246,10 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
229246

230247
/** Stream that emits whenever the hovered menu item changes. */
231248
_hovered(): Observable<MatMenuItem> {
232-
if (this.items) {
233-
return this.items.changes.pipe(
234-
startWith(this.items),
235-
switchMap(items => merge(...items.map(item => item._hovered)))
236-
);
237-
}
238-
239-
return this._ngZone.onStable
240-
.asObservable()
241-
.pipe(take(1), switchMap(() => this._hovered()));
249+
return this._itemChanges.pipe(
250+
startWith(this._items),
251+
switchMap(items => merge(...items.map(item => item._hovered)))
252+
);
242253
}
243254

244255
/** Handle a keyboard event from the menu, delegating to the appropriate action. */
@@ -316,6 +327,35 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
316327
}
317328
}
318329

330+
/**
331+
* Registers a menu item with the menu.
332+
* @docs-private
333+
*/
334+
addItem(item: MatMenuItem) {
335+
// We register the items through this method, rather than picking them up through
336+
// `ContentChildren`, because we need the items to be picked up by their closest
337+
// `mat-menu` ancestor. If we used `@ContentChildren(MatMenuItem, {descendants: true})`,
338+
// all descendant items will bleed into the top-level menu in the case where the consumer
339+
// has `mat-menu` instances nested inside each other.
340+
if (this._items.indexOf(item) === -1) {
341+
this._items.push(item);
342+
this._itemChanges.next(this._items);
343+
}
344+
}
345+
346+
/**
347+
* Removes an item from the menu.
348+
* @docs-private
349+
*/
350+
removeItem(item: MatMenuItem) {
351+
const index = this._items.indexOf(item);
352+
353+
if (this._items.indexOf(item) > -1) {
354+
this._items.splice(index, 1);
355+
this._itemChanges.next(this._items);
356+
}
357+
}
358+
319359
/** Starts the enter animation. */
320360
_startAnimation() {
321361
// @deletion-target 7.0.0 Combine with _resetAnimation.
@@ -329,7 +369,8 @@ export class MatMenu implements OnInit, AfterContentInit, MatMenuPanel, OnDestro
329369
}
330370

331371
/** Callback that is invoked when the panel animation completes. */
332-
_onAnimationDone() {
333-
this._animationDone.next();
372+
_onAnimationDone(event: AnimationEvent) {
373+
this._animationDone.next(event);
374+
this._isAnimating = false;
334375
}
335376
}

src/lib/menu/menu-item.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
OnDestroy,
1515
ViewEncapsulation,
1616
Inject,
17+
Optional,
1718
} from '@angular/core';
1819
import {
1920
CanDisable,
@@ -23,6 +24,7 @@ import {
2324
} from '@angular/material/core';
2425
import {Subject} from 'rxjs';
2526
import {DOCUMENT} from '@angular/common';
27+
import {MAT_MENU_PANEL, MatMenuPanel} from './menu-panel';
2628

2729
// Boilerplate for applying mixins to MatMenuItem.
2830
/** @docs-private */
@@ -70,7 +72,8 @@ export class MatMenuItem extends _MatMenuItemMixinBase
7072
constructor(
7173
private _elementRef: ElementRef,
7274
@Inject(DOCUMENT) document?: any,
73-
private _focusMonitor?: FocusMonitor) {
75+
private _focusMonitor?: FocusMonitor,
76+
@Inject(MAT_MENU_PANEL) @Optional() private _parentMenu?: MatMenuPanel<MatMenuItem>) {
7477

7578
// @deletion-target 7.0.0 make `_focusMonitor` and `document` required params.
7679
super();
@@ -82,6 +85,10 @@ export class MatMenuItem extends _MatMenuItemMixinBase
8285
_focusMonitor.monitor(this._getHostElement(), false);
8386
}
8487

88+
if (_parentMenu && _parentMenu.addItem) {
89+
_parentMenu.addItem(this);
90+
}
91+
8592
this._document = document;
8693
}
8794

@@ -99,6 +106,10 @@ export class MatMenuItem extends _MatMenuItemMixinBase
99106
this._focusMonitor.stopMonitoring(this._getHostElement());
100107
}
101108

109+
if (this._parentMenu && this._parentMenu.removeItem) {
110+
this._parentMenu.removeItem(this);
111+
}
112+
102113
this._hovered.complete();
103114
}
104115

src/lib/menu/menu-panel.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,23 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {EventEmitter, TemplateRef} from '@angular/core';
9+
import {EventEmitter, TemplateRef, InjectionToken} from '@angular/core';
1010
import {MenuPositionX, MenuPositionY} from './menu-positions';
1111
import {Direction} from '@angular/cdk/bidi';
1212
import {FocusOrigin} from '@angular/cdk/a11y';
1313
import {MatMenuContent} from './menu-content';
1414

15+
/**
16+
* Injection token used to provide the parent menu to menu-specific components.
17+
* @docs-private
18+
*/
19+
export const MAT_MENU_PANEL = new InjectionToken<MatMenuPanel>('MAT_MENU_PANEL');
20+
1521
/**
1622
* Interface for a custom menu panel that can be used with `matMenuTriggerFor`.
1723
* @docs-private
1824
*/
19-
export interface MatMenuPanel {
25+
export interface MatMenuPanel<T = any> {
2026
xPosition: MenuPositionX;
2127
yPosition: MenuPositionY;
2228
overlapTrigger: boolean;
@@ -31,4 +37,6 @@ export interface MatMenuPanel {
3137
lazyContent?: MatMenuContent;
3238
backdropClass?: string;
3339
hasBackdrop?: boolean;
40+
addItem?: (item: T) => void;
41+
removeItem?: (item: T) => void;
3442
}

0 commit comments

Comments
 (0)