Skip to content

Commit 836b777

Browse files
authored
fix(cdk/menu): don't prevent default selection key actions (#26296)
In #26051 a couple of `preventDefault` calls were added, because we were moving focus within an enter key `keydown` event which was causing the menu to close immediately after it opens. The problem is that the calls also prevent links from navigating. These changes remove the `preventDefault` calls and resolve the initial issue by ignoring clicks initiated by the keyboard. Fixes #26291.
1 parent 7ab4b1e commit 836b777

File tree

5 files changed

+35
-23
lines changed

5 files changed

+35
-23
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ describe('MenuItem', () => {
6767
expect(menuItem.hasMenu).toBeFalse();
6868
});
6969

70-
it('should prevent the default selection key action', () => {
70+
it('should not prevent the default selection key action', () => {
7171
const event = dispatchKeyboardEvent(nativeButton, 'keydown', ENTER);
7272
fixture.detectChanges();
73-
expect(event.defaultPrevented).toBe(true);
73+
expect(event.defaultPrevented).toBe(false);
7474
});
7575
});
7676

src/cdk/menu/menu-item.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
Output,
1818
} from '@angular/core';
1919
import {BooleanInput, coerceBooleanProperty} from '@angular/cdk/coercion';
20-
import {FocusableOption} from '@angular/cdk/a11y';
20+
import {FocusableOption, InputModalityDetector} from '@angular/cdk/a11y';
2121
import {ENTER, hasModifierKey, LEFT_ARROW, RIGHT_ARROW, SPACE} from '@angular/cdk/keycodes';
2222
import {Directionality} from '@angular/cdk/bidi';
2323
import {fromEvent, Subject} from 'rxjs';
@@ -44,18 +44,14 @@ import {MENU_AIM, Toggler} from './menu-aim';
4444
'[attr.aria-disabled]': 'disabled || null',
4545
'(blur)': '_resetTabIndex()',
4646
'(focus)': '_setTabIndex()',
47-
'(click)': 'trigger()',
47+
'(click)': '_handleClick()',
4848
'(keydown)': '_onKeydown($event)',
4949
},
5050
})
5151
export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, OnDestroy {
52-
/** The directionality (text direction) of the current page. */
5352
protected readonly _dir = inject(Directionality, {optional: true});
54-
55-
/** The menu's native DOM host element. */
53+
private readonly _inputModalityDetector = inject(InputModalityDetector);
5654
readonly _elementRef: ElementRef<HTMLElement> = inject(ElementRef);
57-
58-
/** The Angular zone. */
5955
protected _ngZone = inject(NgZone);
6056

6157
/** The menu aim service used by this menu. */
@@ -200,7 +196,6 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
200196
case SPACE:
201197
case ENTER:
202198
if (!hasModifierKey(event)) {
203-
event.preventDefault();
204199
this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger});
205200
}
206201
break;
@@ -231,6 +226,15 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
231226
}
232227
}
233228

