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

FEI-5533: Re-enable select keyboard tests for Dropdown and Clickable #2420

Merged
merged 9 commits into from
Jan 21, 2025
7 changes: 7 additions & 0 deletions .changeset/mean-cherries-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/wonder-blocks-clickable": major
"@khanacademy/wonder-blocks-dropdown": major
"@khanacademy/wonder-blocks-core": major
---

Fixes keyboard tests in Dropdown and Clickable with specific key events. We now check `event.key` instead of `event.which` or `event.keyCode` to remove deprecated event properties and match the keys returned from Testing Library/userEvent.
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,29 @@ export const UsingAriaAttributes = {
render: SingleSelectAccessibility.bind({}),
name: "Using aria attributes",
};

// This story exists for debugging automated unit tests.
const SingleSelectKeyboardSelection = () => {
const [selectedValue, setSelectedValue] = React.useState("");
return (
<View>
<SingleSelect
placeholder="Choose"
onChange={setSelectedValue}
selectedValue={selectedValue}
>
<OptionItem label="apple" value="apple" />
<OptionItem label="orange" value="orange" />
<OptionItem label="pear" value="pear" />
</SingleSelect>
</View>
);
};

export const UsingKeyboardSelection = {
marcysutton marked this conversation as resolved.
Show resolved Hide resolved
render: SingleSelectKeyboardSelection.bind({}),
name: "Using the keyboard",
parameters: {
chromatic: {disableSnapshot: true},
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@ import * as React from "react";
import {render, screen, fireEvent, waitFor} from "@testing-library/react";
import {MemoryRouter, Switch, Route} from "react-router-dom";
import {userEvent} from "@testing-library/user-event";
import {keys} from "@khanacademy/wonder-blocks-core";

import getClickableBehavior from "../../util/get-clickable-behavior";
import ClickableBehavior from "../clickable-behavior";
import type {ClickableState} from "../clickable-behavior";

const keyCodes = {
tab: 9,
enter: 13,
space: 32,
} as const;

const labelForState = (state: ClickableState): string => {
const labels: Array<any> = [];
if (state.hovered) {
Expand Down Expand Up @@ -195,8 +190,8 @@ describe("ClickableBehavior", () => {
);
const button = await screen.findByRole("button");
expect(button).not.toHaveTextContent("focused");
fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyUp(button, {keyCode: keyCodes.space});
fireEvent.keyDown(button, {key: keys.space});
fireEvent.keyUp(button, {key: keys.space});
// NOTE(kevinb): await userEvent.click() fires other events that we don't want
// affecting this test case.
fireEvent.click(button);
Expand All @@ -219,8 +214,8 @@ describe("ClickableBehavior", () => {
);
const button = await screen.findByRole("button");
expect(button).not.toHaveTextContent("focused");
fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyUp(button, {keyCode: keyCodes.space});
await userEvent.tab();
await userEvent.keyboard(" ");
// NOTE(kevinb): await userEvent.click() fires other events that we don't want
// affecting this test case.
fireEvent.click(button);
Expand All @@ -244,14 +239,14 @@ describe("ClickableBehavior", () => {
);
const button = await screen.findByRole("button");
expect(button).not.toHaveTextContent("pressed");
fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyDown(button, {key: keys.space});
expect(button).toHaveTextContent("pressed");
fireEvent.keyUp(button, {keyCode: keyCodes.space});
fireEvent.keyUp(button, {key: keys.space});
expect(button).not.toHaveTextContent("pressed");

fireEvent.keyDown(button, {keyCode: keyCodes.enter});
fireEvent.keyDown(button, {key: keys.enter});
expect(button).toHaveTextContent("pressed");
fireEvent.keyUp(button, {keyCode: keyCodes.enter});
fireEvent.keyUp(button, {key: keys.enter});
expect(button).not.toHaveTextContent("pressed");
});

Expand Down Expand Up @@ -280,14 +275,14 @@ describe("ClickableBehavior", () => {
);
const link = await screen.findByRole("link");
expect(link).not.toHaveTextContent("pressed");
fireEvent.keyDown(link, {keyCode: keyCodes.enter});
fireEvent.keyDown(link, {key: keys.enter});
expect(link).toHaveTextContent("pressed");
fireEvent.keyUp(link, {keyCode: keyCodes.enter});
fireEvent.keyUp(link, {key: keys.enter});
expect(link).not.toHaveTextContent("pressed");

fireEvent.keyDown(link, {keyCode: keyCodes.space});
fireEvent.keyDown(link, {key: keys.space});
expect(link).not.toHaveTextContent("pressed");
fireEvent.keyUp(link, {keyCode: keyCodes.space});
fireEvent.keyUp(link, {key: keys.space});
expect(link).not.toHaveTextContent("pressed");
});

Expand Down Expand Up @@ -462,19 +457,19 @@ describe("ClickableBehavior", () => {

expect(button).not.toHaveTextContent("focused");
fireEvent.keyUp(button, {
keyCode: keyCodes.tab,
key: keys.tab,
});
expect(button).not.toHaveTextContent("focused");
fireEvent.keyDown(button, {keyCode: keyCodes.tab});
fireEvent.keyDown(button, {key: keys.tab});
expect(button).not.toHaveTextContent("focused");

expect(button).not.toHaveTextContent("pressed");
fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyDown(button, {key: keys.space});
expect(button).not.toHaveTextContent("pressed");
fireEvent.keyUp(button, {keyCode: keyCodes.space});
fireEvent.keyUp(button, {key: keys.space});
expect(button).not.toHaveTextContent("pressed");

fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyDown(button, {key: keys.space});
fireEvent.blur(button);
expect(button).not.toHaveTextContent("pressed");

Expand All @@ -501,9 +496,9 @@ describe("ClickableBehavior", () => {

const anchor = await screen.findByRole("link");
expect(anchor).not.toHaveTextContent("pressed");
fireEvent.keyDown(anchor, {keyCode: keyCodes.enter});
fireEvent.keyDown(anchor, {key: keys.enter});
expect(anchor).not.toHaveTextContent("pressed");
fireEvent.keyUp(anchor, {keyCode: keyCodes.enter});
fireEvent.keyUp(anchor, {key: keys.enter});
expect(anchor).not.toHaveTextContent("pressed");
});

Expand All @@ -525,16 +520,16 @@ describe("ClickableBehavior", () => {
await userEvent.click(button);
expect(onClick).toHaveBeenCalledTimes(1);

fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyUp(button, {keyCode: keyCodes.space});
fireEvent.keyDown(button, {key: keys.space});
fireEvent.keyUp(button, {key: keys.space});
expect(onClick).toHaveBeenCalledTimes(2);

fireEvent.keyDown(button, {keyCode: keyCodes.enter});
fireEvent.keyUp(button, {keyCode: keyCodes.enter});
fireEvent.keyDown(button, {key: keys.enter});
fireEvent.keyUp(button, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(3);

fireEvent.touchStart(button, {keyCode: keyCodes.space});
fireEvent.touchEnd(button, {keyCode: keyCodes.space});
fireEvent.touchStart(button, {key: keys.space});
fireEvent.touchEnd(button, {key: keys.space});
fireEvent.click(button);
expect(onClick).toHaveBeenCalledTimes(4);
});
Expand Down Expand Up @@ -600,21 +595,21 @@ describe("ClickableBehavior", () => {
const link = await screen.findByRole("link");

// Space press should not trigger the onClick
fireEvent.keyDown(link, {keyCode: keyCodes.space});
fireEvent.keyUp(link, {keyCode: keyCodes.space});
fireEvent.keyDown(link, {key: keys.space});
fireEvent.keyUp(link, {key: keys.space});
expect(onClick).toHaveBeenCalledTimes(0);

// Navigation didn't happen with space
expect(window.location.assign).toHaveBeenCalledTimes(0);

// Enter press should trigger the onClick after keyup
fireEvent.keyDown(link, {keyCode: keyCodes.enter});
fireEvent.keyDown(link, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(0);

// Navigation doesn't happen until after enter is released
expect(window.location.assign).toHaveBeenCalledTimes(0);

fireEvent.keyUp(link, {keyCode: keyCodes.enter});
fireEvent.keyUp(link, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(1);

// Navigation happened after enter click
Expand Down Expand Up @@ -730,14 +725,14 @@ describe("ClickableBehavior", () => {

// Enter press should not do anything
const checkbox = await screen.findByRole("checkbox");
fireEvent.keyDown(checkbox, {keyCode: keyCodes.enter});
fireEvent.keyDown(checkbox, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(0);
fireEvent.keyUp(checkbox, {keyCode: keyCodes.enter});
fireEvent.keyUp(checkbox, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(0);

// Space press should trigger the onClick
fireEvent.keyDown(checkbox, {keyCode: keyCodes.space});
fireEvent.keyUp(checkbox, {keyCode: keyCodes.space});
fireEvent.keyDown(checkbox, {key: keys.space});
fireEvent.keyUp(checkbox, {key: keys.space});
expect(onClick).toHaveBeenCalledTimes(1);
});

Expand All @@ -757,15 +752,15 @@ describe("ClickableBehavior", () => {

// Enter press
const button = await screen.findByRole("button");
fireEvent.keyDown(button, {keyCode: keyCodes.enter});
fireEvent.keyDown(button, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(0);
fireEvent.keyUp(button, {keyCode: keyCodes.enter});
fireEvent.keyUp(button, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(1);

// Space press
fireEvent.keyDown(button, {keyCode: keyCodes.space});
fireEvent.keyDown(button, {key: keys.space});
expect(onClick).toHaveBeenCalledTimes(1);
fireEvent.keyUp(button, {keyCode: keyCodes.space});
fireEvent.keyUp(button, {key: keys.space});
expect(onClick).toHaveBeenCalledTimes(2);
});

Expand Down Expand Up @@ -794,10 +789,10 @@ describe("ClickableBehavior", () => {
}

// Enter press on a div
fireEvent.keyDown(clickableDiv, {keyCode: keyCodes.enter});
fireEvent.keyDown(clickableDiv, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(expectedNumberTimesCalled);
fireEvent.keyUp(clickableDiv, {
keyCode: keyCodes.enter,
key: keys.enter,
});
expectedNumberTimesCalled += 1;
expect(onClick).toHaveBeenCalledTimes(expectedNumberTimesCalled);
Expand All @@ -808,9 +803,9 @@ describe("ClickableBehavior", () => {
expect(onClick).toHaveBeenCalledTimes(expectedNumberTimesCalled);

// Space press on a div
fireEvent.keyDown(clickableDiv, {keyCode: keyCodes.space});
fireEvent.keyDown(clickableDiv, {key: keys.space});
expect(onClick).toHaveBeenCalledTimes(expectedNumberTimesCalled);
fireEvent.keyUp(clickableDiv, {keyCode: keyCodes.space});
fireEvent.keyUp(clickableDiv, {key: keys.space});
expectedNumberTimesCalled += 1;
expect(onClick).toHaveBeenCalledTimes(expectedNumberTimesCalled);

Expand Down Expand Up @@ -885,10 +880,10 @@ describe("ClickableBehavior", () => {

const checkbox = await screen.findByRole("checkbox");
// Enter press should not do anything
fireEvent.keyDown(checkbox, {keyCode: keyCodes.enter});
fireEvent.keyDown(checkbox, {key: keys.enter});
// This element still wants to have a click on enter press
fireEvent.click(checkbox);
fireEvent.keyUp(checkbox, {keyCode: keyCodes.enter});
fireEvent.keyUp(checkbox, {key: keys.enter});
expect(onClick).toHaveBeenCalledTimes(0);
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as React from "react";
import {keys} from "@khanacademy/wonder-blocks-core";

// NOTE: Potentially add to this as more cases come up.
export type ClickableRole =
Expand Down Expand Up @@ -229,11 +230,6 @@ const disabledHandlers = {
onKeyUp: () => void 0,
} as const;

const keyCodes = {
enter: 13,
space: 32,
} as const;

const startState: ClickableState = {
hovered: false,
focused: false,
Expand Down Expand Up @@ -560,21 +556,20 @@ export default class ClickableBehavior extends React.Component<
if (onKeyDown) {
onKeyDown(e);
}

const keyCode = e.which || e.keyCode;
const keyName = e.key;
const {triggerOnEnter, triggerOnSpace} =
getAppropriateTriggersForRole(role);
if (
(triggerOnEnter && keyCode === keyCodes.enter) ||
(triggerOnSpace && keyCode === keyCodes.space)
(triggerOnEnter && keyName === keys.enter) ||
(triggerOnSpace && keyName === keys.space)
) {
// This prevents space from scrolling down. It also prevents the
// space and enter keys from triggering click events. We manually
// call the supplied onClick and handle potential navigation in
// handleKeyUp instead.
e.preventDefault();
this.setState({pressed: true});
} else if (!triggerOnEnter && keyCode === keyCodes.enter) {
} else if (!triggerOnEnter && keyName === keys.enter) {
// If the component isn't supposed to trigger on enter, we have to
// keep track of the enter keydown to negate the onClick callback
this.enterClick = true;
Expand All @@ -587,17 +582,17 @@ export default class ClickableBehavior extends React.Component<
onKeyUp(e);
}

const keyCode = e.which || e.keyCode;
const keyName = e.key;
const {triggerOnEnter, triggerOnSpace} =
getAppropriateTriggersForRole(role);
if (
(triggerOnEnter && keyCode === keyCodes.enter) ||
(triggerOnSpace && keyCode === keyCodes.space)
(triggerOnEnter && keyName === keys.enter) ||
(triggerOnSpace && keyName === keys.space)
) {
this.setState({pressed: false, focused: true});

this.runCallbackAndMaybeNavigate(e);
} else if (!triggerOnEnter && keyCode === keyCodes.enter) {
} else if (!triggerOnEnter && keyName === keys.enter) {
this.enterClick = false;
}
};
Expand Down
1 change: 1 addition & 0 deletions packages/wonder-blocks-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export {RenderStateRoot} from "./components/render-state-root";
export {RenderState} from "./components/render-state-context";
export type {AriaRole, AriaAttributes} from "./util/aria-types";
export type {AriaProps, StyleType, PropsFor} from "./util/types";
export {keys} from "./util/keys";
12 changes: 12 additions & 0 deletions packages/wonder-blocks-core/src/util/keys.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Key value mapping reference:
* https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values
*/
export const keys = {
enter: "Enter",
escape: "Escape",
tab: "Tab",
space: " ",
up: "ArrowUp",
down: "ArrowDown",
} as const;
Loading
Loading