Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: change all onKeyPress uses to use onKeyDown instead #927

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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