Skip to content

Commit

Permalink
[fix] Pressable button click regression
Browse files Browse the repository at this point in the history
Update keyboard handling code to avoid calling preventDefault for
'keydown' events occuring on a native button element. This was causing
native 'click' event to be cancelled on buttons.

Fix #2560
  • Loading branch information
necolas committed Jul 19, 2023
1 parent 3babcc4 commit a8a0c17
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,35 @@ describe('components/Pressable', () => {
expect(container.firstChild).toMatchSnapshot();
});

test('press interaction as button (keyboard)', () => {
const onPress = jest.fn();
const preventDefault = jest.fn();
const ref = React.createRef();

function TestCase() {
return (
<Pressable
onPress={(e) => {
onPress(e);
}}
ref={ref}
role="button"
/>
);
}

act(() => {
render(<TestCase />);
});
const target = createEventTarget(ref.current);
act(() => {
target.keydown({ key: ' ', preventDefault });
jest.runAllTimers();
});
// Calling preventDefault prevents native 'click' event dispatch
expect(preventDefault).not.toHaveBeenCalled();
});

describe('prop "ref"', () => {
test('value is set', () => {
const ref = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,15 @@ const Transitions = Object.freeze({
}
});

const getElementRole = (element) => element.getAttribute('role');

const getElementType = (element) => element.tagName.toLowerCase();

const isActiveSignal = (signal) =>
signal === RESPONDER_ACTIVE_PRESS_START ||
signal === RESPONDER_ACTIVE_LONG_PRESS_START;

const isButtonRole = (element) => element.getAttribute('role') === 'button';
const isButtonRole = (element) => getElementRole(element) === 'button';

const isPressStartSignal = (signal) =>
signal === RESPONDER_INACTIVE_PRESS_START ||
Expand All @@ -132,10 +136,10 @@ const isTerminalSignal = (signal) =>

const isValidKeyPress = (event) => {
const { key, target } = event;
const role = target.getAttribute('role');
const isSpacebar = key === ' ' || key === 'Spacebar';

return key === 'Enter' || (isSpacebar && role === 'button');
const isButtonish =
getElementType(target) === 'button' || isButtonRole(target);
return key === 'Enter' || (isSpacebar && isButtonish);
};

const DEFAULT_LONG_PRESS_DELAY_MS = 450; // 500 - 50
Expand Down Expand Up @@ -307,7 +311,7 @@ export default class PressResponder {
document.removeEventListener('keyup', keyupHandler);

const role = target.getAttribute('role');
const elementType = target.tagName.toLowerCase();
const elementType = getElementType(target);

const isNativeInteractiveElement =
role === 'link' ||
Expand Down Expand Up @@ -345,11 +349,15 @@ export default class PressResponder {
// 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
const role = getElementRole(target);
const isButtonLikeRole = role === 'button' || role === 'menuitem';
if (
isSpacebarKey &&
isButtonLikeRole &&
getElementType(target) !== 'button'
) {
// Prevent spacebar scrolling the window if using non-native button
event.preventDefault();
}
event.stopPropagation();
Expand Down

0 comments on commit a8a0c17

Please sign in to comment.