Skip to content

Commit 8e4a2ec

Browse files
authored
fix(cdk/menu): control + option + space not working on VoiceOver (#27401)
In #26296 I added a condition in the click handler for menu triggers and menu items to skip clicks dispatched by the keyboard so that they don't trigger the menu twice. The problem is that this also ends up ignoring the default keyboard shortcut that VoiceOver mentions should be used to trigger the menu (Control + Option + Space), because it ends up being dispatched as a plain `click` event and it doesn't trigger the `keydown` event at all. These changes address the issue by removing the previous condition and inferring whether the event will trigger a click by looking at it and the element it's coming from. Fixes #27376.
1 parent d9c16c5 commit 8e4a2ec

File tree

4 files changed

+47
-23
lines changed

4 files changed

+47
-23
lines changed

src/cdk/menu/event-detection.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import {ElementRef} from '@angular/core';
10+
import {ENTER, SPACE} from '@angular/cdk/keycodes';
11+
12+
/** Checks whether a keyboard event will trigger a native `click` event on an element. */
13+
export function eventDispatchesNativeClick(
14+
elementRef: ElementRef<HTMLElement>,
15+
event: KeyboardEvent,
16+
): boolean {
17+
// Synthetic events won't trigger clicks.
18+
if (!event.isTrusted) {
19+
return false;
20+
}
21+
22+
const el = elementRef.nativeElement;
23+
const keyCode = event.keyCode;
24+
25+
// Buttons trigger clicks both on space and enter events.
26+
if (el.nodeName === 'BUTTON' && !(el as HTMLButtonElement).disabled) {
27+
return keyCode === ENTER || keyCode === SPACE;
28+
}
29+
30+
// Links only trigger clicks on enter.
31+
if (el.nodeName === 'A') {
32+
return keyCode === ENTER;
33+
}
34+
35+
// Any other elements won't dispatch clicks from keyboard events.
36+
return false;
37+
}

src/cdk/menu/menu-item.ts

Lines changed: 5 additions & 13 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, InputModalityDetector} from '@angular/cdk/a11y';
20+
import {FocusableOption} 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';
@@ -27,6 +27,7 @@ import {CDK_MENU, Menu} from './menu-interface';
2727
import {FocusNext, MENU_STACK} from './menu-stack';
2828
import {FocusableElement} from './pointer-focus-tracker';
2929
import {MENU_AIM, Toggler} from './menu-aim';
30+
import {eventDispatchesNativeClick} from './event-detection';
3031

3132
/**
3233
* Directive which provides the ability for an element to be focused and navigated to using the
@@ -44,13 +45,12 @@ import {MENU_AIM, Toggler} from './menu-aim';
4445
'[attr.aria-disabled]': 'disabled || null',
4546
'(blur)': '_resetTabIndex()',
4647
'(focus)': '_setTabIndex()',
47-
'(click)': '_handleClick()',
48+
'(click)': 'trigger()',
4849
'(keydown)': '_onKeydown($event)',
4950
},
5051
})
5152
export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler, OnDestroy {
5253
protected readonly _dir = inject(Directionality, {optional: true});
53-
private readonly _inputModalityDetector = inject(InputModalityDetector);
5454
readonly _elementRef: ElementRef<HTMLElement> = inject(ElementRef);
5555
protected _ngZone = inject(NgZone);
5656

@@ -195,7 +195,8 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
195195
switch (event.keyCode) {
196196
case SPACE:
197197
case ENTER:
198-
if (!hasModifierKey(event)) {
198+
// Skip events that will trigger clicks so the handler doesn't get triggered twice.
199+
if (!hasModifierKey(event) && !eventDispatchesNativeClick(this._elementRef, event)) {
199200
this.trigger({keepOpen: event.keyCode === SPACE && !this.closeOnSpacebarTrigger});
200201
}
201202
break;
@@ -226,15 +227,6 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
226227
}
227228
}
228229

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-
238230
/** Whether this menu item is standalone or within a menu or menu bar. */
239231
private _isStandaloneItem() {
240232
return !this._parentMenu;

src/cdk/menu/menu-trigger.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@ import {
2626
UP_ARROW,
2727
} from '@angular/cdk/keycodes';
2828
import {_getEventTarget} from '@angular/cdk/platform';
29-
import {InputModalityDetector} from '@angular/cdk/a11y';
3029
import {fromEvent} from 'rxjs';
3130
import {filter, takeUntil} from 'rxjs/operators';
3231
import {CDK_MENU, Menu} from './menu-interface';
3332
import {PARENT_OR_NEW_MENU_STACK_PROVIDER} from './menu-stack';
3433
import {MENU_AIM} from './menu-aim';
3534
import {CdkMenuTriggerBase, MENU_TRIGGER} from './menu-trigger-base';
35+
import {eventDispatchesNativeClick} from './event-detection';
3636

3737
/**
3838
* A directive that turns its host element into a trigger for a popup menu.
@@ -70,7 +70,6 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
7070
private readonly _overlay = inject(Overlay);
7171
private readonly _ngZone = inject(NgZone);
7272
private readonly _directionality = inject(Directionality, {optional: true});
73-
private readonly _inputModalityDetector = inject(InputModalityDetector);
7473

7574
/** The parent menu this trigger belongs to. */
7675
private readonly _parentMenu = inject(CDK_MENU, {optional: true});
@@ -130,7 +129,8 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
130129
switch (event.keyCode) {
131130
case SPACE:
132131
case ENTER:
133-
if (!hasModifierKey(event)) {
132+
// Skip events that will trigger clicks so the handler doesn't get triggered twice.
133+
if (!hasModifierKey(event) && !eventDispatchesNativeClick(this._elementRef, event)) {
134134
this.toggle();
135135
this.childMenu?.focusFirstItem('keyboard');
136136
}
@@ -173,12 +173,8 @@ export class CdkMenuTrigger extends CdkMenuTriggerBase implements OnDestroy {
173173

174174
/** Handles clicks on the menu trigger. */
175175
_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-
this.childMenu?.focusFirstItem('mouse');
181-
}
176+
this.toggle();
177+
this.childMenu?.focusFirstItem('mouse');
182178
}
183179

184180
/**

tools/public_api_guard/cdk/menu.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ export class CdkMenuItem implements FocusableOption, FocusableElement, Toggler,
129129
getLabel(): string;
130130
getMenu(): Menu | undefined;
131131
getMenuTrigger(): CdkMenuTrigger | null;
132-
_handleClick(): void;
133132
get hasMenu(): boolean;
134133
isMenuOpen(): boolean;
135134
// (undocumented)

0 commit comments

Comments
 (0)