Skip to content

feat(cdk-experimental/menu): add support for inline menus #20143

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.

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 1 commit into from
Aug 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 0 additions & 19 deletions src/cdk-experimental/menu/menu-errors.ts

This file was deleted.

8 changes: 7 additions & 1 deletion src/cdk-experimental/menu/menu-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,13 @@ export class MenuStack {
}

/** Get the top most element on the stack. */
peek() {
peek(): MenuStackItem | undefined {
return this._elements[this._elements.length - 1];
}
}

/** NoopMenuStack is a placeholder MenuStack used for inline menus. */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify -- does 'inline' mean it's triggered by some means other than a bar? (Such as a button?) I'd make this clearer in the documentation somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline menus aren't triggered open at all - they're always open. Think of the list of mailboxes on gmail (inbox, starred, snoozed, ...).

export class NoopMenuStack extends MenuStack {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeScript lets you do implements MenuStack so you can conform to the same API without actually inheriting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to work only if all members are public. With private and protected it won't allow it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/** Noop push - does not add elements to the MenuStack. */
push(_: MenuStackItem) {}
}
47 changes: 47 additions & 0 deletions src/cdk-experimental/menu/menu.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
import {Component, ViewChild} from '@angular/core';
import {TAB} from '@angular/cdk/keycodes';
import {dispatchKeyboardEvent} from '@angular/cdk/testing/private';
import {By} from '@angular/platform-browser';
import {CdkMenu} from './menu';
import {CdkMenuModule} from './menu-module';
Expand Down Expand Up @@ -158,6 +160,41 @@ describe('Menu', () => {
expect(spy).withContext('Expected initial trigger only').toHaveBeenCalledTimes(1);
});
});

describe('when configured inline', () => {
let fixture: ComponentFixture<InlineMenu>;
let nativeMenu: HTMLElement;
let nativeMenuItems: HTMLElement[];

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [CdkMenuModule],
declarations: [InlineMenu],
}).compileComponents();
}));

beforeEach(() => {
fixture = TestBed.createComponent(InlineMenu);
fixture.detectChanges();

nativeMenu = fixture.debugElement.query(By.directive(CdkMenu)).nativeElement;
nativeMenuItems = fixture.debugElement
.queryAll(By.directive(CdkMenuItem))
.map(e => e.nativeElement);
});

it('should have its tabindex set to 0', () => {
const item = fixture.debugElement.query(By.directive(CdkMenu)).nativeElement;
expect(item.getAttribute('tabindex')).toBe('0');
});

it('should focus the first menu item when it gets tabbed in', () => {
dispatchKeyboardEvent(document, 'keydown', TAB);
nativeMenu.focus();

expect(document.querySelector(':focus')).toEqual(nativeMenuItems[0]);
});
});
});

@Component({
Expand Down Expand Up @@ -229,3 +266,13 @@ class MenuWithConditionalGroup {
@ViewChild(CdkMenuItem) readonly trigger: CdkMenuItem;
@ViewChild(CdkMenuPanel) readonly panel: CdkMenuPanel;
}

@Component({
template: `
<div cdkMenu>
<button cdkMenuItem>Inbox</button>
<button cdkMenuItem>Starred</button>
</div>
`,
})
class InlineMenu {}
31 changes: 22 additions & 9 deletions src/cdk-experimental/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ import {merge, Observable} from 'rxjs';
import {CdkMenuGroup} from './menu-group';
import {CdkMenuPanel} from './menu-panel';
import {Menu, CDK_MENU} from './menu-interface';
import {throwMissingMenuPanelError} from './menu-errors';
import {CdkMenuItem} from './menu-item';
import {MenuStack, MenuStackItem, FocusNext} from './menu-stack';
import {MenuStack, MenuStackItem, FocusNext, NoopMenuStack} from './menu-stack';
import {getItemPointerEntries} from './item-pointer-entries';

/**
Expand All @@ -52,6 +51,7 @@ import {getItemPointerEntries} from './item-pointer-entries';
selector: '[cdkMenu]',
exportAs: 'cdkMenu',
host: {
'[tabindex]': '_isInline() ? 0 : null',
'role': 'menu',
'class': 'cdk-menu',
'[attr.aria-orientation]': 'orientation',
Expand All @@ -71,8 +71,11 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
/** Event emitted when the menu is closed. */
@Output() readonly closed: EventEmitter<void | 'click' | 'tab' | 'escape'> = new EventEmitter();

// We provide a default MenuStack implementation in case the menu is an inline menu.
// For Menus part of a MenuBar nested within a MenuPanel this will be overwritten
// to the correct parent MenuStack.
/** Track the Menus making up the open menu stack. */
_menuStack: MenuStack;
_menuStack: MenuStack = new NoopMenuStack();

/** Handles keyboard events for the menu. */
private _keyManager: FocusKeyManager<CdkMenuItem>;
Expand Down Expand Up @@ -124,6 +127,11 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
this._subscribeToMouseManager();
}

// In Ivy the `host` metadata will be merged, whereas in ViewEngine it is overridden. In order
// to avoid double event listeners, we need to use `HostListener`. Once Ivy is the default, we
// can move this back into `host`.
// tslint:disable:no-host-decorator-in-concrete
@HostListener('focus')
/** Place focus on the first MenuItem in the menu and set the focus origin. */
focusFirstItem(focusOrigin: FocusOrigin = 'program') {
this._keyManager.setFocusOrigin(focusOrigin);
Expand Down Expand Up @@ -181,12 +189,7 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI

/** Register this menu with its enclosing parent menu panel */
private _registerWithParentPanel() {
const parent = this._getMenuPanel();
if (parent) {
parent._registerMenu(this);
} else {
throwMissingMenuPanelError();
}
this._getMenuPanel()?._registerMenu(this);
}

/**
Expand Down Expand Up @@ -323,6 +326,16 @@ export class CdkMenu extends CdkMenuGroup implements Menu, AfterContentInit, OnI
return this.orientation === 'horizontal';
}

/**
* Return true if this menu is an inline menu. That is, it does not exist in a pop-up and is
* always visible in the dom.
*/
_isInline() {
// NoopMenuStack is the default. If this menu is not inline than the NoopMenuStack is replaced
// automatically.
return this._menuStack instanceof NoopMenuStack;
}

ngOnDestroy() {
this._emitClosedEvent();
}
Expand Down
2 changes: 1 addition & 1 deletion src/cdk-experimental/menu/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ export * from './menu-panel';
export * from './menu-group';
export * from './context-menu';

export * from './menu-stack';
export {MenuStack, MenuStackItem} from './menu-stack';
export {CDK_MENU} from './menu-interface';