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

[Expression Remediation] Create arrow key navigation for TabBar component. #1384

Merged
merged 19 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
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,21 +10,22 @@ 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"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-focused_en8zhl-o_O-clickable_1ncqa8p"
role="tab"
tabindex="0"
type="button"
>
<div
class="default_xu2jcg-o_O-base_1pupywz"
class="default_xu2jcg-o_O-base_1pupywz-o_O-focused_2hwz1v"
>
<div
class="default_xu2jcg-o_O-innerBox_qdbgl3"
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,21 +1066,22 @@ 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"
class="button_vr44p2-o_O-reset_152ygtm-o_O-link_13xlah4-o_O-focused_en8zhl-o_O-clickable_1ncqa8p"
role="tab"
tabindex="0"
type="button"
>
<div
class="default_xu2jcg-o_O-base_1pupywz"
class="default_xu2jcg-o_O-base_1pupywz-o_O-focused_2hwz1v"
>
<div
class="default_xu2jcg-o_O-innerBox_qdbgl3"
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,10 +21,10 @@ 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"
role="tablist"
catandthemachines marked this conversation as resolved.
Show resolved Hide resolved
/>
<div
class="default_xu2jcg"
Expand All @@ -34,7 +34,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 +69,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"
catandthemachines marked this conversation as resolved.
Show resolved Hide resolved
>
{selectedPage === "Fractions" && (
Expand Down
80 changes: 80 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,80 @@
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
render(
<TabbarItem
itemType="Numbers"
itemState="active"
focus={true}
role="tab"
onClick={() => {}}
/>,
);

// 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
Loading