Skip to content

Commit 9531b68

Browse files
committed
fix(cdk-experimental/menu): don't steal focus on hover
1 parent 6f9743c commit 9531b68

File tree

8 files changed

+48
-45
lines changed

8 files changed

+48
-45
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ export class CdkContextMenuTrigger extends MenuTrigger implements OnDestroy {
216216

217217
/** Subscribe to the menu stack close events and close this menu when requested. */
218218
private _setMenuStackListener() {
219-
this.menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(item => {
219+
this.menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(({item}) => {
220220
if (item === this.childMenu && this.isOpen()) {
221221
this.closed.next();
222222
this._overlayRef!.detach();

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,6 @@ export class CdkMenuBar extends CdkMenuBase implements AfterContentInit, OnDestr
128128
private _subscribeToMouseManager() {
129129
this._ngZone.runOutsideAngular(() => {
130130
this.pointerTracker = new PointerFocusTracker(this.items);
131-
this.pointerTracker.entered.pipe(takeUntil(this.destroyed)).subscribe(item => {
132-
if (this.hasOpenSubmenu()) {
133-
this.keyManager.setActiveItem(item);
134-
}
135-
});
136131
});
137132
}
138133

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,19 @@ export abstract class CdkMenuBase
105105
* Close the open menu if the current active item opened the requested MenuStackItem.
106106
* @param item the MenuStackItem requested to be closed.
107107
*/
108-
protected closeOpenMenu(menu: MenuStackItem | undefined) {
108+
protected closeOpenMenu(menu: MenuStackItem | undefined, focusTrigger?: boolean) {
109109
const keyManager = this.keyManager;
110110
const trigger = this.openItem;
111111
if (menu === trigger?.getMenuTrigger()?.getMenu()) {
112112
trigger?.getMenuTrigger()?.close();
113113
// If the user has moused over a sibling item we want to focus the element under mouse focus
114114
// not the trigger which previously opened the now closed menu.
115-
if (trigger) {
116-
keyManager.setActiveItem(this.pointerTracker?.activeElement || trigger);
115+
if (focusTrigger) {
116+
if (trigger) {
117+
keyManager.setActiveItem(trigger);
118+
} else {
119+
keyManager.setFirstItemActive();
120+
}
117121
}
118122
}
119123
}
@@ -157,6 +161,6 @@ export abstract class CdkMenuBase
157161
private _subscribeToMenuStackClosed() {
158162
this.menuStack.closed
159163
.pipe(takeUntil(this.destroyed))
160-
.subscribe(item => this.closeOpenMenu(item));
164+
.subscribe(({item, focusParentMenu}) => this.closeOpenMenu(item, focusParentMenu));
161165
}
162166
}

src/cdk-experimental/menu/menu-item-trigger.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ export class CdkMenuItemTrigger extends MenuTrigger implements OnDestroy {
278278
*/
279279
private _registerCloseHandler() {
280280
if (!this._parentMenu) {
281-
this.menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(item => {
281+
this.menuStack.closed.pipe(takeUntil(this._destroyed)).subscribe(({item}) => {
282282
if (item === this.childMenu) {
283283
this.close();
284284
}

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

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,7 @@ import {MENU_AIM, MenuAim, Toggler} from './menu-aim';
4545
'class': 'cdk-menu-item',
4646
'[attr.aria-disabled]': 'disabled || null',
4747
'(blur)': '_resetTabIndex()',
48-
'(mouseout)': '_resetTabIndex()',
4948
'(focus)': '_setTabIndex()',
50-
'(mouseenter)': '_setTabIndex($event)',
5149
'(click)': 'trigger()',
5250
'(keydown)': '_onKeydown($event)',
5351
},
@@ -190,38 +188,41 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
190188

191189
case RIGHT_ARROW:
192190
if (this._parentMenu && this._isParentVertical()) {
193-
if (this._dir?.value === 'rtl') {
194-
if (this._menuStack.hasInlineMenu() || this._menuStack.length() > 1) {
195-
event.preventDefault();
196-
this._menuStack.close(this._parentMenu, FocusNext.previousItem);
197-
}
198-
} else if (!this.hasMenu()) {
199-
if (this._menuStack.hasInlineMenu()) {
200-
event.preventDefault();
201-
this._menuStack.closeAll(FocusNext.nextItem);
202-
}
191+
if (this._dir?.value !== 'rtl') {
192+
this._forwardArrowPressed(event);
193+
} else {
194+
this._backArrowPressed(event);
203195
}
204196
}
205197
break;
206198

207199
case LEFT_ARROW:
208200
if (this._parentMenu && this._isParentVertical()) {
209201
if (this._dir?.value !== 'rtl') {
210-
if (this._menuStack.hasInlineMenu() || this._menuStack.length() > 1) {
211-
event.preventDefault();
212-
this._menuStack.close(this._parentMenu, FocusNext.previousItem);
213-
}
214-
} else if (!this.hasMenu()) {
215-
if (this._menuStack.hasInlineMenu()) {
216-
event.preventDefault();
217-
this._menuStack.closeAll(FocusNext.nextItem);
218-
}
202+
this._backArrowPressed(event);
203+
} else {
204+
this._forwardArrowPressed(event);
219205
}
220206
}
221207
break;
222208
}
223209
}
224210

211+
private _backArrowPressed(event: KeyboardEvent) {
212+
const parentMenu = this._parentMenu!;
213+
if (this._menuStack.hasInlineMenu() || this._menuStack.length() > 1) {
214+
event.preventDefault();
215+
this._menuStack.close(parentMenu, FocusNext.previousItem, true);
216+
}
217+
}
218+
219+
private _forwardArrowPressed(event: KeyboardEvent) {
220+
if (!this.hasMenu() && this._menuStack.hasInlineMenu()) {
221+
event.preventDefault();
222+
this._menuStack.closeAll(FocusNext.nextItem);
223+
}
224+
}
225+
225226
/**
226227
* Subscribe to the mouseenter events and close any sibling menu items if this element is moused
227228
* into.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe('MenuStack', () => {
5656
menuStack.closeAll();
5757

5858
expect(spy).toHaveBeenCalledTimes(3);
59-
const callArgs = spy.calls.all().map((v: jasmine.CallInfo<jasmine.Func>) => v.args[0]);
59+
const callArgs = spy.calls.all().map((v: jasmine.CallInfo<jasmine.Func>) => v.args[0].item);
6060
expect(callArgs).toEqual(menus.reverse());
6161
expect(menuStack.isEmpty()).toBeTrue();
6262
},

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ export const PARENT_OR_NEW_INLINE_MENU_STACK_PROVIDER = {
4141
useFactory: (parentMenuStack?: MenuStack) => parentMenuStack || MenuStack.inline(),
4242
};
4343

44+
export interface MenuStackCloseEvent {
45+
item: MenuStackItem | undefined;
46+
focusParentMenu?: boolean;
47+
}
48+
4449
/**
4550
* MenuStack allows subscribers to listen for close events (when a MenuStackItem is popped off
4651
* of the stack) in order to perform closing actions. Upon the MenuStack being empty it emits
@@ -53,13 +58,13 @@ export class MenuStack {
5358
private readonly _elements: MenuStackItem[] = [];
5459

5560
/** Emits the element which was popped off of the stack when requested by a closer. */
56-
private readonly _close: Subject<MenuStackItem | undefined> = new Subject();
61+
private readonly _close: Subject<MenuStackCloseEvent> = new Subject();
5762

5863
/** Emits once the MenuStack has become empty after popping off elements. */
5964
private readonly _empty: Subject<FocusNext | undefined> = new Subject();
6065

6166
/** Observable which emits the MenuStackItem which has been requested to close. */
62-
readonly closed: Observable<MenuStackItem | undefined> = this._close;
67+
readonly closed: Observable<MenuStackCloseEvent> = this._close;
6368

6469
/**
6570
* Observable which emits when the MenuStack is empty after popping off the last element. It
@@ -87,14 +92,15 @@ export class MenuStack {
8792
* @param lastItem the last item to pop off the stack.
8893
* @param focusNext the event to emit on the `empty` observable if the method call resulted in an
8994
* empty stack. Does not emit if the stack was initially empty or if `lastItem` was not on the
95+
* @param focusParentMenu Whether to focus the parent menu after closing current one.
9096
* stack.
9197
*/
92-
close(lastItem: MenuStackItem, focusNext?: FocusNext) {
98+
close(lastItem: MenuStackItem, focusNext?: FocusNext, focusParentMenu = false) {
9399
if (this._elements.indexOf(lastItem) >= 0) {
94100
let poppedElement: MenuStackItem | undefined;
95101
do {
96102
poppedElement = this._elements.pop();
97-
this._close.next(poppedElement);
103+
this._close.next({item: poppedElement, focusParentMenu});
98104
} while (poppedElement !== lastItem);
99105

100106
if (this.isEmpty()) {
@@ -114,7 +120,7 @@ export class MenuStack {
114120
if (this._elements.indexOf(lastItem) >= 0) {
115121
removed = this.peek() !== lastItem;
116122
while (this.peek() !== lastItem) {
117-
this._close.next(this._elements.pop());
123+
this._close.next({item: this._elements.pop()});
118124
}
119125
}
120126
return removed;
@@ -125,12 +131,12 @@ export class MenuStack {
125131
* @param focusNext the event to emit on the `empty` observable once the stack is emptied. Does
126132
* not emit if the stack was initially empty.
127133
*/
128-
closeAll(focusNext?: FocusNext) {
134+
closeAll(focusNext?: FocusNext, focusParentMenu = false) {
129135
if (!this.isEmpty()) {
130136
while (!this.isEmpty()) {
131137
const menuStackItem = this._elements.pop();
132138
if (menuStackItem) {
133-
this._close.next(menuStackItem);
139+
this._close.next({item: menuStackItem, focusParentMenu});
134140
}
135141
}
136142

src/cdk-experimental/menu/menu.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,12 +130,12 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
130130
case ESCAPE:
131131
if (!hasModifierKey(event)) {
132132
event.preventDefault();
133-
this.menuStack.close(this, FocusNext.currentItem);
133+
this.menuStack.close(this, FocusNext.currentItem, true);
134134
}
135135
break;
136136

137137
case TAB:
138-
this.menuStack.closeAll();
138+
this.menuStack.closeAll(undefined, true);
139139
break;
140140

141141
default:
@@ -174,9 +174,6 @@ export class CdkMenu extends CdkMenuBase implements AfterContentInit, OnDestroy
174174
private _subscribeToMouseManager() {
175175
this._ngZone.runOutsideAngular(() => {
176176
this.pointerTracker = new PointerFocusTracker(this.items);
177-
this.pointerTracker.entered
178-
.pipe(takeUntil(this.closed))
179-
.subscribe(item => this.keyManager.setActiveItem(item));
180177
});
181178
}
182179

0 commit comments

Comments
 (0)