229+
/** Handles clicks on the menu item. */
230+
_handleClick() {
231+
// Don't handle clicks originating from the keyboard since we
232+
// already do the same on `keydown` events for enter and space.
233+
if (this._inputModalityDetector.mostRecentModality !== 'keyboard') {
234+
this.trigger();
235+
}
236+
}
237+
234238
/** Whether this menu item is standalone or within a menu or menu bar. */
235239
private _isStandaloneItem() {
236240
return !this._parentMenu;

src/cdk/menu/menu-trigger.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ describe('MenuTrigger', () => {
438438
expect(nativeMenus.length).toBe(2);
439439
});
440440

441-
it('should toggle the menu on trigger', () => {
441+
it('should toggle the menu on clicks', () => {
442442
nativeTrigger.click();
443443
detectChanges();
444444
expect(nativeMenus.length).toBe(2);
@@ -451,13 +451,13 @@ describe('MenuTrigger', () => {
451451
it('should toggle the menu on keyboard events', () => {
452452
const firstEvent = dispatchKeyboardEvent(nativeTrigger, 'keydown', ENTER);
453453
detectChanges();
454-
expect(firstEvent.defaultPrevented).toBe(true);
454+
expect(firstEvent.defaultPrevented).toBe(false);
455455
expect(nativeMenus.length).toBe(2);
456456

457457
const secondEvent = dispatchKeyboardEvent(nativeTrigger, 'keydown', ENTER);
458458
detectChanges();
459459
expect(nativeMenus.length).toBe(1);
460-
expect(secondEvent.defaultPrevented).toBe(true);
460+
expect(secondEvent.defaultPrevented).toBe(false);
461461
});
462462

463463
it('should close the open menu on background click', () => {

src/cdk/menu/menu-trigger.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
UP_ARROW,
2727
} from '@angular/cdk/keycodes';
2828
import {_getEventTarget} from '@angular/cdk/platform';
29+
import {InputModalityDetector} from '@angular/cdk/a11y';
2930
import {fromEvent} from 'rxjs';
3031
import {filter, takeUntil} from 'rxjs/operators';
3132
import {CDK_MENU, Menu} from './menu-interface';
@@ -51,7 +52,7 @@ import {CdkMenuTriggerBase, MENU_TRIGGER} from './menu-trigger-base';
5152
'(focusin)': '_setHasFocus(true)',
5253
'(focusout)': '_setHasFocus(false)',
5354
'(keydown)': '_toggleOnKeydown($event)',
54-
'(click)': 'toggle()',
55+
'(click)': '_handleClick()',
5556
},
5657
inputs: [
5758
'menuTemplateRef: cdkMenuTriggerFor',
@@ -65,24 +66,18 @@ import {CdkMenuTriggerBase, MENU_TRIGGER} from './menu-trigger-base';
6566
],
6667
})
6768
export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
68-
/** The host element. */
6969
private readonly _elementRef: ElementRef<HTMLElement> = inject(ElementRef);
70-
71-
/** The CDK overlay service. */
7270
private readonly _overlay = inject(Overlay);
73-
74-
/** The Angular zone. */
7571
private readonly _ngZone = inject(NgZone);
72+
private readonly _directionality = inject(Directionality, {optional: true});
73+
private readonly _inputModalityDetector = inject(InputModalityDetector);
7674

7775
/** The parent menu this trigger belongs to. */
7876
private readonly _parentMenu = inject(CDK_MENU, {optional: true});
7977

8078
/** The menu aim service used by this menu. */
8179
private readonly _menuAim = inject(MENU_AIM, {optional: true});
8280

83-
/** The directionality of the page. */
84-
private readonly _directionality = inject(Directionality, {optional: true});
85-
8681
constructor() {
8782
super();
8883
this._setRole();
@@ -136,7 +131,6 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
136131
case SPACE:
137132
case ENTER:
138133
if (!hasModifierKey(event)) {
139-
event.preventDefault();
140134
this.toggle();
141135
this.childMenu?.focusFirstItem('keyboard');
142136
}
@@ -177,6 +171,15 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
177171
}
178172
}
179173

174+
/** Handles clicks on the menu trigger. */
175+
_handleClick() {
176+
// Don't handle clicks originating from the keyboard since we
177+
// already do the same on `keydown` events for enter and space.
178+
if (this._inputModalityDetector.mostRecentModality !== 'keyboard') {
179+
this.toggle();
180+
}
181+
}
182+
180183
/**
181184
* Sets whether the trigger's menu stack has focus.
182185
* @param hasFocus Whether the menu stack has focus.

tools/public_api_guard/cdk/menu.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,22 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
119119
constructor();
120120
protected closeOnSpacebarTrigger: boolean;
121121
protected readonly destroyed: Subject<void>;
122+
// (undocumented)
122123
protected readonly _dir: Directionality | null;
123124
get disabled(): boolean;
124125
set disabled(value: BooleanInput);
126+
// (undocumented)
125127
readonly _elementRef: ElementRef<HTMLElement>;
126128
focus(): void;
127129
getLabel(): string;
128130
getMenu(): Menu | undefined;
129131
getMenuTrigger(): CdkMenuTrigger | null;
132+
_handleClick(): void;
130133
get hasMenu(): boolean;
131134
isMenuOpen(): boolean;
132135
// (undocumented)
133136
ngOnDestroy(): void;
137+
// (undocumented)
134138
protected _ngZone: NgZone;
135139
_onKeydown(event: KeyboardEvent): void;
136140
_resetTabIndex(): void;
@@ -198,6 +202,7 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
198202
constructor();
199203
close(): void;
200204
getMenu(): Menu | undefined;
205+
_handleClick(): void;
201206
open(): void;
202207
_setHasFocus(hasFocus: boolean): void;
203208
toggle(): void;

0 commit comments

Comments
 (0)