Skip to content

feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connect menus #24558

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 15, 2022
Merged

feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connect menus #24558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19af380
feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to connec…
mmalerba Mar 9, 2022
f780d1a
fixup! feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to…
mmalerba Mar 14, 2022
c01a543
fixup! feat(cdk-experimental/menu): Use DI instead of CdkMenuPanel to…
mmalerba Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 40 additions & 28 deletions src/cdk-experimental/menu/context-menu.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,20 +389,28 @@ describe('CdkContextMenuTrigger', () => {
* @param componentClass the component to create
*/
function createComponent<T>(componentClass: Type<T>) {
return function () {
TestBed.configureTestingModule({
imports: [CdkMenuModule],
declarations: [componentClass],
}).compileComponents();
TestBed.configureTestingModule({
imports: [CdkMenuModule],
declarations: [componentClass],
}).compileComponents();

TestBed.createComponent(componentClass).detectChanges();
};
const fixture = TestBed.createComponent(componentClass);
fixture.detectChanges();
return fixture;
}

it('should throw an error if context and menubar trigger share a menu', () => {
expect(createComponent(MenuBarAndContextTriggerShareMenu)).toThrowError(
/CdkMenuPanel is already referenced by different CdkMenuTrigger/,
);
it('should allow a context menu and menubar trigger share a menu', () => {
const fixture = createComponent(MenuBarAndContextTriggerShareMenu);
expect(fixture.componentInstance.menus.length).toBe(0);
fixture.componentInstance.menuBarTrigger.toggle();
fixture.detectChanges();
expect(fixture.componentInstance.menus.length).toBe(1);
fixture.componentInstance.menuBarTrigger.toggle();
fixture.detectChanges();
expect(fixture.componentInstance.menus.length).toBe(0);
fixture.componentInstance.contextTrigger.open({x: 0, y: 0});
fixture.detectChanges();
expect(fixture.componentInstance.menus.length).toBe(1);
});
});
});
Expand All @@ -412,8 +420,8 @@ describe('CdkContextMenuTrigger', () => {
<div [cdkContextMenuTriggerFor]="context"></div>
<div id="other"></div>

<ng-template cdkMenuPanel #context="cdkMenuPanel">
<div cdkMenu [cdkMenuPanel]="context">
<ng-template #context>
<div cdkMenu>
<button id="first_menu_item" cdkMenuItem></button>
</div>
</ng-template>
Expand All @@ -438,12 +446,12 @@ class SimpleContextMenu {
></div>
</div>

<ng-template cdkMenuPanel #cut="cdkMenuPanel">
<div #cut_menu cdkMenu [cdkMenuPanel]="cut"></div>
<ng-template #cut>
<div #cut_menu cdkMenu></div>
</ng-template>

<ng-template cdkMenuPanel #copy="cdkMenuPanel">
<div #copy_menu cdkMenu [cdkMenuPanel]="copy"></div>
<ng-template #copy>
<div #copy_menu cdkMenu></div>
</ng-template>
`,
})
Expand All @@ -461,14 +469,14 @@ class NestedContextMenu {
template: `
<div [cdkContextMenuTriggerFor]="cut"></div>

<ng-template cdkMenuPanel #cut="cdkMenuPanel">
<div #cut_menu cdkMenu [cdkMenuPanel]="cut">
<ng-template #cut>
<div #cut_menu cdkMenu>
<button cdkMenuItem [cdkMenuTriggerFor]="copy"></button>
</div>
</ng-template>

<ng-template cdkMenuPanel #copy="cdkMenuPanel">
<div #copy_menu cdkMenu [cdkMenuPanel]="copy"></div>
<ng-template #copy>
<div #copy_menu cdkMenu></div>
</ng-template>
`,
})
Expand All @@ -486,13 +494,13 @@ class ContextMenuWithSubmenu {
<button #trigger cdkMenuItem [cdkMenuTriggerFor]="file">File</button>
</div>

<ng-template cdkMenuPanel #file="cdkMenuPanel">
<div cdkMenu #file_menu id="file_menu" [cdkMenuPanel]="file"></div>
<ng-template #file>
<div cdkMenu #file_menu id="file_menu"></div>
</ng-template>

<div [cdkContextMenuTriggerFor]="context"></div>
<ng-template cdkMenuPanel #context="cdkMenuPanel">
<div cdkMenu #context_menu [cdkMenuPanel]="context">
<ng-template #context>
<div cdkMenu #context_menu>
<button cdkMenuItem></button>
</div>
</ng-template>
Expand Down Expand Up @@ -523,11 +531,15 @@ class ContextMenuWithMenuBarAndInlineMenu {

<div [cdkContextMenuTriggerFor]="menu"></div>

<ng-template cdkMenuPanel #menu="cdkMenuPanel">
<div cdkMenu [cdkMenuPanel]="menu">
<ng-template #menu>
<div cdkMenu>
<button cdkMenuItem></button>
</div>
</ng-template>
`,
})
class MenuBarAndContextTriggerShareMenu {}
class MenuBarAndContextTriggerShareMenu {
@ViewChild(CdkMenuItemTrigger) menuBarTrigger: CdkMenuItemTrigger;
@ViewChild(CdkContextMenuTrigger) contextTrigger: CdkContextMenuTrigger;
@ViewChildren(CdkMenu) menus: QueryList<CdkMenu>;
}
78 changes: 29 additions & 49 deletions src/cdk-experimental/menu/context-menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import {
Inject,
Injectable,
InjectionToken,
Injector,
Input,
OnDestroy,
Optional,
Output,
TemplateRef,
ViewContainerRef,
} from '@angular/core';
import {Directionality} from '@angular/cdk/bidi';
Expand All @@ -30,10 +32,9 @@ import {Portal, TemplatePortal} from '@angular/cdk/portal';
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
import {merge, partition, Subject} from 'rxjs';
import {skip, takeUntil} from 'rxjs/operators';
import {CdkMenuPanel} from './menu-panel';
import {MenuStack} from './menu-stack';
import {throwExistingMenuStackError} from './menu-errors';
import {MENU_STACK, MenuStack} from './menu-stack';
import {isClickInsideMenuOverlay} from './menu-item-trigger';
import {MENU_TRIGGER, MenuTrigger} from './menu-trigger';

/** Tracks the last open context menu trigger across the entire application. */
@Injectable({providedIn: 'root'})
Expand Down Expand Up @@ -85,26 +86,14 @@ export type ContextMenuCoordinates = {x: number; y: number};
// In cases where the first menu item in the context menu is a trigger the submenu opens on a
// hover event. Offsetting the opened context menu by 2px prevents this from occurring.
{provide: CDK_CONTEXT_MENU_DEFAULT_OPTIONS, useValue: {offsetX: 2, offsetY: 2}},
{provide: MENU_TRIGGER, useExisting: CdkContextMenuTrigger},
{provide: MENU_STACK, useClass: MenuStack},
],
})
export class CdkContextMenuTrigger implements OnDestroy {
export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
/** Template reference variable to the menu to open on right click. */
@Input('cdkContextMenuTriggerFor')
get menuPanel(): CdkMenuPanel {
return this._menuPanel;
}
set menuPanel(panel: CdkMenuPanel) {
if ((typeof ngDevMode === 'undefined' || ngDevMode) && panel._menuStack) {
throwExistingMenuStackError();
}
this._menuPanel = panel;

if (this._menuPanel) {
this._menuPanel._menuStack = this._menuStack;
}
}
/** Reference to the MenuPanel this trigger toggles. */
private _menuPanel: CdkMenuPanel;
private _menuTemplateRef: TemplateRef<unknown>;

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

/** The content of the menu panel opened by this trigger. */
private _panelContent: TemplatePortal;
private _menuPortal: TemplatePortal;

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

/** The menu stack for this trigger and its associated menus. */
private readonly _menuStack = new MenuStack();

/** Emits when the outside pointer events listener on the overlay should be stopped. */
private readonly _stopOutsideClicksListener = merge(this.closed, this._destroyed);

constructor(
injector: Injector,
protected readonly _viewContainerRef: ViewContainerRef,
private readonly _overlay: Overlay,
private readonly _contextMenuTracker: ContextMenuTracker,
@Inject(MENU_STACK) menuStack: MenuStack,
@Inject(CDK_CONTEXT_MENU_DEFAULT_OPTIONS) private readonly _options: ContextMenuOptions,
@Optional() private readonly _directionality?: Directionality,
) {
super(injector, menuStack);
this._setMenuStackListener();
}

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

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

/** Close the opened menu. */
close() {
this._menuStack.closeAll();
this.menuStack.closeAll();
}

/**
Expand All @@ -208,11 +197,11 @@ export class CdkContextMenuTrigger implements OnDestroy {

// A context menu can be triggered via a mouse right click or a keyboard shortcut.
if (event.button === 2) {
this._menuPanel._menu?.focusFirstItem('mouse');
this.childMenu?.focusFirstItem('mouse');
} else if (event.button === 0) {
this._menuPanel._menu?.focusFirstItem('keyboard');
this.childMenu?.focusFirstItem('keyboard');
} else {
this._menuPanel._menu?.focusFirstItem('program');
this.childMenu?.focusFirstItem('program');
}
}
}
Expand Down Expand Up @@ -267,18 +256,23 @@ export class CdkContextMenuTrigger implements OnDestroy {
* content to change dynamically and be reflected in the application.
*/
private _getMenuContent(): Portal<unknown> {
const hasMenuContentChanged = this.menuPanel._templateRef !== this._panelContent?.templateRef;
if (this.menuPanel && (!this._panelContent || hasMenuContentChanged)) {
this._panelContent = new TemplatePortal(this.menuPanel._templateRef, this._viewContainerRef);
const hasMenuContentChanged = this._menuTemplateRef !== this._menuPortal?.templateRef;
if (this._menuTemplateRef && (!this._menuPortal || hasMenuContentChanged)) {
this._menuPortal = new TemplatePortal(
this._menuTemplateRef,
this._viewContainerRef,
undefined,
this.getChildMenuInjector(),
);
}

return this._panelContent;
return this._menuPortal;
}

/** Subscribe to the menu stack close events and close this menu when requested. */
private _setMenuStackListener() {
this._menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(item => {
if (item === this._menuPanel._menu && this.isOpen()) {
this.menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(item => {
if (item === this.childMenu && this.isOpen()) {
this.closed.next();
this._overlayRef!.detach();
}
Expand All @@ -300,15 +294,14 @@ export class CdkContextMenuTrigger implements OnDestroy {
}
outsideClicks.pipe(takeUntil(this._stopOutsideClicksListener)).subscribe(event => {
if (!isClickInsideMenuOverlay(event.target as Element)) {
this._menuStack.closeAll();
this.menuStack.closeAll();
}
});
}
}

ngOnDestroy() {
this._destroyOverlay();
this._resetPanelMenuStack();

this._destroyed.next();
this._destroyed.complete();
Expand All @@ -321,17 +314,4 @@ export class CdkContextMenuTrigger implements OnDestroy {
this._overlayRef = null;
}
}

/** Set the menu panels menu stack back to null. */
private _resetPanelMenuStack() {
// If a ContextMenuTrigger is placed in a conditionally rendered view, each time the trigger is
// rendered the panel setter for ContextMenuTrigger is called. From the first render onward,
// the attached CdkMenuPanel has the MenuStack set. Since we throw an error if a panel already
// has a stack set, we want to reset the attached stack here to prevent the error from being
// thrown if the trigger re-configures its attached panel (in the case where there is a 1:1
// relationship between the panel and trigger).
if (this._menuPanel) {
this._menuPanel._menuStack = null;
}
}
}
24 changes: 12 additions & 12 deletions src/cdk-experimental/menu/menu-bar.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,23 +1191,23 @@ class MenuBarRadioGroup {}
<button cdkMenuItem [cdkMenuTriggerFor]="edit">Edit</button>
</div>

<ng-template cdkMenuPanel #file="cdkMenuPanel">
<div cdkMenu id="file_menu" [cdkMenuPanel]="file">
<ng-template #file>
<div cdkMenu id="file_menu">
<button cdkMenuItem>Save</button>
<button cdkMenuItem [cdkMenuTriggerFor]="share">Share</button>
<button cdkMenuItem>Open</button>
</div>
</ng-template>

<ng-template cdkMenuPanel #share="cdkMenuPanel">
<div cdkMenu id="share_menu" [cdkMenuPanel]="share">
<ng-template #share>
<div cdkMenu id="share_menu">
<button (cdkMenuItemTriggered)="clickEmitter.next()" cdkMenuItem>Email</button>
<button cdkMenuItem>Chat</button>
</div>
</ng-template>

<ng-template cdkMenuPanel #edit="cdkMenuPanel">
<div cdkMenu id="edit_menu" [cdkMenuPanel]="edit">
<ng-template #edit>
<div cdkMenu id="edit_menu">
<button cdkMenuItem>Undo</button>
<button cdkMenuItem>Redo</button>
</div>
Expand All @@ -1231,8 +1231,8 @@ class MultiMenuWithSubmenu {
<button cdkMenuItem [cdkMenuTriggerFor]="font">Font size</button>
</div>

<ng-template cdkMenuPanel #font="cdkMenuPanel">
<div cdkMenu id="font_menu" [cdkMenuPanel]="font">
<ng-template #font>
<div cdkMenu id="font_menu">
<button cdkMenuItemCheckbox>Small</button>
<button cdkMenuItemCheckbox>Large</button>
</div>
Expand All @@ -1257,8 +1257,8 @@ class MenuWithCheckboxes {
<button cdkMenuItem [cdkMenuTriggerFor]="text">Text</button>
</div>

<ng-template cdkMenuPanel #text="cdkMenuPanel">
<div cdkMenu id="text_menu" [cdkMenuPanel]="text">
<ng-template #text>
<div cdkMenu id="text_menu">
<button cdkMenuItemRadio>Bold</button>
<button cdkMenuItemRadio>Italic</button>
</div>
Expand All @@ -1283,8 +1283,8 @@ class MenuWithRadioButtons {
<button cdkMenuItem [cdkMenuTriggerFor]="sub1">Trigger</button>
</div>

<ng-template cdkMenuPanel #sub1="cdkMenuPanel">
<div cdkMenu [cdkMenuPanel]="sub1">
<ng-template #sub1>
<div cdkMenu>
<div cdkMenuGroup>
<button cdkMenuItemCheckbox>Trigger</button>
<span id="inner-element">A nested non-menuitem element</span>
Expand Down
6 changes: 3 additions & 3 deletions src/cdk-experimental/menu/menu-bar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {Subject, merge} from 'rxjs';
import {CdkMenuGroup} from './menu-group';
import {CDK_MENU, Menu} from './menu-interface';
import {CdkMenuItem} from './menu-item';
import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
import {MenuStack, MenuStackItem, FocusNext, MENU_STACK} from './menu-stack';
import {PointerFocusTracker} from './pointer-focus-tracker';
import {MenuAim, MENU_AIM} from './menu-aim';

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

constructor(
readonly _menuStack: MenuStack,
private readonly _ngZone: NgZone,
readonly _elementRef: ElementRef<HTMLElement>,
@Inject(MENU_STACK) readonly _menuStack: MenuStack,
@Self() @Optional() @Inject(MENU_AIM) private readonly _menuAim?: MenuAim,
@Optional() private readonly _dir?: Directionality,
) {
Expand Down
Loading