Skip to content

Commit

Permalink
fixup! fix(cdk-experimental/menu): API, code, and docs cleanup pass
Browse files Browse the repository at this point in the history
  • Loading branch information
mmalerba committed Apr 8, 2022
1 parent 7148d2a commit 673d7f8
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 71 deletions.
3 changes: 2 additions & 1 deletion src/cdk-experimental/menu/context-menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ export class CdkContextMenuTrigger extends CdkMenuTriggerBase implements OnDestr
private _open(coordinates: ContextMenuCoordinates, ignoreFirstOutsideAuxClick: boolean) {
if (this.disabled) {
return;
} else if (this.isOpen()) {
}
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.childMenu!);
Expand Down
46 changes: 30 additions & 16 deletions src/cdk-experimental/menu/menu-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,15 @@ import {
Self,
} from '@angular/core';
import {Directionality} from '@angular/cdk/bidi';
import {DOWN_ARROW, ESCAPE, LEFT_ARROW, RIGHT_ARROW, TAB, UP_ARROW} from '@angular/cdk/keycodes';
import {
DOWN_ARROW,
ESCAPE,
hasModifierKey,
LEFT_ARROW,
RIGHT_ARROW,
TAB,
UP_ARROW,
} from '@angular/cdk/keycodes';
import {takeUntil} from 'rxjs/operators';
import {CdkMenuGroup} from './menu-group';
import {CDK_MENU} from './menu-interface';
Expand Down Expand Up @@ -82,31 +90,37 @@ export class CdkMenuBar extends CdkMenuBase implements AfterContentInit {
case DOWN_ARROW:
case LEFT_ARROW:
case RIGHT_ARROW:
const horizontalArrows = event.keyCode === LEFT_ARROW || event.keyCode === RIGHT_ARROW;
// For a horizontal menu if the left/right keys were clicked, or a vertical menu if the
// up/down keys were clicked: if the current menu is open, close it then focus and open the
// next menu.
if (horizontalArrows) {
event.preventDefault();
if (!hasModifierKey(event)) {
const horizontalArrows = event.keyCode === LEFT_ARROW || event.keyCode === RIGHT_ARROW;
// For a horizontal menu if the left/right keys were clicked, or a vertical menu if the
// up/down keys were clicked: if the current menu is open, close it then focus and open the
// next menu.
if (horizontalArrows) {
event.preventDefault();

const prevIsOpen = keyManager.activeItem?.isMenuOpen();
keyManager.activeItem?.getMenuTrigger()?.close();
const prevIsOpen = keyManager.activeItem?.isMenuOpen();
keyManager.activeItem?.getMenuTrigger()?.close();

keyManager.setFocusOrigin('keyboard');
keyManager.onKeydown(event);
if (prevIsOpen) {
keyManager.activeItem?.getMenuTrigger()?.open();
keyManager.setFocusOrigin('keyboard');
keyManager.onKeydown(event);
if (prevIsOpen) {
keyManager.activeItem?.getMenuTrigger()?.open();
}
}
}
break;

case ESCAPE:
event.preventDefault();
keyManager.activeItem?.getMenuTrigger()?.close();
if (!hasModifierKey(event)) {
event.preventDefault();
keyManager.activeItem?.getMenuTrigger()?.close();
}
break;

case TAB:
keyManager.activeItem?.getMenuTrigger()?.close();
if (!hasModifierKey(event, 'altKey', 'metaKey', 'ctrlKey')) {
keyManager.activeItem?.getMenuTrigger()?.close();
}
break;

default:
Expand Down
23 changes: 11 additions & 12 deletions src/cdk-experimental/menu/menu-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ let nextId = 0;
host: {
'role': 'menu',
'class': '', // reset the css class added by the super-class
'[tabindex]': '_tabIndex',
'[tabindex]': '_getTabIndex()',
'[id]': 'id',
'[attr.aria-orientation]': 'orientation',
'[attr.data-cdk-menu-stack-id]': 'menuStack.id',
Expand Down Expand Up @@ -70,9 +70,6 @@ export abstract class CdkMenuBase
/** The menu's native DOM host element. */
readonly nativeElement: HTMLElement;

/** Whether this menu's menu stack has focus. */
_tabIndex: number | null;

/** Handles keyboard events for the menu. */
protected keyManager: FocusKeyManager<CdkMenuItem>;

Expand All @@ -85,6 +82,9 @@ export abstract class CdkMenuBase
/** Tracks the users mouse movements over the menu. */
protected pointerTracker?: PointerFocusTracker<CdkMenuItem>;

/** Whether this menu's menu stack has focus. */
private _menuStackHasFocus = false;

protected constructor(
/** The host element. */
elementRef: ElementRef<HTMLElement>,
Expand All @@ -105,7 +105,6 @@ export abstract class CdkMenuBase
if (!this.isInline) {
this.menuStack.push(this);
}
this._calculateTabIndex(false);
this._setKeyManager();
this._subscribeToMenuStackHasFocus();
this._subscribeToMenuOpen();
Expand Down Expand Up @@ -137,6 +136,12 @@ export abstract class CdkMenuBase
this.keyManager.setLastItemActive();
}

/** Gets the tabindex for this menu. */
_getTabIndex() {
const tabindexIfInline = this._menuStackHasFocus ? -1 : 0;
return this.isInline ? tabindexIfInline : null;
}

/**
* Close the open menu if the current active item opened the requested MenuStackItem.
* @param menu The menu requested to be closed.
Expand Down Expand Up @@ -207,17 +212,11 @@ export abstract class CdkMenuBase
private _subscribeToMenuStackHasFocus() {
if (this.isInline) {
this.menuStack.hasFocus.pipe(takeUntil(this.destroyed)).subscribe(hasFocus => {
this._calculateTabIndex(hasFocus);
this._menuStackHasFocus = hasFocus;
});
}
}

/** Calculate the tabindex for the menu host element. */
private _calculateTabIndex(menuStackHasFocus: boolean) {
const tabindexIfInline = menuStackHasFocus ? -1 : 0;
this._tabIndex = this.isInline ? tabindexIfInline : null;
}

/**
* Set the PointerFocusTracker and ensure that when mouse focus changes the key manager is updated
* with the latest menu item under mouse focus.
Expand Down
6 changes: 3 additions & 3 deletions src/cdk-experimental/menu/menu-item.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class SingleMenuItem {}

@Component({
template: `
<button cdkMenuItem [cdkMenuItemTypeahead]="typeahead">
<button cdkMenuItem [cdkMenuitemTypeaheadLabel]="typeahead">
<mat-icon>unicorn</mat-icon>
Click me!
</button>
Expand All @@ -153,7 +153,7 @@ class MenuItemWithIcon {

@Component({
template: `
<button cdkMenuItem [cdkMenuItemTypeahead]="typeahead">
<button cdkMenuItem [cdkMenuitemTypeaheadLabel]="typeahead">
<div class="material-icons">unicorn</div>
Click me!
</button>
Expand All @@ -170,7 +170,7 @@ class MenuItemWithBoldElement {}

@Component({
template: `
<button cdkMenuItem [cdkMenuItemTypeahead]="typeahead">
<button cdkMenuItem [cdkMenuitemTypeaheadLabel]="typeahead">
<div>
<div class="material-icons">unicorn</div>
<div>
Expand Down
37 changes: 22 additions & 15 deletions src/cdk-experimental/menu/menu-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from '@angular/core';
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
import {FocusableOption} from '@angular/cdk/a11y';
import {ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
import {ENTER, hasModifierKey, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
import {Directionality} from '@angular/cdk/bidi';
import {fromEvent, Subject} from 'rxjs';
import {filter, takeUntil} from 'rxjs/operators';
Expand Down Expand Up @@ -64,7 +64,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
* The text used to locate this item during menu typeahead. If not specified,
* the `textContent` of the item will be used.
*/
@Input('cdkMenuItemTypeahead') typeahead: string;
@Input('cdkMenuitemTypeaheadLabel') typeaheadLabel: string | null;

/**
* If this MenuItem is a regular MenuItem, outputs when it is triggered by a keyboard or mouse
Expand Down Expand Up @@ -113,6 +113,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,

ngOnDestroy() {
this.destroyed.next();
this.destroyed.complete();
}

/** Place focus on the element. */
Expand Down Expand Up @@ -156,7 +157,7 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,

/** Get the label for this element which is required by the FocusableOption interface. */
getLabel(): string {
return this.typeahead || this._elementRef.nativeElement.textContent?.trim() || '';
return this.typeaheadLabel || this._elementRef.nativeElement.textContent?.trim() || '';
}

/** Reset the tabindex to -1. */
Expand Down Expand Up @@ -191,26 +192,32 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
switch (event.keyCode) {
case SPACE:
case ENTER:
event.preventDefault();
this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger});
if (!hasModifierKey(event)) {
event.preventDefault();
this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger});
}
break;

case RIGHT_ARROW:
if (this._parentMenu && this._isParentVertical()) {
if (this._dir?.value !== 'rtl') {
this._forwardArrowPressed(event);
} else {
this._backArrowPressed(event);
if (!hasModifierKey(event)) {
if (this._parentMenu && this._isParentVertical()) {
if (this._dir?.value !== 'rtl') {
this._forwardArrowPressed(event);
} else {
this._backArrowPressed(event);
}
}
}
break;

case LEFT_ARROW:
if (this._parentMenu && this._isParentVertical()) {
if (this._dir?.value !== 'rtl') {
this._backArrowPressed(event);
} else {
this._forwardArrowPressed(event);
if (!hasModifierKey(event)) {
if (this._parentMenu && this._isParentVertical()) {
if (this._dir?.value !== 'rtl') {
this._backArrowPressed(event);
} else {
this._forwardArrowPressed(event);
}
}
}
break;
Expand Down
52 changes: 34 additions & 18 deletions src/cdk-experimental/menu/menu-trigger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,15 @@ import {
STANDARD_DROPDOWN_ADJACENT_POSITIONS,
STANDARD_DROPDOWN_BELOW_POSITIONS,
} from '@angular/cdk/overlay';
import {DOWN_ARROW, ENTER, LEFT_ARROW, RIGHT_ARROW, SPACE, UP_ARROW} from '@angular/cdk/keycodes';
import {
DOWN_ARROW,
ENTER,
hasModifierKey,
LEFT_ARROW,
RIGHT_ARROW,
SPACE,
UP_ARROW,
} from '@angular/cdk/keycodes';
import {fromEvent} from 'rxjs';
import {filter, takeUntil} from 'rxjs/operators';
import {CDK_MENU, Menu} from './menu-interface';
Expand Down Expand Up @@ -130,35 +138,43 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
switch (keyCode) {
case SPACE:
case ENTER:
event.preventDefault();
this.toggle();
this.childMenu?.focusFirstItem('keyboard');
if (!hasModifierKey(event)) {
event.preventDefault();
this.toggle();
this.childMenu?.focusFirstItem('keyboard');
}
break;

case RIGHT_ARROW:
if (this._parentMenu && isParentVertical && this._directionality?.value !== 'rtl') {
event.preventDefault();
this.open();
this.childMenu?.focusFirstItem('keyboard');
if (!hasModifierKey(event)) {
if (this._parentMenu && isParentVertical && this._directionality?.value !== 'rtl') {
event.preventDefault();
this.open();
this.childMenu?.focusFirstItem('keyboard');
}
}
break;

case LEFT_ARROW:
if (this._parentMenu && isParentVertical && this._directionality?.value === 'rtl') {
event.preventDefault();
this.open();
this.childMenu?.focusFirstItem('keyboard');
if (!hasModifierKey(event)) {
if (this._parentMenu && isParentVertical && this._directionality?.value === 'rtl') {
event.preventDefault();
this.open();
this.childMenu?.focusFirstItem('keyboard');
}
}
break;

case DOWN_ARROW:
case UP_ARROW:
if (!isParentVertical) {
event.preventDefault();
this.open();
keyCode === DOWN_ARROW
? this.childMenu?.focusFirstItem('keyboard')
: this.childMenu?.focusLastItem('keyboard');
if (!hasModifierKey(event)) {
if (!isParentVertical) {
event.preventDefault();
this.open();
keyCode === DOWN_ARROW
? this.childMenu?.focusFirstItem('keyboard')
: this.childMenu?.focusLastItem('keyboard');
}
}
break;
}
Expand Down
12 changes: 8 additions & 4 deletions src/cdk-experimental/menu/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,11 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
switch (event.keyCode) {
case LEFT_ARROW:
case RIGHT_ARROW:
event.preventDefault();
keyManager.setFocusOrigin('keyboard');
keyManager.onKeydown(event);
if (!hasModifierKey(event)) {
event.preventDefault();
keyManager.setFocusOrigin('keyboard');
keyManager.onKeydown(event);
}
break;

case ESCAPE:
Expand All @@ -119,7 +121,9 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
break;

case TAB:
this.menuStack.closeAll({focusParentTrigger: true});
if (!hasModifierKey(event, 'altKey', 'metaKey', 'ctrlKey')) {
this.menuStack.closeAll({focusParentTrigger: true});
}
break;

default:
Expand Down
4 changes: 2 additions & 2 deletions src/material-experimental/menubar/menubar-item.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ function removeIcons(element: Element) {
})
export class MatMenuBarItem extends CdkMenuItem {
override getLabel(): string {
if (this.typeahead !== undefined) {
return this.typeahead;
if (this.typeaheadLabel !== undefined) {
return this.typeaheadLabel || '';
}
const clone = this._elementRef.nativeElement.cloneNode(true) as Element;
removeIcons(clone);
Expand Down

0 comments on commit 673d7f8

Please sign in to comment.