Skip to content

Commit

Permalink
[Expression Remediation] Create arrow key navigation for TabBar compo…
Browse files Browse the repository at this point in the history
…nent. (#1384)

## Summary:
Updating TabBar and TabbarItem component to support arrow key navigation.

Included in this PR
- The creation of arrow key navigation for Tabbar and specifically the TabItems used for changing the presenting panel.
- Implementation of functionality for tab bar items, specifically the focus property and ability to change the role type.

Issue: https://khanacademy.atlassian.net/browse/LEMS-2130

## Test plan:
Storybook
- http://localhost:6006/?path=/story/math-input-full-keypad--everything-minus-navigation-pad
- Confirm that tabbing goes through only the selected tabitem, and to select another tab a user must use their left and right arrow keys.

Author: catandthemachines

Reviewers: catandthemachines, handeyeco, mark-fitzgerald, anakaren-rojas

Required Reviewers:

Approved By: mark-fitzgerald

Checks: ⌛ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1384
  • Loading branch information
catandthemachines committed Jul 3, 2024
1 parent 4b56e10 commit 5de4833
Show file tree
Hide file tree
Showing 12 changed files with 231 additions and 86 deletions.
5 changes: 5 additions & 0 deletions .changeset/polite-snails-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/math-input": minor
---

Updating TabBar experience in to use arrow-key navigation to access the other TabItems. This will ensure the Expression Widget in perseus has proper keyboard navigation for users.
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@ exports[`keypad should snapshot expanded: first render 1`] = `
>
<div
class="default_xu2jcg-o_O-tabbar_721koc"
role="tablist"
>
<div
class="default_xu2jcg-o_O-pages_cjo0m2"
role="tablist"
>
<button
aria-disabled="false"
aria-label="Numbers"
aria-selected="true"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="0"
type="button"
>
<div
Expand Down Expand Up @@ -53,6 +54,7 @@ exports[`keypad should snapshot expanded: first render 1`] = `
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="-1"
type="button"
>
<div
Expand Down Expand Up @@ -84,6 +86,7 @@ exports[`keypad should snapshot expanded: first render 1`] = `
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="-1"
type="button"
>
<div
Expand Down Expand Up @@ -115,6 +118,7 @@ exports[`keypad should snapshot expanded: first render 1`] = `
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="-1"
type="button"
>
<div
Expand Down Expand Up @@ -170,7 +174,6 @@ exports[`keypad should snapshot expanded: first render 1`] = `
<div
aria-label="Keypad"
class="default_xu2jcg-o_O-keypadGrid_ztxlrb-o_O-expressionGrid_1fuqhx9"
tabindex="0"
>
<div
class="default_xu2jcg-o_O-inlineStyles_1c2q4k1"
Expand Down Expand Up @@ -1063,17 +1066,18 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
>
<div
class="default_xu2jcg-o_O-tabbar_721koc"
role="tablist"
>
<div
class="default_xu2jcg-o_O-pages_cjo0m2"
role="tablist"
>
<button
aria-disabled="false"
aria-label="Numbers"
aria-selected="true"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="0"
type="button"
>
<div
Expand Down Expand Up @@ -1106,6 +1110,7 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="-1"
type="button"
>
<div
Expand Down Expand Up @@ -1137,6 +1142,7 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="-1"
type="button"
>
<div
Expand Down Expand Up @@ -1168,6 +1174,7 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
tabindex="-1"
type="button"
>
<div
Expand Down Expand Up @@ -1223,7 +1230,6 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
<div
aria-label="Keypad"
class="default_xu2jcg-o_O-keypadGrid_ztxlrb-o_O-expressionGrid_1fuqhx9"
tabindex="0"
>
<div
class="default_xu2jcg-o_O-inlineStyles_1c2q4k1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,7 @@ exports[`mobile keypad should render keypad when active 1`] = `
>
<div
class="default_xu2jcg-o_O-tabbar_721koc"
role="tablist"
>
<div
class="default_xu2jcg-o_O-pages_cjo0m2"
/>
<div
class="default_xu2jcg"
>
Expand All @@ -34,7 +30,8 @@ exports[`mobile keypad should render keypad when active 1`] = `
aria-label="Dismiss"
aria-selected="false"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-clickable_1ncqa8p"
role="tab"
role="button"
tabindex="0"
type="button"
>
<div
Expand Down Expand Up @@ -68,7 +65,6 @@ exports[`mobile keypad should render keypad when active 1`] = `
<div
aria-label="Keypad"
class="default_xu2jcg-o_O-keypadGrid_ztxlrb-o_O-fractionsGrid_1aelnry"
tabindex="0"
>
<div
class="default_xu2jcg-o_O-inlineStyles_1c2q4k1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe("keypad", () => {

// Assert
expect(
screen.getByRole("tab", {
screen.getByRole("button", {
name: "Dismiss",
}),
).toBeInTheDocument();
Expand All @@ -125,7 +125,7 @@ describe("keypad", () => {

// Assert
expect(
screen.queryByRole("tab", {
screen.queryByRole("button", {
name: "Dismiss",
}),
).not.toBeInTheDocument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe("mobile keypad", () => {
);

// Assert
expect(screen.queryAllByRole("tab")).not.toHaveLength(0);
expect(screen.queryAllByRole("button")).not.toHaveLength(0);
});

it("should fire an 'opened' event when activated", () => {
Expand Down
1 change: 0 additions & 1 deletion packages/math-input/src/components/keypad/keypad.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ export default function Keypad(props: Props) {
<View style={styles.keypadInnerContainer}>
<View
style={[styles.keypadGrid, gridStyle]}
tabIndex={0}
aria-label="Keypad"
>
{selectedPage === "Fractions" && (
Expand Down
82 changes: 82 additions & 0 deletions packages/math-input/src/components/tabbar/__tests__/item.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import {render, screen} from "@testing-library/react";
import {userEvent as userEventLib} from "@testing-library/user-event";
import * as React from "react";

import TabbarItem from "../item";

describe("<TabbarItem />", () => {
let userEvent;
beforeEach(() => {
userEvent = userEventLib.setup({
advanceTimers: jest.advanceTimersByTime,
});
});

it("renders tab item", async () => {
// Arrange
render(
<TabbarItem
itemType="Numbers"
itemState="active"
role="tab"
onClick={() => {}}
/>,
);

// Assert
expect(screen.getByRole("tab", {name: "Numbers"})).toBeInTheDocument();
expect(screen.getByRole("tab", {name: "Numbers"})).not.toHaveFocus();
});

it("renders tab item with role button", async () => {
// Arrange
render(
<TabbarItem
itemType="Numbers"
itemState="active"
role="button"
onClick={() => {}}
/>,
);

// Assert
expect(
screen.getByRole("button", {name: "Numbers"}),
).toBeInTheDocument();
});

it("can set focus when focus is enabled", async () => {
// Arrange
jest.useFakeTimers();
render(
<TabbarItem
itemType="Numbers"
itemState="active"
focus={true}
role="tab"
onClick={() => {}}
/>,
);
jest.runAllTimers();

// Assert
expect(screen.getByRole("tab", {name: "Numbers"})).toHaveFocus();
});

it("handles onClick callback", async () => {
// Arrange
const mockClick = jest.fn();
render(
<TabbarItem
itemType="Numbers"
itemState="active"
role="tab"
onClick={mockClick}
/>,
);

// Assert
await userEvent.click(screen.getByRole("tab", {name: "Numbers"}));
expect(mockClick).toHaveBeenCalled();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ describe("<Tabbar />", () => {
);

// Assert
expect(screen.getByRole("tab", {name: "Dismiss"})).toBeInTheDocument();
expect(
screen.getByRole("button", {name: "Dismiss"}),
).toBeInTheDocument();
});

it("does not show dismiss button without onClickClose callback", async () => {
Expand All @@ -88,7 +90,7 @@ describe("<Tabbar />", () => {

// Assert
expect(
screen.queryByRole("tab", {name: "Dismiss"}),
screen.queryByRole("button", {name: "Dismiss"}),
).not.toBeInTheDocument();
});

Expand All @@ -105,7 +107,7 @@ describe("<Tabbar />", () => {
);

// Assert
await userEvent.click(screen.getByRole("tab", {name: "Dismiss"}));
await userEvent.click(screen.getByRole("button", {name: "Dismiss"}));
expect(mockClickCloseCallback).toHaveBeenCalled();
});
});
Loading

0 comments on commit 5de4833

Please sign in to comment.