Skip to content

Commit dc020c7

Browse files
authored
feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connect menus (#24558)
* feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connect menus * fixup! feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connect menus * fixup! feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connect menus
1 parent cbef92a commit dc020c7

26 files changed

+394
-471
lines changed

src/cdk-experimental/menu/context-menu.spec.ts

Lines changed: 40 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -389,20 +389,28 @@ describe('CdkContextMenuTrigger', () => {
389389
* @param componentClass the component to create
390390
*/
391391
function createComponent<T>(componentClass: Type<T>) {
392-
return function () {
393-
TestBed.configureTestingModule({
394-
imports: [CdkMenuModule],
395-
declarations: [componentClass],
396-
}).compileComponents();
392+
TestBed.configureTestingModule({
393+
imports: [CdkMenuModule],
394+
declarations: [componentClass],
395+
}).compileComponents();
397396

398-
TestBed.createComponent(componentClass).detectChanges();
399-
};
397+
const fixture = TestBed.createComponent(componentClass);
398+
fixture.detectChanges();
399+
return fixture;
400400
}
401401

402-
it('should throw an error if context and menubar trigger share a menu', () => {
403-
expect(createComponent(MenuBarAndContextTriggerShareMenu)).toThrowError(
404-
/CdkMenuPanel is already referenced by different CdkMenuTrigger/,
405-
);
402+
it('should allow a context menu and menubar trigger share a menu', () => {
403+
const fixture = createComponent(MenuBarAndContextTriggerShareMenu);
404+
expect(fixture.componentInstance.menus.length).toBe(0);
405+
fixture.componentInstance.menuBarTrigger.toggle();
406+
fixture.detectChanges();
407+
expect(fixture.componentInstance.menus.length).toBe(1);
408+
fixture.componentInstance.menuBarTrigger.toggle();
409+
fixture.detectChanges();
410+
expect(fixture.componentInstance.menus.length).toBe(0);
411+
fixture.componentInstance.contextTrigger.open({x: 0, y: 0});
412+
fixture.detectChanges();
413+
expect(fixture.componentInstance.menus.length).toBe(1);
406414
});
407415
});
408416
});
@@ -412,8 +420,8 @@ describe('CdkContextMenuTrigger', () => {
412420
<div [cdkContextMenuTriggerFor]="context"></div>
413421
<div id="other"></div>
414422
415-
<ng-template cdkMenuPanel #context="cdkMenuPanel">
416-
<div cdkMenu [cdkMenuPanel]="context">
423+
<ng-template #context>
424+
<div cdkMenu>
417425
<button id="first_menu_item" cdkMenuItem></button>
418426
</div>
419427
</ng-template>
@@ -438,12 +446,12 @@ class SimpleContextMenu {
438446
></div>
439447
</div>
440448
441-
<ng-template cdkMenuPanel #cut="cdkMenuPanel">
442-
<div #cut_menu cdkMenu [cdkMenuPanel]="cut"></div>
449+
<ng-template #cut>
450+
<div #cut_menu cdkMenu></div>
443451
</ng-template>
444452
445-
<ng-template cdkMenuPanel #copy="cdkMenuPanel">
446-
<div #copy_menu cdkMenu [cdkMenuPanel]="copy"></div>
453+
<ng-template #copy>
454+
<div #copy_menu cdkMenu></div>
447455
</ng-template>
448456
`,
449457
})
@@ -461,14 +469,14 @@ class NestedContextMenu {
461469
template: `
462470
<div [cdkContextMenuTriggerFor]="cut"></div>
463471
464-
<ng-template cdkMenuPanel #cut="cdkMenuPanel">
465-
<div #cut_menu cdkMenu [cdkMenuPanel]="cut">
472+
<ng-template #cut>
473+
<div #cut_menu cdkMenu>
466474
<button cdkMenuItem [cdkMenuTriggerFor]="copy"></button>
467475
</div>
468476
</ng-template>
469477
470-
<ng-template cdkMenuPanel #copy="cdkMenuPanel">
471-
<div #copy_menu cdkMenu [cdkMenuPanel]="copy"></div>
478+
<ng-template #copy>
479+
<div #copy_menu cdkMenu></div>
472480
</ng-template>
473481
`,
474482
})
@@ -486,13 +494,13 @@ class ContextMenuWithSubmenu {
486494
<button #trigger cdkMenuItem [cdkMenuTriggerFor]="file">File</button>
487495
</div>
488496
489-
<ng-template cdkMenuPanel #file="cdkMenuPanel">
490-
<div cdkMenu #file_menu id="file_menu" [cdkMenuPanel]="file"></div>
497+
<ng-template #file>
498+
<div cdkMenu #file_menu id="file_menu"></div>
491499
</ng-template>
492500
493501
<div [cdkContextMenuTriggerFor]="context"></div>
494-
<ng-template cdkMenuPanel #context="cdkMenuPanel">
495-
<div cdkMenu #context_menu [cdkMenuPanel]="context">
502+
<ng-template #context>
503+
<div cdkMenu #context_menu>
496504
<button cdkMenuItem></button>
497505
</div>
498506
</ng-template>
@@ -523,11 +531,15 @@ class ContextMenuWithMenuBarAndInlineMenu {
523531
524532
<div [cdkContextMenuTriggerFor]="menu"></div>
525533
526-
<ng-template cdkMenuPanel #menu="cdkMenuPanel">
527-
<div cdkMenu [cdkMenuPanel]="menu">
534+
<ng-template #menu>
535+
<div cdkMenu>
528536
<button cdkMenuItem></button>
529537
</div>
530538
</ng-template>
531539
`,
532540
})
533-
class MenuBarAndContextTriggerShareMenu {}
541+
class MenuBarAndContextTriggerShareMenu {
542+
@ViewChild(CdkMenuItemTrigger) menuBarTrigger: CdkMenuItemTrigger;
543+
@ViewChild(CdkContextMenuTrigger) contextTrigger: CdkContextMenuTrigger;
544+
@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
545+
}

src/cdk-experimental/menu/context-menu.ts

Lines changed: 29 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@ import {
1212
Inject,
1313
Injectable,
1414
InjectionToken,
15+
Injector,
1516
Input,
1617
OnDestroy,
1718
Optional,
1819
Output,
20+
TemplateRef,
1921
ViewContainerRef,
2022
} from '@angular/core';
2123
import {Directionality} from '@angular/cdk/bidi';
@@ -30,10 +32,9 @@ import {Portal, TemplatePortal} from '@angular/cdk/portal';
3032
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
3133
import {merge, partition, Subject} from 'rxjs';
3234
import {skip, takeUntil} from 'rxjs/operators';
33-
import {CdkMenuPanel} from './menu-panel';
34-
import {MenuStack} from './menu-stack';
35-
import {throwExistingMenuStackError} from './menu-errors';
35+
import {MENU_STACK, MenuStack} from './menu-stack';
3636
import {isClickInsideMenuOverlay} from './menu-item-trigger';
37+
import {MENU_TRIGGER, MenuTrigger} from './menu-trigger';
3738

3839
/** Tracks the last open context menu trigger across the entire application. */
3940
@Injectable({providedIn: 'root'})
@@ -85,26 +86,14 @@ export type ContextMenuCoordinates = {x: number; y: number};
8586
// In cases where the first menu item in the context menu is a trigger the submenu opens on a
8687
// hover event. Offsetting the opened context menu by 2px prevents this from occurring.
8788
{provide: CDK_CONTEXT_MENU_DEFAULT_OPTIONS, useValue: {offsetX: 2, offsetY: 2}},
89+
{provide: MENU_TRIGGER, useExisting: CdkContextMenuTrigger},
90+
{provide: MENU_STACK, useClass: MenuStack},
8891
],
8992
})
90-
export class CdkContextMenuTrigger implements OnDestroy {
93+
export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
9194
/** Template reference variable to the menu to open on right click. */
9295
@Input('cdkContextMenuTriggerFor')
93-
get menuPanel(): CdkMenuPanel {
94-
return this._menuPanel;
95-
}
96-
set menuPanel(panel: CdkMenuPanel) {
97-
if ((typeof ngDevMode === 'undefined' || ngDevMode) && panel._menuStack) {
98-
throwExistingMenuStackError();
99-
}
100-
this._menuPanel = panel;
101-
102-
if (this._menuPanel) {
103-
this._menuPanel._menuStack = this._menuStack;
104-
}
105-
}
106-
/** Reference to the MenuPanel this trigger toggles. */
107-
private _menuPanel: CdkMenuPanel;
96+
private _menuTemplateRef: TemplateRef<unknown>;
10897

10998
/** Emits when the attached menu is requested to open. */
11099
@Output('cdkContextMenuOpened') readonly opened: EventEmitter<void> = new EventEmitter();
@@ -126,24 +115,24 @@ export class CdkContextMenuTrigger implements OnDestroy {
126115
private _overlayRef: OverlayRef | null = null;
127116

128117
/** The content of the menu panel opened by this trigger. */
129-
private _panelContent: TemplatePortal;
118+
private _menuPortal: TemplatePortal;
130119

131120
/** Emits when the element is destroyed. */
132121
private readonly _destroyed = new Subject<void>();
133122

134-
/** The menu stack for this trigger and its associated menus. */
135-
private readonly _menuStack = new MenuStack();
136-
137123
/** Emits when the outside pointer events listener on the overlay should be stopped. */
138124
private readonly _stopOutsideClicksListener = merge(this.closed, this._destroyed);
139125

140126
constructor(
127+
injector: Injector,
141128
protected readonly _viewContainerRef: ViewContainerRef,
142129
private readonly _overlay: Overlay,
143130
private readonly _contextMenuTracker: ContextMenuTracker,
131+
@Inject(MENU_STACK) menuStack: MenuStack,
144132
@Inject(CDK_CONTEXT_MENU_DEFAULT_OPTIONS) private readonly _options: ContextMenuOptions,
145133
@Optional() private readonly _directionality?: Directionality,
146134
) {
135+
super(injector, menuStack);
147136
this._setMenuStackListener();
148137
}
149138

@@ -161,7 +150,7 @@ export class CdkContextMenuTrigger implements OnDestroy {
161150
} else if (this.isOpen()) {
162151
// since we're moving this menu we need to close any submenus first otherwise they end up
163152
// disconnected from this one.
164-
this._menuStack.closeSubMenuOf(this._menuPanel._menu!);
153+
this.menuStack.closeSubMenuOf(this.childMenu!);
165154

166155
(
167156
this._overlayRef!.getConfig().positionStrategy as FlexibleConnectedPositionStrategy
@@ -186,7 +175,7 @@ export class CdkContextMenuTrigger implements OnDestroy {
186175

187176
/** Close the opened menu. */
188177
close() {
189-
this._menuStack.closeAll();
178+
this.menuStack.closeAll();
190179
}
191180

192181
/**
@@ -208,11 +197,11 @@ export class CdkContextMenuTrigger implements OnDestroy {
208197

209198
// A context menu can be triggered via a mouse right click or a keyboard shortcut.
210199
if (event.button === 2) {
211-
this._menuPanel._menu?.focusFirstItem('mouse');
200+
this.childMenu?.focusFirstItem('mouse');
212201
} else if (event.button === 0) {
213-
this._menuPanel._menu?.focusFirstItem('keyboard');
202+
this.childMenu?.focusFirstItem('keyboard');
214203
} else {
215-
this._menuPanel._menu?.focusFirstItem('program');
204+
this.childMenu?.focusFirstItem('program');
216205
}
217206
}
218207
}
@@ -267,18 +256,23 @@ export class CdkContextMenuTrigger implements OnDestroy {
267256
* content to change dynamically and be reflected in the application.
268257
*/
269258
private _getMenuContent(): Portal<unknown> {
270-
const hasMenuContentChanged = this.menuPanel._templateRef !== this._panelContent?.templateRef;
271-
if (this.menuPanel && (!this._panelContent || hasMenuContentChanged)) {
272-
this._panelContent = new TemplatePortal(this.menuPanel._templateRef, this._viewContainerRef);
259+
const hasMenuContentChanged = this._menuTemplateRef !== this._menuPortal?.templateRef;
260+
if (this._menuTemplateRef && (!this._menuPortal || hasMenuContentChanged)) {
261+
this._menuPortal = new TemplatePortal(
262+
this._menuTemplateRef,
263+
this._viewContainerRef,
264+
undefined,
265+
this.getChildMenuInjector(),
266+
);
273267
}
274268

275-
return this._panelContent;
269+
return this._menuPortal;
276270
}
277271

278272
/** Subscribe to the menu stack close events and close this menu when requested. */
279273
private _setMenuStackListener() {
280-
this._menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(item => {
281-
if (item === this._menuPanel._menu && this.isOpen()) {
274+
this.menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(item => {
275+
if (item === this.childMenu && this.isOpen()) {
282276
this.closed.next();
283277
this._overlayRef!.detach();
284278
}
@@ -300,15 +294,14 @@ export class CdkContextMenuTrigger implements OnDestroy {
300294
}
301295
outsideClicks.pipe(takeUntil(this._stopOutsideClicksListener)).subscribe(event => {
302296
if (!isClickInsideMenuOverlay(event.target as Element)) {
303-
this._menuStack.closeAll();
297+
this.menuStack.closeAll();
304298
}
305299
});
306300
}
307301
}
308302

309303
ngOnDestroy() {
310304
this._destroyOverlay();
311-
this._resetPanelMenuStack();
312305

313306
this._destroyed.next();
314307
this._destroyed.complete();
@@ -321,17 +314,4 @@ export class CdkContextMenuTrigger implements OnDestroy {
321314
this._overlayRef = null;
322315
}
323316
}
324-
325-
/** Set the menu panels menu stack back to null. */
326-
private _resetPanelMenuStack() {
327-
// If a ContextMenuTrigger is placed in a conditionally rendered view, each time the trigger is
328-
// rendered the panel setter for ContextMenuTrigger is called. From the first render onward,
329-
// the attached CdkMenuPanel has the MenuStack set. Since we throw an error if a panel already
330-
// has a stack set, we want to reset the attached stack here to prevent the error from being
331-
// thrown if the trigger re-configures its attached panel (in the case where there is a 1:1
332-
// relationship between the panel and trigger).
333-
if (this._menuPanel) {
334-
this._menuPanel._menuStack = null;
335-
}
336-
}
337317
}

src/cdk-experimental/menu/menu-bar.spec.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1191,23 +1191,23 @@ class MenuBarRadioGroup {}
11911191
<button cdkMenuItem [cdkMenuTriggerFor]="edit">Edit</button>
11921192
</div>
11931193
1194-
<ng-template cdkMenuPanel #file="cdkMenuPanel">
1195-
<div cdkMenu id="file_menu" [cdkMenuPanel]="file">
1194+
<ng-template #file>
1195+
<div cdkMenu id="file_menu">
11961196
<button cdkMenuItem>Save</button>
11971197
<button cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
11981198
<button cdkMenuItem>Open</button>
11991199
</div>
12001200
</ng-template>
12011201
1202-
<ng-template cdkMenuPanel #share="cdkMenuPanel">
1203-
<div cdkMenu id="share_menu" [cdkMenuPanel]="share">
1202+
<ng-template #share>
1203+
<div cdkMenu id="share_menu">
12041204
<button (cdkMenuItemTriggered)="clickEmitter.next()" cdkMenuItem>Email</button>
12051205
<button cdkMenuItem>Chat</button>
12061206
</div>
12071207
</ng-template>
12081208
1209-
<ng-template cdkMenuPanel #edit="cdkMenuPanel">
1210-
<div cdkMenu id="edit_menu" [cdkMenuPanel]="edit">
1209+
<ng-template #edit>
1210+
<div cdkMenu id="edit_menu">
12111211
<button cdkMenuItem>Undo</button>
12121212
<button cdkMenuItem>Redo</button>
12131213
</div>
@@ -1231,8 +1231,8 @@ class MultiMenuWithSubmenu {
12311231
<button cdkMenuItem [cdkMenuTriggerFor]="font">Font size</button>
12321232
</div>
12331233
1234-
<ng-template cdkMenuPanel #font="cdkMenuPanel">
1235-
<div cdkMenu id="font_menu" [cdkMenuPanel]="font">
1234+
<ng-template #font>
1235+
<div cdkMenu id="font_menu">
12361236
<button cdkMenuItemCheckbox>Small</button>
12371237
<button cdkMenuItemCheckbox>Large</button>
12381238
</div>
@@ -1257,8 +1257,8 @@ class MenuWithCheckboxes {
12571257
<button cdkMenuItem [cdkMenuTriggerFor]="text">Text</button>
12581258
</div>
12591259
1260-
<ng-template cdkMenuPanel #text="cdkMenuPanel">
1261-
<div cdkMenu id="text_menu" [cdkMenuPanel]="text">
1260+
<ng-template #text>
1261+
<div cdkMenu id="text_menu">
12621262
<button cdkMenuItemRadio>Bold</button>
12631263
<button cdkMenuItemRadio>Italic</button>
12641264
</div>
@@ -1283,8 +1283,8 @@ class MenuWithRadioButtons {
12831283
<button cdkMenuItem [cdkMenuTriggerFor]="sub1">Trigger</button>
12841284
</div>
12851285
1286-
<ng-template cdkMenuPanel #sub1="cdkMenuPanel">
1287-
<div cdkMenu [cdkMenuPanel]="sub1">
1286+
<ng-template #sub1>
1287+
<div cdkMenu>
12881288
<div cdkMenuGroup>
12891289
<button cdkMenuItemCheckbox>Trigger</button>
12901290
<span id="inner-element">A nested non-menuitem element</span>

src/cdk-experimental/menu/menu-bar.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {Subject, merge} from 'rxjs';
2727
import {CdkMenuGroup} from './menu-group';
2828
import {CDK_MENU, Menu} from './menu-interface';
2929
import {CdkMenuItem} from './menu-item';
30-
import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
30+
import {MenuStack, MenuStackItem, FocusNext, MENU_STACK} from './menu-stack';
3131
import {PointerFocusTracker} from './pointer-focus-tracker';
3232
import {MenuAim, MENU_AIM} from './menu-aim';
3333

@@ -51,7 +51,7 @@ import {MenuAim, MENU_AIM} from './menu-aim';
5151
providers: [
5252
{provide: CdkMenuGroup, useExisting: CdkMenuBar},
5353
{provide: CDK_MENU, useExisting: CdkMenuBar},
54-
{provide: MenuStack, useClass: MenuStack},
54+
{provide: MENU_STACK, useClass: MenuStack},
5555
],
5656
})
5757
export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit, OnDestroy {
@@ -78,9 +78,9 @@ export class CdkMenuBar extends CdkMenuGroup implements Menu, AfterContentInit,
7878
private _openItem?: CdkMenuItem;
7979

8080
constructor(
81-
readonly _menuStack: MenuStack,
8281
private readonly _ngZone: NgZone,
8382
readonly _elementRef: ElementRef<HTMLElement>,
83+
@Inject(MENU_STACK) readonly _menuStack: MenuStack,
8484
@Self() @Optional() @Inject(MENU_AIM) private readonly _menuAim?: MenuAim,
8585
@Optional() private readonly _dir?: Directionality,
8686
) {

0 commit comments

Comments
 (0)