[fix] Change logic of PressResponder for keyboards

This patch removes the keyboard `onClick` shim from `unstable_createElement`
and adds it to the `PressResponder`.

Previously, the logic in `createElement` was calling `onClick` during
`onKeyDown`. This could result in React state update warnings if the host
element was no longer in the DOM during `keyup`.

Fix #2051
This commit is contained in:
Nicolas Gallagher
2021-06-14 10:25:20 -07:00
parent 6d7a1eaf0e
commit 2a930b7e3f
5 changed files with 101 additions and 165 deletions
@@ -62,14 +62,14 @@ exports[`components/Pressable hover interaction 3`] = `
/>
`;
exports[`components/Pressable press interaction 1`] = `
exports[`components/Pressable press interaction (keyboard) 1`] = `
<div
class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
tabindex="0"
/>
`;
exports[`components/Pressable press interaction 2`] = `
exports[`components/Pressable press interaction (keyboard) 2`] = `
<div
class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
style="outline: press-ring;"
@@ -81,7 +81,28 @@ exports[`components/Pressable press interaction 2`] = `
</div>
`;
exports[`components/Pressable press interaction 3`] = `
exports[`components/Pressable press interaction (keyboard) 3`] = `null`;
exports[`components/Pressable press interaction (pointer) 1`] = `
<div
class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
tabindex="0"
/>
`;
exports[`components/Pressable press interaction (pointer) 2`] = `
<div
class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
style="outline: press-ring;"
tabindex="0"
>
<div
data-testid="press-content"
/>
</div>
`;
exports[`components/Pressable press interaction (pointer) 3`] = `
<div
class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
style=""
@@ -121,7 +121,7 @@ describe('components/Pressable', () => {
expect(container.firstChild).toMatchSnapshot();
});
test('press interaction', () => {
test('press interaction (pointer)', () => {
let container;
const onContextMenu = jest.fn();
const onPress = jest.fn();
@@ -162,6 +162,50 @@ describe('components/Pressable', () => {
expect(onContextMenu).toBeCalled();
});
test('press interaction (keyboard)', () => {
let container;
const onPress = jest.fn();
const onPressIn = jest.fn();
const onPressOut = jest.fn();
const ref = React.createRef();
function TestCase() {
const [shown, setShown] = React.useState(true);
return shown ? (
<Pressable
children={({ pressed }) => (pressed ? <div data-testid="press-content" /> : null)}
onPress={(e) => {
onPress(e);
setShown(false);
}}
onPressIn={onPressIn}
onPressOut={onPressOut}
ref={ref}
style={({ pressed }) => [pressed && { outline: 'press-ring' }]}
/>
) : null;
}
act(() => {
({ container } = render(<TestCase />));
});
const target = createEventTarget(ref.current);
expect(container.firstChild).toMatchSnapshot();
act(() => {
target.keydown({ key: 'Enter' });
jest.runAllTimers();
});
expect(onPressIn).toBeCalled();
expect(container.firstChild).toMatchSnapshot();
act(() => {
target.keyup({ key: 'Enter' });
jest.runAllTimers();
});
expect(onPressOut).toBeCalled();
expect(onPress).toBeCalled();
expect(container.firstChild).toMatchSnapshot();
});
describe('prop "ref"', () => {
test('value is set', () => {
const ref = jest.fn();
@@ -95,117 +95,6 @@ describe('modules/createDOMProps', () => {
});
});
describe('prop "onClick"', () => {
const callsOnClick = (component, accessibilityRole, accessibilityDisabled = false) => {
const onClick = jest.fn();
const event = { stopPropagation: jest.fn() };
const finalProps = createDOMProps(component, {
accessibilityRole,
accessibilityDisabled,
onClick
});
finalProps.onClick(event);
return onClick.mock.calls.length === 1;
};
test('is called for various roles', () => {
expect(callsOnClick('div', 'link')).toBe(true);
expect(callsOnClick('div', 'button')).toBe(true);
expect(callsOnClick('div', 'textbox')).toBe(true);
expect(callsOnClick('div', 'bogus')).toBe(true);
expect(callsOnClick('a')).toBe(true);
expect(callsOnClick('button')).toBe(true);
expect(callsOnClick('input')).toBe(true);
expect(callsOnClick('select')).toBe(true);
expect(callsOnClick('textarea')).toBe(true);
expect(callsOnClick('h1')).toBe(true);
});
test('is not called when disabled is true', () => {
expect(callsOnClick('div', 'link', true)).toBe(false);
expect(callsOnClick('div', 'button', true)).toBe(false);
expect(callsOnClick('a', undefined, true)).toBe(false);
expect(callsOnClick('button', undefined, true)).toBe(false);
expect(callsOnClick('input', undefined, true)).toBe(false);
expect(callsOnClick('select', undefined, true)).toBe(false);
expect(callsOnClick('textarea', undefined, true)).toBe(false);
expect(callsOnClick('div', 'textbox', true)).toBe(true);
expect(callsOnClick('div', 'bogus', true)).toBe(true);
expect(callsOnClick('h1', undefined, true)).toBe(true);
});
});
describe('prop "onKeyDown"', () => {
const callsOnClick = (key) => (component, accessibilityRole, accessibilityDisabled = false) => {
const onClick = jest.fn();
const onKeyDown = jest.fn();
const event = { key, preventDefault: jest.fn() };
const finalProps = createDOMProps(component, {
accessibilityRole,
accessibilityDisabled,
onClick,
onKeyDown
});
finalProps.onKeyDown(event);
// The original onKeyDown should always be called
expect(onKeyDown).toHaveBeenCalled();
return onClick.mock.calls.length === 1;
};
const respondsToEnter = callsOnClick('Enter');
const respondsToSpace = callsOnClick(' ');
test('does not emulate "onClick" when disabled', () => {
expect(respondsToEnter('div', 'link', true)).toBe(false);
expect(respondsToEnter('div', 'button', true)).toBe(false);
expect(respondsToEnter('div', 'textbox', true)).toBe(false);
expect(respondsToEnter('div', 'bogus', true)).toBe(false);
});
test('does not emulate "onClick" for native elements', () => {
expect(respondsToEnter('a')).toBe(false);
expect(respondsToEnter('button')).toBe(false);
expect(respondsToEnter('input')).toBe(false);
expect(respondsToEnter('select')).toBe(false);
expect(respondsToEnter('textarea')).toBe(false);
expect(respondsToEnter('h1')).toBe(false);
expect(respondsToEnter('div', 'link')).toBe(false);
expect(respondsToSpace('a')).toBe(false);
expect(respondsToSpace('button')).toBe(false);
expect(respondsToSpace('input')).toBe(false);
expect(respondsToSpace('select')).toBe(false);
expect(respondsToSpace('textarea')).toBe(false);
expect(respondsToSpace('h1')).toBe(false);
expect(respondsToSpace('div', 'link')).toBe(false);
});
test('emulates "onClick" for "Enter" for certain roles', () => {
expect(respondsToEnter('div', 'button')).toBe(true);
expect(respondsToEnter('div', 'textbox')).toBe(false);
expect(respondsToEnter('div', 'bogus')).toBe(false);
});
test('emulates "onClick" for "Enter" for items marked accessible', () => {
const onClick = jest.fn();
const event = { key: 'Enter', preventDefault: jest.fn() };
const finalProps = createDOMProps('div', {
accessibilityRole: 'article',
focusable: true,
onClick
});
finalProps.onKeyDown(event);
expect(onClick).toHaveBeenCalled();
});
test('emulates "onClick" for "Space" for certain roles', () => {
expect(respondsToSpace('div', 'button')).toBe(true);
expect(respondsToSpace('div', 'textbox')).toBe(false);
expect(respondsToSpace('div', 'bogus')).toBe(false);
});
});
test('prop "accessibilityLabel" becomes "aria-label"', () => {
const accessibilityLabel = 'accessibilityLabel';
const props = createProps({ accessibilityLabel });
@@ -136,15 +136,6 @@ const createDOMProps = (elementType, props) => {
const role = AccessibilityUtil.propsToAriaRole(props);
const isNativeInteractiveElement =
role === 'link' ||
elementType === 'a' ||
elementType === 'button' ||
elementType === 'input' ||
elementType === 'select' ||
elementType === 'textarea' ||
domProps.contentEditable != null;
// DEPRECATED
if (accessibilityState != null) {
for (const prop in accessibilityState) {
@@ -417,42 +408,6 @@ const createDOMProps = (elementType, props) => {
domProps['data-testid'] = testID;
}
// Keyboard accessibility
// Button-like roles should trigger 'onClick' if SPACE key is pressed.
// Button-like roles should not trigger 'onClick' if they are disabled.
if (isNativeInteractiveElement || role === 'button' || (_focusable === true && !disabled)) {
const onClick = domProps.onClick;
if (onClick != null) {
if (disabled) {
// Prevent click propagating if the element is disabled. See #1757
domProps.onClick = function (e) {
e.stopPropagation();
};
} else if (!isNativeInteractiveElement) {
// For native elements that are focusable but don't dispatch 'click' events
// for keyboards.
const onKeyDown = domProps.onKeyDown;
domProps.onKeyDown = function (e) {
const { key, repeat } = e;
const isSpacebarKey = key === ' ' || key === 'Spacebar';
const isButtonRole = role === 'button';
if (onKeyDown != null) {
onKeyDown(e);
}
if (!repeat && key === 'Enter') {
onClick(e);
} else if (isSpacebarKey && isButtonRole) {
if (!repeat) {
onClick(e);
}
// Prevent spacebar scrolling the window
e.preventDefault();
}
};
}
}
}
return domProps;
};
@@ -130,11 +130,11 @@ const isTerminalSignal = (signal) =>
signal === RESPONDER_TERMINATED || signal === RESPONDER_RELEASE;
const isValidKeyPress = (event) => {
const key = event.key;
const target = event.currentTarget;
const { key, target } = event;
const role = target.getAttribute('role');
const isSpacebar = key === ' ' || key === 'Spacebar';
return !event.repeat && (key === 'Enter' || (isSpacebar && role === 'button'));
return key === 'Enter' || (isSpacebar && role === 'button');
};
const DEFAULT_LONG_PRESS_DELAY_MS = 450; // 500 - 50
@@ -298,9 +298,27 @@ export default class PressResponder {
};
const keyupHandler = (event: KeyboardEvent) => {
if (this._touchState !== NOT_RESPONDER) {
const { onPress } = this._config;
const { target } = event;
if (this._touchState !== NOT_RESPONDER && isValidKeyPress(event)) {
end(event);
document.removeEventListener('keyup', keyupHandler);
const role = target.getAttribute('role');
const elementType = target.tagName.toLowerCase();
const isNativeInteractiveElement =
role === 'link' ||
elementType === 'a' ||
elementType === 'button' ||
elementType === 'input' ||
elementType === 'select' ||
elementType === 'textarea';
if (onPress != null && !isNativeInteractiveElement) {
onPress(event);
}
}
};
@@ -317,13 +335,22 @@ export default class PressResponder {
},
onKeyDown: (event) => {
if (isValidKeyPress(event)) {
const { disabled } = this._config;
const { key, target } = event;
if (!disabled && isValidKeyPress(event)) {
if (this._touchState === NOT_RESPONDER) {
start(event, false);
// Listen to 'keyup' on document to account for situations where
// focus is moved to another element during 'keydown'.
document.addEventListener('keyup', keyupHandler);
}
const role = target.getAttribute('role');
const isSpacebarKey = key === ' ' || key === 'Spacebar';
const isButtonRole = role === 'button' || role === 'menuitem';
if (isSpacebarKey && isButtonRole) {
// Prevent spacebar scrolling the window
event.preventDefault();
}
event.stopPropagation();
}
},