Skip to content

Commit a9a555b

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[issues] Make sure Enter/Space trigger button clicks.
This reverts the previous CL (https://crrev.com/c/5939709), which switched the logic from 'keydown' to 'keyup', and instead makes sure that the buttons don't propagate 'keydown' events up to their parents, but instead handle these as clicks themselves. This matches the behavior of the treeview examples in the WAI-ARIA pattern guidelines[^1]. [^1]: https://www.w3.org/WAI/ARIA/apg/patterns/ Fixed: 373168872 Change-Id: Ie988c674cd10e93a7c4cef91acc203936061f65e Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5968396 Auto-Submit: Benedikt Meurer <[email protected]> Reviewed-by: Kim-Anh Tran <[email protected]> Commit-Queue: Benedikt Meurer <[email protected]> Commit-Queue: Kim-Anh Tran <[email protected]>
1 parent f3e9646 commit a9a555b

File tree

5 files changed

+25
-20
lines changed

5 files changed

+25
-20
lines changed

front_end/panels/issues/components/HideIssuesMenu.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ export class HideIssuesMenu extends HTMLElement {
4343
this.#shadow.adoptedStyleSheets = [hideIssuesMenuStyles];
4444
}
4545

46-
onMenuOpen(event: Event): void {
46+
onMenuOpen(event: MouseEvent): void {
4747
event.stopPropagation();
48-
const buttonElement = this.#shadow.querySelector('button');
48+
const buttonElement = this.#shadow.querySelector('devtools-button');
4949
const contextMenu = new UI.ContextMenu.ContextMenu(event, {
5050
x: buttonElement?.getBoundingClientRect().left,
5151
y: buttonElement?.getBoundingClientRect().bottom,
@@ -55,6 +55,14 @@ export class HideIssuesMenu extends HTMLElement {
5555
void contextMenu.show();
5656
}
5757

58+
onKeydown(event: KeyboardEvent): void {
59+
if (event.key === 'Enter' || event.key === 'Space') {
60+
// Make sure we don't propagate 'Enter' or 'Space' key events to parents,
61+
// so that these get turned into 'click' events properly.
62+
event.stopImmediatePropagation();
63+
}
64+
}
65+
5866
#render(): void {
5967
// Disabled until https://crbug.com/1079231 is fixed.
6068
// clang-format off
@@ -63,7 +71,8 @@ export class HideIssuesMenu extends HTMLElement {
6371
.data=${{variant: Buttons.Button.Variant.ICON,iconName: 'dots-vertical', title: i18nString(UIStrings.tooltipTitle)} as Buttons.Button.ButtonData}
6472
.jslogContext=${'hide-issues'}
6573
class="hide-issues-menu-btn"
66-
@click=${this.onMenuOpen.bind(this)}></devtools-button>
74+
@click=${this.onMenuOpen}
75+
@keydown=${this.onKeydown}></devtools-button>
6776
`, this.#shadow, {host: this});
6877
}
6978
}

front_end/testing/DOMHelpers.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -163,17 +163,6 @@ export function dispatchKeyDownEvent<T extends Element>(element: T, options: Key
163163
}
164164
}
165165

166-
/**
167-
* Dispatches a keyup event. Errors if the event was not dispatched successfully.
168-
*/
169-
export function dispatchKeyUpEvent<T extends Element>(element: T, options: KeyboardEventInit = {}) {
170-
const clickEvent = new KeyboardEvent('keyup', options);
171-
const success = element.dispatchEvent(clickEvent);
172-
if (!success) {
173-
assert.fail('Failed to trigger keydown event successfully.');
174-
}
175-
}
176-
177166
export function dispatchInputEvent<T extends Element>(element: T, options: InputEventInit = {}) {
178167
const inputEvent = new InputEvent('input', options);
179168
element.dispatchEvent(inputEvent);

front_end/ui/legacy/Treeoutline.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// found in the LICENSE file.
44

55
import * as Platform from '../../core/platform/platform.js';
6-
import {dispatchKeyUpEvent, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
6+
import {dispatchKeyDownEvent, renderElementIntoDOM} from '../../testing/DOMHelpers.js';
77

88
import * as UI from './legacy.js';
99

@@ -18,7 +18,7 @@ describe('TreeOutline', () => {
1818
tree.appendChild(parent);
1919
parent.select();
2020

21-
dispatchKeyUpEvent(tree.contentElement, {bubbles: true, key: 'Enter'});
21+
dispatchKeyDownEvent(tree.contentElement, {bubbles: true, key: 'Enter'});
2222
assert.isTrue(parent.expanded, 'Enter key was supposed to expand the parent node');
2323
});
2424

@@ -32,7 +32,7 @@ describe('TreeOutline', () => {
3232
parent.select();
3333
parent.expand();
3434

35-
dispatchKeyUpEvent(tree.contentElement, {bubbles: true, key: 'Enter'});
35+
dispatchKeyDownEvent(tree.contentElement, {bubbles: true, key: 'Enter'});
3636
assert.isFalse(parent.expanded, 'Enter key was supposed to collapse the parent node');
3737
});
3838
});
@@ -106,7 +106,7 @@ describe('TreeOutline', () => {
106106

107107
function sendKey(key: string) {
108108
const deepActiveElement = Platform.DOMUtilities.deepActiveElement(document);
109-
deepActiveElement!.dispatchEvent(new KeyboardEvent('keyup', {bubbles: true, cancelable: true, key}));
109+
deepActiveElement!.dispatchEvent(new KeyboardEvent('keydown', {bubbles: true, cancelable: true, key}));
110110
}
111111
});
112112
});

front_end/ui/legacy/Treeoutline.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export class TreeOutline extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
9797
this.comparator = null;
9898

9999
this.contentElement = this.rootElementInternal.childrenListNode;
100-
this.contentElement.addEventListener('keyup', this.treeKeyUp.bind(this), false);
100+
this.contentElement.addEventListener('keydown', this.treeKeyDown.bind(this), false);
101101

102102
this.preventTabOrder = false;
103103
this.showSelectionOnKeyboardFocus = false;
@@ -293,7 +293,7 @@ export class TreeOutline extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
293293
return true;
294294
}
295295

296-
private treeKeyUp(event: KeyboardEvent): void {
296+
private treeKeyDown(event: KeyboardEvent): void {
297297
if (event.shiftKey || event.metaKey || event.ctrlKey || isEditing()) {
298298
return;
299299
}

front_end/ui/legacy/UIUtils.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,13 @@ export function createTextButton(text: string, clickHandler?: ((arg0: Event) =>
11001100
button.variant = opts?.variant ? opts.variant : Buttons.Button.Variant.OUTLINED;
11011101
if (clickHandler) {
11021102
button.addEventListener('click', clickHandler);
1103+
button.addEventListener('keydown', (event: KeyboardEvent): void => {
1104+
if (event.key === 'Enter' || event.key === 'Space') {
1105+
// Make sure we don't propagate 'Enter' or 'Space' key events to parents,
1106+
// so that these get turned into 'click' events properly.
1107+
event.stopImmediatePropagation();
1108+
}
1109+
});
11031110
}
11041111
if (opts?.jslogContext) {
11051112
button.setAttribute('jslog', `${VisualLogging.action().track({click: true}).context(opts.jslogContext)}`);

0 commit comments

Comments
 (0)