From 5de483386693884ccdef22b9740582e6098a5baa Mon Sep 17 00:00:00 2001
From: Cat Johnson <123020281+catandthemachines@users.noreply.github.com>
Date: Wed, 3 Jul 2024 13:59:18 -0700
Subject: [PATCH] [Expression Remediation] Create arrow key navigation for
 TabBar component. (#1384)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## 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: https://github.com/Khan/perseus/pull/1384
---
 .changeset/polite-snails-know.md              |   5 +
 .../__snapshots__/keypad.test.tsx.snap        |  14 +-
 .../__snapshots__/mobile-keypad.test.tsx.snap |   8 +-
 .../keypad/__tests__/keypad.test.tsx          |   4 +-
 .../keypad/__tests__/mobile-keypad.test.tsx   |   2 +-
 .../src/components/keypad/keypad.tsx          |   1 -
 .../components/tabbar/__tests__/item.test.tsx |  82 +++++++++++
 .../tabbar/__tests__/tabbar.test.tsx          |   8 +-
 .../math-input/src/components/tabbar/item.tsx | 127 +++++++++++-------
 .../src/components/tabbar/tabbar.tsx          |  62 ++++++---
 .../components/__tests__/math-input.test.tsx  |   2 -
 .../__tests__/expression-mobile.test.tsx      |   2 +-
 12 files changed, 231 insertions(+), 86 deletions(-)
 create mode 100644 .changeset/polite-snails-know.md
 create mode 100644 packages/math-input/src/components/tabbar/__tests__/item.test.tsx

diff --git a/.changeset/polite-snails-know.md b/.changeset/polite-snails-know.md
new file mode 100644
index 0000000000..4817ab23ed
--- /dev/null
+++ b/.changeset/polite-snails-know.md
@@ -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.
diff --git a/packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap b/packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap
index f261ded6fa..bcb11f03e6 100644
--- a/packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap
+++ b/packages/math-input/src/components/keypad/__tests__/__snapshots__/keypad.test.tsx.snap
@@ -10,10 +10,10 @@ 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"
@@ -21,6 +21,7 @@ exports[`keypad should snapshot expanded: first render 1`] = `
             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
@@ -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
@@ -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
@@ -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
@@ -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"
@@ -1063,10 +1066,10 @@ 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"
@@ -1074,6 +1077,7 @@ exports[`keypad should snapshot unexpanded: first render 1`] = `
             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
@@ -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
@@ -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
@@ -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
@@ -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"
diff --git a/packages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap b/packages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap
index 3947c7b393..d80bee5b46 100644
--- a/packages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap
+++ b/packages/math-input/src/components/keypad/__tests__/__snapshots__/mobile-keypad.test.tsx.snap
@@ -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"
           >
@@ -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
@@ -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"
diff --git a/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx b/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx
index cc90aa8e8c..4aee5e1145 100644
--- a/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx
+++ b/packages/math-input/src/components/keypad/__tests__/keypad.test.tsx
@@ -110,7 +110,7 @@ describe("keypad", () => {
 
         // Assert
         expect(
-            screen.getByRole("tab", {
+            screen.getByRole("button", {
                 name: "Dismiss",
             }),
         ).toBeInTheDocument();
@@ -125,7 +125,7 @@ describe("keypad", () => {
 
         // Assert
         expect(
-            screen.queryByRole("tab", {
+            screen.queryByRole("button", {
                 name: "Dismiss",
             }),
         ).not.toBeInTheDocument();
diff --git a/packages/math-input/src/components/keypad/__tests__/mobile-keypad.test.tsx b/packages/math-input/src/components/keypad/__tests__/mobile-keypad.test.tsx
index 502a76691c..b4a108825d 100644
--- a/packages/math-input/src/components/keypad/__tests__/mobile-keypad.test.tsx
+++ b/packages/math-input/src/components/keypad/__tests__/mobile-keypad.test.tsx
@@ -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", () => {
diff --git a/packages/math-input/src/components/keypad/keypad.tsx b/packages/math-input/src/components/keypad/keypad.tsx
index 4aaf90f145..4e21539b2e 100644
--- a/packages/math-input/src/components/keypad/keypad.tsx
+++ b/packages/math-input/src/components/keypad/keypad.tsx
@@ -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" && (
diff --git a/packages/math-input/src/components/tabbar/__tests__/item.test.tsx b/packages/math-input/src/components/tabbar/__tests__/item.test.tsx
new file mode 100644
index 0000000000..9c301320cf
--- /dev/null
+++ b/packages/math-input/src/components/tabbar/__tests__/item.test.tsx
@@ -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();
+    });
+});
diff --git a/packages/math-input/src/components/tabbar/__tests__/tabbar.test.tsx b/packages/math-input/src/components/tabbar/__tests__/tabbar.test.tsx
index 2766350da0..ac4f3f47a2 100644
--- a/packages/math-input/src/components/tabbar/__tests__/tabbar.test.tsx
+++ b/packages/math-input/src/components/tabbar/__tests__/tabbar.test.tsx
@@ -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 () => {
@@ -88,7 +90,7 @@ describe("<Tabbar />", () => {
 
         // Assert
         expect(
-            screen.queryByRole("tab", {name: "Dismiss"}),
+            screen.queryByRole("button", {name: "Dismiss"}),
         ).not.toBeInTheDocument();
     });
 
@@ -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();
     });
 });
diff --git a/packages/math-input/src/components/tabbar/item.tsx b/packages/math-input/src/components/tabbar/item.tsx
index d51bd63b95..de95e5adb6 100644
--- a/packages/math-input/src/components/tabbar/item.tsx
+++ b/packages/math-input/src/components/tabbar/item.tsx
@@ -3,6 +3,7 @@ import {View} from "@khanacademy/wonder-blocks-core";
 import {color} from "@khanacademy/wonder-blocks-tokens";
 import {StyleSheet} from "aphrodite";
 import * as React from "react";
+import {useEffect, useRef} from "react";
 
 import IconAsset from "./icons";
 
@@ -75,70 +76,94 @@ function imageTintColor(
     return color.offBlack64;
 }
 export type ItemState = "active" | "inactive" | "disabled";
-type Props = {
+type TabItemProps = {
     onClick: () => void;
     itemState: ItemState;
     itemType: KeypadPageType;
+    focus?: boolean;
+    role: "tab" | "button";
 };
 
-class TabbarItem extends React.Component<Props> {
-    render(): React.ReactNode {
-        const {onClick, itemType, itemState} = this.props;
-        return (
-            <Clickable
-                onClick={onClick}
-                disabled={itemState === "disabled"}
-                aria-label={itemType}
-                style={styles.clickable}
-                aria-selected={itemState === "active"}
-                role="tab"
-            >
-                {({hovered, focused, pressed}) => {
-                    const tintColor = imageTintColor(
-                        itemState,
-                        hovered,
-                        focused,
-                        pressed,
-                    );
+function TabbarItem(props: TabItemProps): React.ReactElement {
+    const {onClick, itemType, itemState, focus, role} = props;
+    const tabRef = useRef<{focus: () => void}>(null);
 
-                    return (
+    useEffect(() => {
+        let timeout;
+        if (role === "tab" && focus) {
+            /**
+             * When tabs are within a WonderBlocks Popover component, the
+             * manner in which the component is rendered and moved causes
+             * focus to snap to the bottom of the page on first focus.
+             *
+             * This timeout moves around that by delaying the focus enough
+             * to wait for the WonderBlock Popover to move to the correct
+             * location and scroll the user to the correct location.
+             * */
+            timeout = setTimeout(() => {
+                if (tabRef?.current) {
+                    // Move element into view when it is focused
+                    tabRef?.current.focus();
+                }
+            }, 0);
+        }
+
+        return () => clearTimeout(timeout);
+    }, [role, focus, tabRef]);
+
+    return (
+        <Clickable
+            onClick={onClick}
+            disabled={itemState === "disabled"}
+            aria-label={itemType}
+            style={styles.clickable}
+            aria-selected={itemState === "active"}
+            tabIndex={role === "button" ? 0 : focus ? 0 : -1}
+            role={role}
+            ref={tabRef}
+        >
+            {({hovered, focused, pressed}) => {
+                const tintColor = imageTintColor(
+                    itemState,
+                    hovered,
+                    focused,
+                    pressed,
+                );
+
+                return (
+                    <View
+                        style={[
+                            styles.base,
+                            itemState !== "disabled" &&
+                                hovered &&
+                                styles.hovered,
+                            focused && styles.focused,
+                            pressed && styles.pressed,
+                        ]}
+                    >
                         <View
                             style={[
-                                styles.base,
-                                itemState !== "disabled" &&
-                                    hovered &&
-                                    styles.hovered,
-                                focused && styles.focused,
-                                pressed && styles.pressed,
+                                styles.innerBox,
+                                pressed && styles.innerBoxPressed,
                             ]}
                         >
+                            <IconAsset type={itemType} tintColor={tintColor} />
+                        </View>
+                        {itemState === "active" && (
                             <View
                                 style={[
-                                    styles.innerBox,
-                                    pressed && styles.innerBoxPressed,
+                                    styles.activeIndicator,
+                                    {
+                                        backgroundColor: tintColor,
+                                    },
                                 ]}
-                            >
-                                <IconAsset
-                                    type={itemType}
-                                    tintColor={tintColor}
-                                />
-                            </View>
-                            {itemState === "active" && (
-                                <View
-                                    style={[
-                                        styles.activeIndicator,
-                                        {
-                                            backgroundColor: tintColor,
-                                        },
-                                    ]}
-                                />
-                            )}
-                        </View>
-                    );
-                }}
-            </Clickable>
-        );
-    }
+                            />
+                        )}
+                    </View>
+                );
+            }}
+        </Clickable>
+    );
 }
 
 export const TabbarItemForTesting = TabbarItem;
diff --git a/packages/math-input/src/components/tabbar/tabbar.tsx b/packages/math-input/src/components/tabbar/tabbar.tsx
index 0d9a3cd2ac..5dcf0ebe66 100644
--- a/packages/math-input/src/components/tabbar/tabbar.tsx
+++ b/packages/math-input/src/components/tabbar/tabbar.tsx
@@ -1,6 +1,7 @@
 import {View} from "@khanacademy/wonder-blocks-core";
 import {StyleSheet} from "aphrodite";
 import * as React from "react";
+import {useState} from "react";
 
 import TabbarItem from "./item";
 
@@ -31,27 +32,58 @@ type Props = {
 
 function Tabbar(props: Props): React.ReactElement {
     const {items, onClickClose, selectedItem, onSelectItem, style} = props;
+    const selectedIndex = items.indexOf(selectedItem);
+
+    const [focus, setFocus] = useState(
+        selectedIndex === -1 ? 0 : selectedIndex,
+    );
+    /**
+     * Custom function to handle arrow key navigation for the TabBar for each TabItem.
+     * This implementation also circular in that if the user goes past the end of
+     * the list they will go back to the beginning and vise versa.
+     * This is the recommended pattern per WCAG implementation:
+     * https://www.w3.org/WAI/ARIA/apg/patterns/tabs/examples/tabs-manual/
+     * @param e - onKeyDown event data.
+     */
+    const onArrowKeyFocus = (e) => {
+        if (e.keyCode === 39) {
+            // Right arrow
+            setFocus(focus === items.length - 1 ? 0 : focus + 1);
+        } else if (e.keyCode === 37) {
+            // Left arrow
+            setFocus(focus === 0 ? items.length - 1 : focus - 1);
+        }
+    };
 
     return (
-        <View style={[styles.tabbar, style]} role="tablist">
-            <View style={[styles.pages]}>
-                {items.map((item) => (
-                    <TabbarItem
-                        key={`tabbar-item-${item}`}
-                        itemState={
-                            item === selectedItem ? "active" : "inactive"
-                        }
-                        itemType={item}
-                        onClick={() => {
-                            onSelectItem(item);
-                        }}
-                    />
-                ))}
-            </View>
+        <View style={[styles.tabbar, style]}>
+            {items.length > 0 && (
+                <View
+                    style={[styles.pages]}
+                    role="tablist"
+                    onKeyDown={onArrowKeyFocus}
+                >
+                    {items.map((item, index) => (
+                        <TabbarItem
+                            role="tab"
+                            key={`tabbar-item-${item}`}
+                            itemState={
+                                item === selectedItem ? "active" : "inactive"
+                            }
+                            itemType={item}
+                            focus={focus === index}
+                            onClick={() => {
+                                onSelectItem(item);
+                            }}
+                        />
+                    ))}
+                </View>
+            )}
 
             <View>
                 {onClickClose && (
                     <TabbarItem
+                        role="button"
                         itemState="inactive"
                         itemType="Dismiss"
                         onClick={onClickClose}
diff --git a/packages/perseus/src/components/__tests__/math-input.test.tsx b/packages/perseus/src/components/__tests__/math-input.test.tsx
index 4c4ca41669..f0ac28fe3e 100644
--- a/packages/perseus/src/components/__tests__/math-input.test.tsx
+++ b/packages/perseus/src/components/__tests__/math-input.test.tsx
@@ -162,8 +162,6 @@ describe("Perseus' MathInput", () => {
         // focusing the input triggers the popover
         screen.getByRole("button").click();
         await userEvent.tab(); // to "123" tab
-        await userEvent.tab(); // to extra keys tab
-        await userEvent.tab(); // to whole keypad
         await userEvent.tab(); // to "1" button
         await userEvent.keyboard("{enter}");
         jest.runOnlyPendingTimers();
diff --git a/packages/perseus/src/widgets/__tests__/expression-mobile.test.tsx b/packages/perseus/src/widgets/__tests__/expression-mobile.test.tsx
index 15cdd642d1..e6c73333a8 100644
--- a/packages/perseus/src/widgets/__tests__/expression-mobile.test.tsx
+++ b/packages/perseus/src/widgets/__tests__/expression-mobile.test.tsx
@@ -139,7 +139,7 @@ describe("expression mobile", () => {
             expect(screen.getByRole("button", {name: "1"})).toBeVisible(),
         );
 
-        await userEvent.click(screen.getByRole("tab", {name: "Dismiss"}));
+        await userEvent.click(screen.getByRole("button", {name: "Dismiss"}));
 
         // wait for the keypad to close
         await waitFor(() =>