Skip to content

Commit cea4172

Browse files
comp615necolas
authored andcommitted
[fix] Prevent onClick being called when certain roles are disabled
Fix #1779 Close #1780
1 parent a2d72ee commit cea4172

File tree

3 files changed

+101
-33
lines changed

3 files changed

+101
-33
lines changed

packages/react-native-web/src/exports/createElement/__tests__/index-test.js

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -23,37 +23,5 @@ describe('exports/createElement', () => {
2323
const { container } = render(createElement(Custom, { accessibilityRole: 'link' }));
2424
expect(container.firstChild.nodeName).toBe('DIV');
2525
});
26-
27-
const testRole = ({ accessibilityRole, disabled }) => {
28-
[{ key: 'Enter' }, { key: ' ' }].forEach(({ key }) => {
29-
test(`"onClick" is ${disabled ? 'not ' : ''}called when "${key}" key is pressed`, () => {
30-
const onClick = jest.fn();
31-
const { container } = render(
32-
createElement('span', { accessibilityRole, disabled, onClick })
33-
);
34-
const event = document.createEvent('CustomEvent');
35-
event.initCustomEvent('keydown', true, true);
36-
event.key = key;
37-
container.firstChild.dispatchEvent(event);
38-
expect(onClick).toHaveBeenCalledTimes(disabled ? 0 : 1);
39-
});
40-
});
41-
};
42-
43-
describe('value is "button" and disabled is "true"', () => {
44-
testRole({ accessibilityRole: 'button', disabled: true });
45-
});
46-
47-
describe('value is "button" and disabled is "false"', () => {
48-
testRole({ accessibilityRole: 'button', disabled: false });
49-
});
50-
51-
describe('value is "menuitem" and disabled is "true"', () => {
52-
testRole({ accessibilityRole: 'menuitem', disabled: true });
53-
});
54-
55-
describe('value is "menuitem" and disabled is "false"', () => {
56-
testRole({ accessibilityRole: 'menuitem', disabled: false });
57-
});
5826
});
5927
});

packages/react-native-web/src/modules/createDOMProps/__tests__/index-test.js

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,106 @@ describe('modules/createDOMProps', () => {
140140
});
141141
});
142142

