Skip to content

Commit

Permalink
refactor: change all onKeyPress uses to use onKeyDown instead (#927)
Browse files Browse the repository at this point in the history
  • Loading branch information
derekhassan authored Feb 9, 2023
1 parent 7b048dd commit d65b947
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 26 deletions.
14 changes: 7 additions & 7 deletions packages/card/components/ButtonCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,27 +40,27 @@ const ButtonCard = ({
hasFocus,
onClick,
className,
onKeyPress,
onKeyDown,
"data-cy": dataCy = "buttonCard",
...other
}: ButtonCardProps) => {
const tabIndex = disabled ? -1 : 0;
// mimic native <button> keyboard behavior without using a <button>
const keyPressClick = e => {
const keyDownClick = e => {
if (onClick && (e.key === " " || e.key === "Enter")) {
onClick(e);
}
};
const handleKeyPress = e => {
keyPressClick(e);
if (onKeyPress) {
onKeyPress(e);
const handleKeyDown = e => {
keyDownClick(e);
if (onKeyDown) {
onKeyDown(e);
}
};
const buttonProps = !isInput
? {
tabIndex,
onKeyPress: handleKeyPress,
onKeyDown: handleKeyDown,
onClick,
role: "button",
"aria-disabled": disabled,
Expand Down
2 changes: 1 addition & 1 deletion packages/clickable/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

The Clickable is a higher order presentational component, children get the necessary a11y attributes added.

Use this whenever you want to bind an action on click onto a HTML element. The component will add the appropiate role attribute, add a tabIndex, add a focus state and bind the action to `onClick` and `onKeyPress`. The `onKeyPress` has a wrapping function which filters out eveerything except the space key and enter key events. This prevents that the function gets triggered by accident.
Use this whenever you want to bind an action on click onto a HTML element. The component will add the appropriate role attribute, add a tabIndex, add a focus state and bind the action to `onClick` and `onKeyDown`. The `onKeyDown` has a wrapping function which filters out everything except the space key and enter key events. This prevents that the function gets triggered by accident.
6 changes: 3 additions & 3 deletions packages/clickable/components/clickable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export interface ClickableProps {
*/
children: React.ReactElement<HTMLElement> & React.ReactNode;
/**
* Action is a event handler for the onClick and onKeypress events
* Action is a event handler for the onClick and onKeyDown events
*/
action?: (event?: React.SyntheticEvent<HTMLElement>) => void;
/**
Expand Down Expand Up @@ -39,7 +39,7 @@ export const Clickable = ({
}: ClickableProps) => {
const { className = "" } = children.props;

const handleKeyPress = (event: React.KeyboardEvent<HTMLElement>): void => {
const handleKeyDown = (event: React.KeyboardEvent<HTMLElement>): void => {
// action can be undefined from components SidebarItem and SidebarSubMenuItem
if (action && (event.key === " " || event.key === "Enter")) {
action(event);
Expand All @@ -51,7 +51,7 @@ export const Clickable = ({
className: cx(className, pointer, { [outline]: disableFocusOutline }),
role,
tabIndex,
onKeyPress: handleKeyPress,
onKeyDown: handleKeyDown,
"data-cy": dataCy
});
};
Expand Down
14 changes: 7 additions & 7 deletions packages/clickable/tests/clickable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ describe("Clickable", () => {
expect(action).toHaveBeenCalled();
});

it("has onKeyPress function and reacts on space", async () => {
it("has onKeyDown function and reacts on space", async () => {
const action = jest.fn();
const { getByRole } = render(
<Clickable action={action}>
<span>onKeyPress</span>
<span>onKeyDown</span>
</Clickable>
);

Expand All @@ -46,11 +46,11 @@ describe("Clickable", () => {
expect(action).toHaveBeenCalled();
});

it("has onKeyPress function and reacts on Enter", async () => {
it("has onKeyDown function and reacts on Enter", async () => {
const action = jest.fn();
const { getByRole } = render(
<Clickable action={action}>
<span>onKeyPress</span>
<span>onKeyDown</span>
</Clickable>
);

Expand All @@ -60,11 +60,11 @@ describe("Clickable", () => {
await userEvent.keyboard("[Enter]");
expect(action).toHaveBeenCalled();
});
it("does not react on e keypress", async () => {
it("does not react on e keydown", async () => {
const action = jest.fn();
const { getByRole } = render(
<Clickable action={action}>
<span>onKeyPress</span>
<span>onKeyDown</span>
</Clickable>
);

Expand All @@ -79,7 +79,7 @@ describe("Clickable", () => {
const action = jest.fn();
const { getByRole } = render(
<Clickable action={action}>
<span>onKeyPress</span>
<span>onKeyDown</span>
</Clickable>
);

Expand Down
8 changes: 0 additions & 8 deletions packages/textInput/components/TextInputWithBadges.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,6 @@ const TextInputWithBadges = (props: TextInputWithBadgesProps) => {
inputProps.onKeyDown = handleKeyDown;
inputProps.onKeyUp = handleKeyUp;
inputProps.onFocus = inputOnFocus;
inputProps.onKeyPress = e => {
if (props.onKeyPress) {
props.onKeyPress(e);
}
if (e.key === "Enter") {
e.preventDefault();
}
};
inputProps.onBlur = handleBlur;
inputProps.type = "text";
inputProps.ref = inputRef;
Expand Down

0 comments on commit d65b947

Please sign in to comment.