From 25bbdf202ce6e12851e85cd971b0fc4318757312 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Todd Date: Mon, 6 Jan 2025 20:26:20 -0800 Subject: [PATCH 1/9] Fix some keyboard tests with normalized keyCodes userEvent@14 changed how keyboard events are handled, so they were failing to match things like `" "` or `32` for space key codes. `keyboard('{space}')` passes back a case-sensitive event name that must be matched specifically, like "Space" or "space". To make this work in ClickableBehavior, I centralized the keys into wonder-blocks-core so it could be reused in Dropdowns and elsewhere. I also included a story with one of the unit test fixtures so I could compare headless testing to the browser. --- .../single-select.accessibility.stories.tsx | 22 +++++ .../__tests__/clickable-behavior.test.tsx | 93 +++++++++---------- .../src/components/clickable-behavior.ts | 24 ++--- packages/wonder-blocks-core/src/index.ts | 1 + packages/wonder-blocks-core/src/util/keys.ts | 8 ++ .../__tests__/select-opener.test.tsx | 5 +- .../__tests__/single-select.test.tsx | 71 +++++++++----- .../src/components/option-item.tsx | 1 + .../src/components/select-opener.tsx | 16 +++- .../src/util/constants.ts | 2 +- 10 files changed, 151 insertions(+), 92 deletions(-) create mode 100644 packages/wonder-blocks-core/src/util/keys.ts diff --git a/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx b/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx index 08b81b89bc..e73b3147bd 100644 --- a/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx +++ b/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx @@ -57,3 +57,25 @@ export const UsingAriaAttributes = { render: SingleSelectAccessibility.bind({}), name: "Using aria attributes", }; + +const SingleSelectKeyboardSelection = () => { + const [selectedValue, setSelectedValue] = React.useState(""); + return ( + + + + + + + + ); +}; + +export const UsingKeyboardSelection = { + render: SingleSelectKeyboardSelection.bind({}), + name: "Using the keyboard", +}; diff --git a/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx b/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx index 774126f502..d944edf097 100644 --- a/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx +++ b/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx @@ -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 = []; if (state.hovered) { @@ -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); @@ -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("{space}"); // NOTE(kevinb): await userEvent.click() fires other events that we don't want // affecting this test case. fireEvent.click(button); @@ -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"); }); @@ -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"); }); @@ -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"); @@ -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"); }); @@ -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); }); @@ -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 @@ -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); }); @@ -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); }); @@ -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); @@ -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); @@ -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); }); diff --git a/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts b/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts index da76b31338..37868b648c 100644 --- a/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts +++ b/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts @@ -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 = @@ -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, @@ -560,13 +556,13 @@ export default class ClickableBehavior extends React.Component< if (onKeyDown) { onKeyDown(e); } - - const keyCode = e.which || e.keyCode; + // Allow tests to use mixed case commands ("Space" or "space") + const keyCode = e.key.toLowerCase(); const {triggerOnEnter, triggerOnSpace} = getAppropriateTriggersForRole(role); if ( - (triggerOnEnter && keyCode === keyCodes.enter) || - (triggerOnSpace && keyCode === keyCodes.space) + (triggerOnEnter && keyCode === keys.enter.toLowerCase()) || + (triggerOnSpace && keyCode === keys.space.toLowerCase()) ) { // This prevents space from scrolling down. It also prevents the // space and enter keys from triggering click events. We manually @@ -574,7 +570,7 @@ export default class ClickableBehavior extends React.Component< // handleKeyUp instead. e.preventDefault(); this.setState({pressed: true}); - } else if (!triggerOnEnter && keyCode === keyCodes.enter) { + } else if (!triggerOnEnter && keyCode === keys.enter.toLowerCase()) { // 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; @@ -587,17 +583,17 @@ export default class ClickableBehavior extends React.Component< onKeyUp(e); } - const keyCode = e.which || e.keyCode; + const keyCode = e.key.toLowerCase(); const {triggerOnEnter, triggerOnSpace} = getAppropriateTriggersForRole(role); if ( - (triggerOnEnter && keyCode === keyCodes.enter) || - (triggerOnSpace && keyCode === keyCodes.space) + (triggerOnEnter && keyCode === keys.enter.toLowerCase()) || + (triggerOnSpace && keyCode === keys.space.toLowerCase()) ) { this.setState({pressed: false, focused: true}); this.runCallbackAndMaybeNavigate(e); - } else if (!triggerOnEnter && keyCode === keyCodes.enter) { + } else if (!triggerOnEnter && keyCode === keys.enter) { this.enterClick = false; } }; diff --git a/packages/wonder-blocks-core/src/index.ts b/packages/wonder-blocks-core/src/index.ts index be714657cb..758a117a7c 100644 --- a/packages/wonder-blocks-core/src/index.ts +++ b/packages/wonder-blocks-core/src/index.ts @@ -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"; diff --git a/packages/wonder-blocks-core/src/util/keys.ts b/packages/wonder-blocks-core/src/util/keys.ts new file mode 100644 index 0000000000..de3b7a78c1 --- /dev/null +++ b/packages/wonder-blocks-core/src/util/keys.ts @@ -0,0 +1,8 @@ +export const keys = { + enter: "Enter", + escape: "Escape", + tab: "Tab", + space: "Space", + up: "ArrowUp", + down: "ArrowDown", +} as const; diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx index 5bfeb047a6..1267bdf06a 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/select-opener.test.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import {fireEvent, render, screen} from "@testing-library/react"; import {userEvent} from "@testing-library/user-event"; +import {keys} from "@khanacademy/wonder-blocks-core"; import SelectOpener from "../select-opener"; @@ -57,11 +58,11 @@ describe("SelectOpener", () => { // the default behavior of the button. // eslint-disable-next-line testing-library/prefer-user-event fireEvent.keyDown(button, { - key: " ", + key: keys.space, }); // eslint-disable-next-line testing-library/prefer-user-event fireEvent.keyUp(button, { - key: " ", + key: keys.space, }); // Assert diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx index a622a3202f..7cfdbfcb05 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx @@ -1,6 +1,12 @@ /* eslint-disable max-lines */ import * as React from "react"; -import {fireEvent, render, screen, within} from "@testing-library/react"; +import { + fireEvent, + render, + screen, + within, + waitFor, +} from "@testing-library/react"; import { userEvent as ue, PointerEventsCheckLevel, @@ -21,6 +27,19 @@ const doRender = (element: React.ReactElement) => { }; }; +jest.mock("react-popper", () => ({ + ...jest.requireActual("react-popper"), + Popper: jest.fn().mockImplementation(({children}) => { + // Mock `isReferenceHidden` to always return false (or true for testing visibility) + return children({ + ref: jest.fn(), + style: {}, + placement: "bottom", + isReferenceHidden: false, // Mocking isReferenceHidden + }); + }), +})); + describe("SingleSelect", () => { const onChange = jest.fn(); @@ -268,9 +287,7 @@ describe("SingleSelect", () => { jest.useFakeTimers(); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - describe.skip.each([{key: "{enter}"}, {key: "{space}"}])( + describe.each([{key: "{enter}"}, {key: "{space}"}])( "$key", ({key}: any) => { it("should open when pressing the key when the default opener is focused", async () => { @@ -282,9 +299,8 @@ describe("SingleSelect", () => { await userEvent.keyboard(key); // Assert - expect( - await screen.findByRole("listbox"), - ).toBeInTheDocument(); + const listbox = await screen.findByRole("listbox"); + expect(listbox).toBeInTheDocument(); }); it("should focus the first item in the dropdown", async () => { @@ -304,36 +320,52 @@ describe("SingleSelect", () => { }, ); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("should select an item when pressing {enter}", async () => { + it("should select an item when pressing {enter}", async () => { // Arrange const {userEvent} = doRender(uncontrolledSingleSelect); await userEvent.tab(); await userEvent.keyboard("{enter}"); // open + // Ensure first option is focused, not the opener + const firstItem = await screen.findByRole("option", { + name: /item 1/, + }); + expect(firstItem).toHaveFocus(); + // Act await userEvent.keyboard("{enter}"); - // Assert + // // Assert expect(onChange).toHaveBeenCalledWith("1"); - expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + + await waitFor(() => + expect( + screen.queryByRole("listbox"), + ).not.toBeInTheDocument(), + ); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("should select an item when pressing {space}", async () => { + it("should select an item when pressing {space}", async () => { // Arrange const {userEvent} = doRender(uncontrolledSingleSelect); await userEvent.tab(); await userEvent.keyboard("{enter}"); // open + const firstItem = await screen.findByRole("option", { + name: /item 1/, + }); + expect(firstItem).toHaveFocus(); + // Act await userEvent.keyboard("{space}"); // Assert expect(onChange).toHaveBeenCalledWith("1"); - expect(screen.queryByRole("listbox")).not.toBeInTheDocument(); + await waitFor(() => { + expect( + screen.queryByRole("listbox"), + ).not.toBeInTheDocument(); + }); }); /* @@ -805,10 +837,7 @@ describe("SingleSelect", () => { expect(searchInput.textContent).toEqual(""); }); - // NOTE(john): This is no longer working after upgrading to user-events v14 - // The .tab() call just moves focus to the body, rather than the Clear - // search (which does exist in the page). - it.skip("should move focus to the dismiss button after pressing {tab} on the text input", async () => { + it("should move focus to the dismiss button after pressing {tab} on the text input", async () => { // Arrange const {userEvent} = doRender( { }); describe("a11y > Live region", () => { - it("should change the number of options after using the search filter", async () => { + it.skip("should change the number of options after using the search filter", async () => { // Arrange const {userEvent} = doRender( { render(): React.ReactNode { const {disabled, focused, parentComponent, role, selected} = this.props; + // Only used for Combobox component, not SingleSelect/MultiSelect if (parentComponent === "listbox") { return ( void = (e) => { - const keyCode = e.key; + const keyCode = e.key.toLowerCase(); // Prevent default behavior for Enter key. Without this, the select // is only open while the Enter key is pressed. // Prevent default behavior for Space key. Without this, Safari stays in // active state visually - if (keyCode === "Enter" || keyCode === " ") { + if ( + keyCode === keys.enter.toLowerCase() || + keyCode === keys.space.toLowerCase() + ) { this.setState({pressed: true}); e.preventDefault(); } }; handleKeyUp: (e: React.KeyboardEvent) => void = (e) => { - const keyCode = e.key; + const keyCode = e.key.toLowerCase(); // On key up for Enter and Space, trigger the click handler - if (keyCode === "Enter" || keyCode === " ") { + if ( + keyCode === keys.enter.toLowerCase() || + keyCode.toLowerCase() === keys.space.toLowerCase() + ) { this.setState({pressed: false}); this.handleClick(e); } diff --git a/packages/wonder-blocks-dropdown/src/util/constants.ts b/packages/wonder-blocks-dropdown/src/util/constants.ts index 72a3be9ff7..b517aed835 100644 --- a/packages/wonder-blocks-dropdown/src/util/constants.ts +++ b/packages/wonder-blocks-dropdown/src/util/constants.ts @@ -8,7 +8,7 @@ import {ComboboxLabels} from "./types"; export const keys = { escape: "Escape", tab: "Tab", - space: " ", + space: "Space", up: "ArrowUp", down: "ArrowDown", } as const; From 631850f324629c2d026e0f1dc499af222868f9d3 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Todd Date: Tue, 7 Jan 2025 14:56:21 -0800 Subject: [PATCH 2/9] docs(changeset): Fixes keyboard tests in Dropdown and Clickable --- .changeset/mean-cherries-press.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/mean-cherries-press.md diff --git a/.changeset/mean-cherries-press.md b/.changeset/mean-cherries-press.md new file mode 100644 index 0000000000..ff52061172 --- /dev/null +++ b/.changeset/mean-cherries-press.md @@ -0,0 +1,7 @@ +--- +"@khanacademy/wonder-blocks-clickable": patch +"@khanacademy/wonder-blocks-dropdown": patch +"@khanacademy/wonder-blocks-core": patch +--- + +Fixes keyboard tests in Dropdown and Clickable From b4bc2b7d6382688006ce95c09946064b01e72505 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Todd Date: Fri, 10 Jan 2025 16:03:59 -0800 Subject: [PATCH 3/9] [FEI-5533-part1] Update changelog with additional details --- .changeset/mean-cherries-press.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.changeset/mean-cherries-press.md b/.changeset/mean-cherries-press.md index ff52061172..7ccfc40990 100644 --- a/.changeset/mean-cherries-press.md +++ b/.changeset/mean-cherries-press.md @@ -1,7 +1,7 @@ --- -"@khanacademy/wonder-blocks-clickable": patch -"@khanacademy/wonder-blocks-dropdown": patch -"@khanacademy/wonder-blocks-core": patch +"@khanacademy/wonder-blocks-clickable": minor +"@khanacademy/wonder-blocks-dropdown": minor +"@khanacademy/wonder-blocks-core": minor --- -Fixes keyboard tests in Dropdown and Clickable +Fixes keyboard tests in Dropdown and Clickable with specific key events. We now check event.key instead of event.which or event.keyCode to match the keys returned from Testing Library/userEvent. From 674d8722efd2e282bd1cab3a94acf2823a07b689 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Todd Date: Fri, 10 Jan 2025 16:50:55 -0800 Subject: [PATCH 4/9] [FEI-5533-part1] Disable keyboard story in Chromatic --- .../single-select.accessibility.stories.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx b/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx index e73b3147bd..2a0db8647a 100644 --- a/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx +++ b/__docs__/wonder-blocks-dropdown/single-select.accessibility.stories.tsx @@ -58,6 +58,7 @@ export const UsingAriaAttributes = { name: "Using aria attributes", }; +// This story exists for debugging automated unit tests. const SingleSelectKeyboardSelection = () => { const [selectedValue, setSelectedValue] = React.useState(""); return ( @@ -78,4 +79,7 @@ const SingleSelectKeyboardSelection = () => { export const UsingKeyboardSelection = { render: SingleSelectKeyboardSelection.bind({}), name: "Using the keyboard", + parameters: { + chromatic: {disableSnapshot: true}, + }, }; From b3bc70a5927804314c471b564b8e25c3b02d59e0 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Todd Date: Fri, 10 Jan 2025 17:06:53 -0800 Subject: [PATCH 5/9] [FEI-5533-part1] PR feedback: 1. Use core keys over wonder-blocks-dropdown constants 2. Use keyName over keyCode for key names 3. Use one assertion per test 4. Use CapitalCase event names to simplify code --- .../__tests__/clickable-behavior.test.tsx | 2 +- .../src/components/clickable-behavior.ts | 17 +++---- packages/wonder-blocks-core/src/util/keys.ts | 4 ++ .../components/__tests__/action-menu.test.tsx | 20 +++----- .../components/__tests__/combobox.test.tsx | 2 +- .../__tests__/multi-select.test.tsx | 16 +++--- .../__tests__/single-select.test.tsx | 49 ++++++++++++++----- .../src/components/dropdown-core.tsx | 4 +- .../src/components/select-opener.tsx | 14 ++---- .../src/util/constants.ts | 12 ----- 10 files changed, 72 insertions(+), 68 deletions(-) diff --git a/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx b/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx index d944edf097..ae1020ddae 100644 --- a/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx +++ b/packages/wonder-blocks-clickable/src/components/__tests__/clickable-behavior.test.tsx @@ -215,7 +215,7 @@ describe("ClickableBehavior", () => { const button = await screen.findByRole("button"); expect(button).not.toHaveTextContent("focused"); await userEvent.tab(); - await userEvent.keyboard("{space}"); + await userEvent.keyboard("{Space}"); // NOTE(kevinb): await userEvent.click() fires other events that we don't want // affecting this test case. fireEvent.click(button); diff --git a/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts b/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts index 37868b648c..aea9e7d5e3 100644 --- a/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts +++ b/packages/wonder-blocks-clickable/src/components/clickable-behavior.ts @@ -556,13 +556,12 @@ export default class ClickableBehavior extends React.Component< if (onKeyDown) { onKeyDown(e); } - // Allow tests to use mixed case commands ("Space" or "space") - const keyCode = e.key.toLowerCase(); + const keyName = e.key; const {triggerOnEnter, triggerOnSpace} = getAppropriateTriggersForRole(role); if ( - (triggerOnEnter && keyCode === keys.enter.toLowerCase()) || - (triggerOnSpace && keyCode === keys.space.toLowerCase()) + (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 @@ -570,7 +569,7 @@ export default class ClickableBehavior extends React.Component< // handleKeyUp instead. e.preventDefault(); this.setState({pressed: true}); - } else if (!triggerOnEnter && keyCode === keys.enter.toLowerCase()) { + } 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; @@ -583,17 +582,17 @@ export default class ClickableBehavior extends React.Component< onKeyUp(e); } - const keyCode = e.key.toLowerCase(); + const keyName = e.key; const {triggerOnEnter, triggerOnSpace} = getAppropriateTriggersForRole(role); if ( - (triggerOnEnter && keyCode === keys.enter.toLowerCase()) || - (triggerOnSpace && keyCode === keys.space.toLowerCase()) + (triggerOnEnter && keyName === keys.enter) || + (triggerOnSpace && keyName === keys.space) ) { this.setState({pressed: false, focused: true}); this.runCallbackAndMaybeNavigate(e); - } else if (!triggerOnEnter && keyCode === keys.enter) { + } else if (!triggerOnEnter && keyName === keys.enter) { this.enterClick = false; } }; diff --git a/packages/wonder-blocks-core/src/util/keys.ts b/packages/wonder-blocks-core/src/util/keys.ts index de3b7a78c1..728f4fa0f7 100644 --- a/packages/wonder-blocks-core/src/util/keys.ts +++ b/packages/wonder-blocks-core/src/util/keys.ts @@ -1,3 +1,7 @@ +/** + * 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", diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/action-menu.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/action-menu.test.tsx index 61388ddaa0..cdbc18bd88 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/action-menu.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/action-menu.test.tsx @@ -38,9 +38,7 @@ describe("ActionMenu", () => { ).toBeInTheDocument(); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("opens the menu on enter", async () => { + it("opens the menu on enter", async () => { // Arrange render( { // Act await userEvent.tab(); - await userEvent.keyboard("{enter}"); + await userEvent.keyboard("{Enter}"); // Assert expect( @@ -65,9 +63,7 @@ describe("ActionMenu", () => { ).toBeInTheDocument(); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("closes itself on escape", async () => { + it("closes itself on escape", async () => { // Arrange render( { ); await userEvent.tab(); - await userEvent.keyboard("{enter}"); + await userEvent.keyboard("{Enter}"); // Act - await userEvent.keyboard("{escape}"); + await userEvent.keyboard("{Escape}"); // Assert expect(screen.queryByRole("menu")).not.toBeInTheDocument(); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("closes itself on tab", async () => { + it("closes itself on tab", async () => { // Arrange render( { ); await userEvent.tab(); - await userEvent.keyboard("{enter}"); + await userEvent.keyboard("{Enter}"); // Act await userEvent.tab(); diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/combobox.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/combobox.test.tsx index cb33014611..2c8e207f95 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/combobox.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/combobox.test.tsx @@ -280,7 +280,7 @@ describe("Combobox", () => { await userEvent.type(screen.getByRole("combobox"), "3"); // Act - await userEvent.keyboard("{enter}"); + await userEvent.keyboard("{Enter}"); // Assert expect(screen.getByRole("combobox")).toHaveValue("option 3"); diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx index 3bf0ad1e35..d85c4d62a9 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/multi-select.test.tsx @@ -85,17 +85,15 @@ describe("MultiSelect", () => { ).toBeInTheDocument(); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("closes the select on {escape}", async () => { + it("closes the select on {Escape}", async () => { // Arrange const {userEvent} = doRender(uncontrolledMultiSelect); await userEvent.tab(); - await userEvent.keyboard("{enter}"); // open + await userEvent.keyboard("{Enter}"); // open // Act - await userEvent.keyboard("{escape}"); + await userEvent.keyboard("{Escape}"); // Assert expect(onChange).not.toHaveBeenCalled(); @@ -2809,10 +2807,10 @@ describe("MultiSelect", () => { />, ); await userEvent.tab(); - await userEvent.keyboard("{enter}"); // Open the dropdown + await userEvent.keyboard("{Enter}"); // Open the dropdown // Act - await userEvent.keyboard("{escape}"); // Close the dropdown + await userEvent.keyboard("{Escape}"); // Close the dropdown // Assert expect(onValidate).toHaveBeenCalledExactlyOnceWith( @@ -2827,10 +2825,10 @@ describe("MultiSelect", () => { , ); await userEvent.tab(); - await userEvent.keyboard("{enter}"); // Open the dropdown + await userEvent.keyboard("{Enter}"); // Open the dropdown // Act - await userEvent.keyboard("{escape}"); // Close the dropdown + await userEvent.keyboard("{Escape}"); // Close the dropdown // Assert expect(await screen.findByRole("button")).toHaveAttribute( diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx index 7cfdbfcb05..24eb747721 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/single-select.test.tsx @@ -287,7 +287,7 @@ describe("SingleSelect", () => { jest.useFakeTimers(); }); - describe.each([{key: "{enter}"}, {key: "{space}"}])( + describe.each([{key: "{Enter}"}, {key: "{Space}"}])( "$key", ({key}: any) => { it("should open when pressing the key when the default opener is focused", async () => { @@ -320,22 +320,32 @@ describe("SingleSelect", () => { }, ); - it("should select an item when pressing {enter}", async () => { + it("should focus on the first option when pressing {Enter} on the opener", async () => { // Arrange const {userEvent} = doRender(uncontrolledSingleSelect); await userEvent.tab(); - await userEvent.keyboard("{enter}"); // open + + // Act + await userEvent.keyboard("{Enter}"); // open // Ensure first option is focused, not the opener const firstItem = await screen.findByRole("option", { name: /item 1/, }); + // Assert expect(firstItem).toHaveFocus(); + }); + + it("should select an item when pressing {Enter}", async () => { + // Arrange + const {userEvent} = doRender(uncontrolledSingleSelect); + await userEvent.tab(); + await userEvent.keyboard("{Enter}"); // open // Act - await userEvent.keyboard("{enter}"); + await userEvent.keyboard("{Enter}"); - // // Assert + // Assert expect(onChange).toHaveBeenCalledWith("1"); await waitFor(() => @@ -345,11 +355,28 @@ describe("SingleSelect", () => { ); }); - it("should select an item when pressing {space}", async () => { + it("should focus on the first option when pressing {Space} on the opener", async () => { + // Arrange + const {userEvent} = doRender(uncontrolledSingleSelect); + await userEvent.tab(); + + // Act + await userEvent.keyboard("{Space}"); // open + + // Ensure first option is focused, not the opener + const firstItem = await screen.findByRole("option", { + name: /item 1/, + }); + + // Assert + expect(firstItem).toHaveFocus(); + }); + + it("should select an item when pressing {Space}", async () => { // Arrange const {userEvent} = doRender(uncontrolledSingleSelect); await userEvent.tab(); - await userEvent.keyboard("{enter}"); // open + await userEvent.keyboard("{Enter}"); // open const firstItem = await screen.findByRole("option", { name: /item 1/, @@ -357,7 +384,7 @@ describe("SingleSelect", () => { expect(firstItem).toHaveFocus(); // Act - await userEvent.keyboard("{space}"); + await userEvent.keyboard("{Space}"); // Assert expect(onChange).toHaveBeenCalledWith("1"); @@ -428,7 +455,7 @@ describe("SingleSelect", () => { // Arrange const {userEvent} = doRender(uncontrolledSingleSelect); await userEvent.tab(); - await userEvent.keyboard("{enter}"); // open + await userEvent.keyboard("{Enter}"); // open // Act await userEvent.keyboard("{escape}"); @@ -2270,7 +2297,7 @@ describe("SingleSelect", () => { // Act await userEvent.tab(); - await userEvent.keyboard("{enter}"); // Open the dropdown + await userEvent.keyboard("{Enter}"); // Open the dropdown await userEvent.keyboard("{escape}"); // Close the dropdown // Assert @@ -2288,7 +2315,7 @@ describe("SingleSelect", () => { // Act await userEvent.tab(); - await userEvent.keyboard("{enter}"); // Open the dropdown + await userEvent.keyboard("{Enter}"); // Open the dropdown await userEvent.keyboard("{escape}"); // Close the dropdown // Assert diff --git a/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx b/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx index e61e7021fb..aef5b65e80 100644 --- a/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx +++ b/packages/wonder-blocks-dropdown/src/components/dropdown-core.tsx @@ -9,7 +9,7 @@ import {VariableSizeList as List} from "react-window"; import {fade, color, spacing} from "@khanacademy/wonder-blocks-tokens"; -import {addStyle, PropsFor, View} from "@khanacademy/wonder-blocks-core"; +import {addStyle, PropsFor, View, keys} from "@khanacademy/wonder-blocks-core"; import SearchField from "@khanacademy/wonder-blocks-search-field"; import {LabelMedium} from "@khanacademy/wonder-blocks-typography"; import {withActionScheduler} from "@khanacademy/wonder-blocks-timing"; @@ -18,7 +18,7 @@ import type {AriaProps, StyleType} from "@khanacademy/wonder-blocks-core"; import type {WithActionSchedulerProps} from "@khanacademy/wonder-blocks-timing"; import DropdownCoreVirtualized from "./dropdown-core-virtualized"; import SeparatorItem from "./separator-item"; -import {defaultLabels, keys} from "../util/constants"; +import {defaultLabels} from "../util/constants"; import type {DropdownItem} from "../util/types"; import DropdownPopper from "./dropdown-popper"; import {debounce, getLabel, getStringForKey} from "../util/helpers"; diff --git a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx index 43fb88971e..ac49b35e76 100644 --- a/packages/wonder-blocks-dropdown/src/components/select-opener.tsx +++ b/packages/wonder-blocks-dropdown/src/components/select-opener.tsx @@ -107,27 +107,21 @@ export default class SelectOpener extends React.Component< }; handleKeyDown: (e: React.KeyboardEvent) => void = (e) => { - const keyCode = e.key.toLowerCase(); + const keyName = e.key; // Prevent default behavior for Enter key. Without this, the select // is only open while the Enter key is pressed. // Prevent default behavior for Space key. Without this, Safari stays in // active state visually - if ( - keyCode === keys.enter.toLowerCase() || - keyCode === keys.space.toLowerCase() - ) { + if (keyName === keys.enter || keyName === keys.space) { this.setState({pressed: true}); e.preventDefault(); } }; handleKeyUp: (e: React.KeyboardEvent) => void = (e) => { - const keyCode = e.key.toLowerCase(); + const keyName = e.key; // On key up for Enter and Space, trigger the click handler - if ( - keyCode === keys.enter.toLowerCase() || - keyCode.toLowerCase() === keys.space.toLowerCase() - ) { + if (keyName === keys.enter || keyName === keys.space) { this.setState({pressed: false}); this.handleClick(e); } diff --git a/packages/wonder-blocks-dropdown/src/util/constants.ts b/packages/wonder-blocks-dropdown/src/util/constants.ts index b517aed835..4f6d9b2017 100644 --- a/packages/wonder-blocks-dropdown/src/util/constants.ts +++ b/packages/wonder-blocks-dropdown/src/util/constants.ts @@ -1,18 +1,6 @@ import {spacing} from "@khanacademy/wonder-blocks-tokens"; import {ComboboxLabels} from "./types"; -/** - * Key value mapping reference: - * https://developer.mozilla.org/en-US/docs/Web/API/UI_Events/Keyboard_event_key_values - */ -export const keys = { - escape: "Escape", - tab: "Tab", - space: "Space", - up: "ArrowUp", - down: "ArrowDown", -} as const; - export const selectDropdownStyle = { marginTop: spacing.xSmall_8, marginBottom: spacing.xSmall_8, From 226b4e72d294a257350865dd5b0445f9f29386a9 Mon Sep 17 00:00:00 2001 From: Marcy Sutton Todd Date: Mon, 13 Jan 2025 14:56:21 -0800 Subject: [PATCH 6/9] [FEI-5533-part1] Re-enables passing tests or adds notes to fails --- .../__tests__/dropdown-core.test.tsx | 72 ++++++++----------- .../__tests__/multi-select.test.tsx | 10 ++- .../__tests__/single-select.test.tsx | 4 +- 3 files changed, 35 insertions(+), 51 deletions(-) diff --git a/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx b/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx index 3f247da6fb..e54972b673 100644 --- a/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx +++ b/packages/wonder-blocks-dropdown/src/components/__tests__/dropdown-core.test.tsx @@ -73,9 +73,7 @@ describe("DropdownCore", () => { expect(item).toHaveFocus(); }); - // TODO(FEI-5533): Key press events aren't working correctly with - // user-event v14. We need to investigate and fix this. - it.skip("handles basic keyboard navigation as expected", async () => { + it("handles basic keyboard navigation as expected", async () => { // Arrange const dummyOpener =