143+
describe('prop "onClick"', () => {
144+
const callsOnClick = (component, accessibilityRole, disabled = false) => {
145+
const onClick = jest.fn();
146+
const event = { stopPropagation: jest.fn() };
147+
const finalProps = createDOMProps(component, { accessibilityRole, disabled, onClick });
148+
finalProps.onClick(event);
149+
return onClick.mock.calls.length === 1;
150+
};
151+
152+
test('is called for various roles', () => {
153+
expect(callsOnClick('div', 'link')).toBe(true);
154+
expect(callsOnClick('div', 'button')).toBe(true);
155+
expect(callsOnClick('div', 'textbox')).toBe(true);
156+
expect(callsOnClick('div', 'menuitem')).toBe(true);
157+
expect(callsOnClick('div', 'bogus')).toBe(true);
158+
expect(callsOnClick('a')).toBe(true);
159+
expect(callsOnClick('button')).toBe(true);
160+
expect(callsOnClick('input')).toBe(true);
161+
expect(callsOnClick('select')).toBe(true);
162+
expect(callsOnClick('textarea')).toBe(true);
163+
expect(callsOnClick('h1')).toBe(true);
164+
});
165+
166+
test('is not called when disabled is true', () => {
167+
expect(callsOnClick('div', 'link', true)).toBe(false);
168+
expect(callsOnClick('div', 'button', true)).toBe(false);
169+
expect(callsOnClick('div', 'menuitem', true)).toBe(false);
170+
expect(callsOnClick('a', undefined, true)).toBe(false);
171+
expect(callsOnClick('button', undefined, true)).toBe(false);
172+
expect(callsOnClick('input', undefined, true)).toBe(false);
173+
expect(callsOnClick('select', undefined, true)).toBe(false);
174+
expect(callsOnClick('textarea', undefined, true)).toBe(false);
175+
176+
expect(callsOnClick('div', 'textbox', true)).toBe(true);
177+
expect(callsOnClick('div', 'bogus', true)).toBe(true);
178+
expect(callsOnClick('h1', undefined, true)).toBe(true);
179+
});
180+
});
181+
182+
describe('prop "onKeyDown"', () => {
183+
const callsOnClick = key => (component, accessibilityRole, disabled = false) => {
184+
const onClick = jest.fn();
185+
const onKeyDown = jest.fn();
186+
const event = { key, preventDefault: jest.fn() };
187+
const finalProps = createDOMProps(component, {
188+
accessibilityRole,
189+
disabled,
190+
onClick,
191+
onKeyDown
192+
});
193+
finalProps.onKeyDown(event);
194+
// The original onKeyDown should always be called
195+
expect(onKeyDown).toHaveBeenCalled();
196+
return onClick.mock.calls.length === 1;
197+
};
198+
199+
const respondsToEnter = callsOnClick('Enter');
200+
const respondsToSpace = callsOnClick(' ');
201+
202+
test('does not emulate "onClick" when disabled', () => {
203+
expect(respondsToEnter('div', 'link', true)).toBe(false);
204+
expect(respondsToEnter('div', 'button', true)).toBe(false);
205+
expect(respondsToEnter('div', 'textbox', true)).toBe(false);
206+
expect(respondsToEnter('div', 'menuitem', true)).toBe(false);
207+
expect(respondsToEnter('div', 'bogus', true)).toBe(false);
208+
});
209+
210+
test('does not emulate "onClick" for native elements', () => {
211+
expect(respondsToEnter('a')).toBe(false);
212+
expect(respondsToEnter('button')).toBe(false);
213+
expect(respondsToEnter('input')).toBe(false);
214+
expect(respondsToEnter('select')).toBe(false);
215+
expect(respondsToEnter('textarea')).toBe(false);
216+
expect(respondsToEnter('h1')).toBe(false);
217+
expect(respondsToEnter('div', 'link')).toBe(false);
218+
219+
expect(respondsToSpace('a')).toBe(false);
220+
expect(respondsToSpace('button')).toBe(false);
221+
expect(respondsToSpace('input')).toBe(false);
222+
expect(respondsToSpace('select')).toBe(false);
223+
expect(respondsToSpace('textarea')).toBe(false);
224+
expect(respondsToSpace('h1')).toBe(false);
225+
expect(respondsToSpace('div', 'link')).toBe(false);
226+
});
227+
228+
test('emulates "onClick" for "Enter" for certain roles', () => {
229+
expect(respondsToEnter('div', 'button')).toBe(true);
230+
expect(respondsToEnter('div', 'menuitem')).toBe(true);
231+
expect(respondsToEnter('div', 'textbox')).toBe(false);
232+
expect(respondsToEnter('div', 'bogus')).toBe(false);
233+
});
234+
235+
test('emulates "onClick" for "Space" for certain roles', () => {
236+
expect(respondsToSpace('div', 'button')).toBe(true);
237+
expect(respondsToSpace('div', 'menuitem')).toBe(true);
238+
expect(respondsToSpace('div', 'textbox')).toBe(false);
239+
expect(respondsToSpace('div', 'bogus')).toBe(false);
240+
});
241+
});
242+
143243
test('prop "accessibilityLabel" becomes "aria-label"', () => {
144244
const accessibilityLabel = 'accessibilityLabel';
145245
const props = createProps({ accessibilityLabel });

packages/react-native-web/src/modules/createDOMProps/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ const createDOMProps = (component, props) => {
225225
// Keyboard accessibility
226226
// Button-like roles should trigger 'onClick' if SPACE key is pressed.
227227
// Button-like roles should not trigger 'onClick' if they are disabled.
228-
if (domProps['data-focusable']) {
228+
if (isNativeInteractiveElement || role === 'button' || role === 'menuitem') {
229229
const onClick = domProps.onClick;
230230
if (onClick != null) {
231231
if (disabled) {

0 commit comments

Comments
 (0)