Fix tab focus

Author: connor-baer

packages/circuit-ui/components/Tabs/Tabs.spec.tsx

@@ -73,7 +73,7 @@ describe('Tabs', () => {
   it('should go to the previous tab on left arrow press', async () => {
     render(
       <Tabs
-        initialSelectedIndex={1}
+        initialSelectedId="b"
         items={[
           { id: 'a', tab: 'tab-a', panel: 'panel-a' },
           { id: 'b', tab: 'tab-b', panel: 'panel-b' },
@@ -85,7 +85,7 @@ describe('Tabs', () => {
     await userEvent.keyboard('{Tab}');
 
     const tabs = screen.getAllByRole('tab');
-    expect(tabs[0]).toHaveFocus();
+    expect(tabs[1]).toHaveFocus();
 
     const panelB = screen.getByRole('tabpanel');
     expect(panelB).toHaveAccessibleName('tab-b');
@@ -100,7 +100,6 @@ describe('Tabs', () => {
   it('should focus the current panel on down arrow press', async () => {
     render(
       <Tabs
-        initialSelectedIndex={1}
         items={[
           { id: 'a', tab: 'tab-a', panel: 'panel-a' },
           { id: 'b', tab: 'tab-b', panel: 'panel-b' },

packages/circuit-ui/components/Tabs/Tabs.tsx

@@ -15,13 +15,7 @@
 
 'use client';
 
-import {
-  createRef,
-  Fragment,
-  useState,
-  type KeyboardEvent,
-  type ReactNode,
-} from 'react';
+import { Fragment, useState, type KeyboardEvent, type ReactNode } from 'react';
 
 import {
   isArrowLeft,
@@ -35,9 +29,11 @@ import { TabPanel } from './components/TabPanel/index.js';
 
 export interface TabsProps extends TabListProps {
   /**
-   * The index of the initially selected tab.
+   * The id of the initially selected tab.
+   *
+   * @default items[0].id
    */
-  initialSelectedIndex?: number;
+  initialSelectedId?: string;
   /**
    * A collection of tabs with an id, the tab label, and panel content.
    */
@@ -48,34 +44,43 @@ export interface TabsProps extends TabListProps {
   }[];
 }
 
-export function Tabs({ initialSelectedIndex = 0, items, ...props }: TabsProps) {
-  const [selectedIndex, setSelectedIndex] = useState(initialSelectedIndex);
-
-  const tabPanelsRefs = createRefs(items.length);
+export function Tabs({
+  items,
+  initialSelectedId = items[0].id,
+  ...props
+}: TabsProps) {
+  const [selectedId, setSelectedId] = useState(initialSelectedId);
 
   const handleTabKeyDown = (event: KeyboardEvent) => {
+    const selectedIndex = items.findIndex((item) => item.id === selectedId);
+
     if (isArrowLeft(event)) {
-      const previousTab = Math.max(0, selectedIndex - 1);
-      setSelectedIndex(previousTab);
+      const previousIndex = selectedIndex - 1;
+      if (previousIndex >= 0) {
+        const previousId = items[previousIndex].id;
+        setSelectedId(previousId);
+        document.getElementById(`tab-${previousId}`)?.focus();
+      }
     } else if (isArrowRight(event)) {
-      const nextTab = Math.min(items.length - 1, selectedIndex + 1);
-      setSelectedIndex(nextTab);
-    } else if (isArrowDown(event)) {
-      const panelRef = tabPanelsRefs[selectedIndex].current;
-      if (panelRef) {
-        panelRef.focus();
+      const nextIndex = selectedIndex + 1;
+      if (nextIndex <= items.length - 1) {
+        const nextId = items[nextIndex].id;
+        setSelectedId(nextId);
+        document.getElementById(`tab-${nextId}`)?.focus();
       }
+    } else if (isArrowDown(event)) {
+      document.getElementById(`panel-${selectedId}`)?.focus();
     }
   };
 
   return (
     <Fragment>
       <TabList {...props}>
-        {items.map(({ id, tab }, index) => (
+        {items.map(({ id, tab }) => (
           <Tab
             key={id}
-            selected={selectedIndex === index}
-            onClick={() => setSelectedIndex(index)}
+            selected={selectedId === id}
+            onClick={() => setSelectedId(id)}
             id={`tab-${id}`}
             aria-controls={`panel-${id}`}
             onKeyDown={handleTabKeyDown}
@@ -84,23 +89,16 @@ export function Tabs({ initialSelectedIndex = 0, items, ...props }: TabsProps) {
           </Tab>
         ))}
       </TabList>
-      {items.map(({ id, panel }, index) => (
+      {items.map(({ id, panel }) => (
         <TabPanel
           key={id}
           id={`panel-${id}`}
           aria-labelledby={`tab-${id}`}
-          hidden={selectedIndex !== index}
-          ref={tabPanelsRefs[index]}
+          hidden={selectedId !== id}
         >
           {panel}
         </TabPanel>
       ))}
     </Fragment>
   );
 }
-
-function createRefs(length: number) {
-  return Array.from(Array(length).keys()).map(() =>
-    createRef<HTMLDivElement>(),
-  );
-}

packages/circuit-ui/components/Tabs/components/Tab/Tab.spec.tsx

@@ -13,34 +13,25 @@
  * limitations under the License.
  */
 
-import { describe, expect, it, vi } from 'vitest';
+import { describe, expect, it } from 'vitest';
 import { createRef } from 'react';
 
-import { render } from '../../../../util/test-utils.js';
+import { render, screen } from '../../../../util/test-utils.js';
 
 import { Tab } from './Tab.js';
 
 describe('Tab', () => {
   it('should merge a custom class name with the default ones', () => {
     const className = 'foo';
-    const { container } = render(<Tab className={className} />);
-    const element = container.querySelector('button');
-    expect(element?.className).toContain(className);
+    render(<Tab className={className} />);
+    const element = screen.getByRole('tab');
+    expect(element.className).toContain(className);
   });
 
   it('should forward a ref', () => {
     const ref = createRef<HTMLButtonElement>();
-    const { container } = render(<Tab ref={ref} />);
-    const button = container.querySelector('button');
-    expect(ref.current).toBe(button);
-  });
-
-  it('should be focused when selected', () => {
-    const ref = createRef<HTMLButtonElement>();
-    const { container, rerender } = render(<Tab ref={ref} selected={false} />);
-    const button = container.querySelector('button');
-    vi.spyOn(button, 'focus');
-    rerender(<Tab ref={ref} selected />);
-    expect(button?.focus).toHaveBeenCalledOnce();
+    render(<Tab ref={ref} />);
+    const tab = screen.getByRole('tab');
+    expect(ref.current).toBe(tab);
   });
 });

packages/circuit-ui/components/Tabs/components/Tab/Tab.tsx

@@ -19,14 +19,11 @@ import {
   forwardRef,
   type AnchorHTMLAttributes,
   type ButtonHTMLAttributes,
-  useEffect,
-  useRef,
 } from 'react';
 
 import { useComponents } from '../../../ComponentsContext/index.js';
 import type { EmotionAsPropType } from '../../../../types/prop-types.js';
 import { clsx } from '../../../../styles/clsx.js';
-import { applyMultipleRefs } from '../../../../util/refs.js';
 
 import classes from './Tab.module.css';
 
@@ -49,19 +46,13 @@ const tabIndex = (selected: boolean) => (selected ? undefined : -1);
  */
 export const Tab = forwardRef<HTMLButtonElement, TabProps>(
   ({ selected = false, className, ...props }, ref) => {
-    const tabRef = useRef<HTMLButtonElement>(null);
     const components = useComponents();
     const Link = components.Link as EmotionAsPropType;
     const Element = props.href ? Link : 'button';
 
-    useEffect(() => {
-      if (selected) {
-        tabRef?.current?.focus({ preventScroll: true });
-      }
-    }, [selected]);
     return (
       <Element
-        ref={applyMultipleRefs(tabRef, ref)}
+        ref={ref}
         role="tab"
         aria-selected={selected}
         tabIndex={tabIndex(selected)}

packages/circuit-ui/components/Tabs/components/TabList/TabList.tsx

@@ -13,7 +13,7 @@
  * limitations under the License.
  */
 
-import { Children, type HTMLAttributes } from 'react';
+import { Children, forwardRef, type HTMLAttributes } from 'react';
 
 import { clsx } from '../../../../styles/clsx.js';
 import { utilClasses } from '../../../../styles/utility.js';
@@ -29,32 +29,29 @@ const MOBILE_AUTOSTRETCH_ITEMS_MAX = 3;
 /**
  * TabList component that wraps the Tab components
  */
-export function TabList({
-  className,
-  style = {},
-  stretched,
-  children,
-  ...props
-}: TabListProps) {
-  const numberOfTabs = Children.toArray(children).length;
-  const tabWidth = Math.floor(100 / numberOfTabs);
-  const stretchOnMobile = numberOfTabs <= MOBILE_AUTOSTRETCH_ITEMS_MAX;
-  return (
-    <div
-      className={clsx(classes.wrapper, utilClasses.hideScrollbar, className)}
-      style={{ ...style, '--tab-list-width': tabWidth }}
-    >
+export const TabList = forwardRef<HTMLDivElement, TabListProps>(
+  ({ className, style = {}, stretched, children, ...props }, ref) => {
+    const numberOfTabs = Children.toArray(children).length;
+    const tabWidth = Math.floor(100 / numberOfTabs);
+    const stretchOnMobile = numberOfTabs <= MOBILE_AUTOSTRETCH_ITEMS_MAX;
+    return (
       <div
-        className={clsx(
-          classes.base,
-          stretched && classes.stretched,
-          stretchOnMobile && classes['stretched-mobile'],
-        )}
-        {...props}
-        role="tablist"
+        ref={ref}
+        className={clsx(classes.wrapper, utilClasses.hideScrollbar, className)}
+        style={{ ...style, '--tab-list-width': tabWidth }}
       >
-        {children}
+        <div
+          className={clsx(
+            classes.base,
+            stretched && classes.stretched,
+            stretchOnMobile && classes['stretched-mobile'],
+          )}
+          {...props}
+          role="tablist"
+        >
+          {children}
+        </div>
       </div>
-    </div>
-  );
-}
+    );
+  },
+);