Skip to content

Commit adad71a

Browse files
oprnoisysocks
authored andcommitted
Stop keypresses being caught by other elements when they happen in a CustomSelectControl (#30557)
* Stop the keydown event from propagating in CustomSelect component * Stop the synthetic event propagating instead of native This stops other listeners receiving this event, whereas stopping the native event still somehow allowed propagation. * Move keyDown handler into useCallback and use optional chaining * Add unit tests for CustomSelectControl * Update changelog * Add inline comment about role and remove unneeded class name * Move mocked function into test * Add correct spacing to changelog * Add options as an array of key value pairs * Use accessible roles instead of looking for elements by className
1 parent 04883dd commit adad71a

File tree

3 files changed

+63
-1
lines changed

3 files changed

+63
-1
lines changed

packages/components/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Bug Fix
6+
7+
- Prevent keyDown events from propagating up in `CustomSelectControl` ([#30557](https://github.com/WordPress/gutenberg/pull/30557))
8+
59
## 19.2.0 (2022-01-04)
610

711
### Experimental

packages/components/src/custom-select-control/index.js

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import classnames from 'classnames';
99
*/
1010
import { Icon, check, chevronDown } from '@wordpress/icons';
1111
import { __, sprintf } from '@wordpress/i18n';
12+
import { useCallback } from '@wordpress/element';
13+
1214
/**
1315
* Internal dependencies
1416
*/
@@ -98,6 +100,15 @@ export default function CustomSelectControl( {
98100
className: 'components-custom-select-control__menu',
99101
'aria-hidden': ! isOpen,
100102
} );
103+
104+
const onKeyDownHandler = useCallback(
105+
( e ) => {
106+
e.stopPropagation();
107+
menuProps?.onKeyDown?.( e );
108+
},
109+
[ menuProps ]
110+
);
111+
101112
// We need this here, because the null active descendant is not fully ARIA compliant.
102113
if (
103114
menuProps[ 'aria-activedescendant' ]?.startsWith( 'downshift-null' )
@@ -141,7 +152,8 @@ export default function CustomSelectControl( {
141152
className="components-custom-select-control__button-icon"
142153
/>
143154
</Button>
144-
<ul { ...menuProps }>
155+
{ /* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */ }
156+
<ul { ...menuProps } onKeyDown={ onKeyDownHandler }>
145157
{ isOpen &&
146158
items.map( ( item, index ) => (
147159
// eslint-disable-next-line react/jsx-key
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* External dependencies
3+
*/
4+
import { render, fireEvent, screen } from '@testing-library/react';
5+
6+
/**
7+
* WordPress dependencies
8+
*/
9+
import { CustomSelectControl } from '@wordpress/components';
10+
11+
describe( 'CustomSelectControl', () => {
12+
it( 'Captures the keypress event and does not let it propagate', () => {
13+
const onKeyDown = jest.fn();
14+
const options = [
15+
{
16+
key: 'one',
17+
name: 'Option one',
18+
},
19+
{
20+
key: 'two',
21+
name: 'Option two',
22+
},
23+
{
24+
key: 'three',
25+
name: 'Option three',
26+
},
27+
];
28+
29+
render(
30+
<div
31+
// This role="none" is required to prevent an eslint warning about accessibility.
32+
role="none"
33+
onKeyDown={ onKeyDown }
34+
>
35+
<CustomSelectControl options={ options } />
36+
</div>
37+
);
38+
const toggleButton = screen.getByRole( 'button' );
39+
fireEvent.click( toggleButton );
40+
41+
const customSelect = screen.getByRole( 'listbox' );
42+
fireEvent.keyDown( customSelect );
43+
44+
expect( onKeyDown ).toHaveBeenCalledTimes( 0 );
45+
} );
46+
} );

0 commit comments

Comments
 (0)