fix(component): add missing aria roles to tabs (#762)

bigcommerce · Mar 21, 2022 · 7bcc3a0 · 7bcc3a0
1 parent 4ba68b4
commit 7bcc3a0
Show file tree

Hide file tree

Showing 5 changed files with 137 additions and 30 deletions.
diff --git a/packages/big-design/src/components/Tabs/Tabs.tsx b/packages/big-design/src/components/Tabs/Tabs.tsx
@@ -1,8 +1,11 @@
 import React, { HTMLAttributes, memo } from 'react';
 
+import { warning } from '../../utils';
+
 import { StyledTab, StyledTabs } from './styled';
 
 export interface TabItem {
+  ariaControls?: string;
   id: string;
   title: string;
   disabled?: boolean;
@@ -26,12 +29,25 @@ export const Tabs: React.FC<TabsProps> = memo(
       }
     };
 
+    const activeItem = activeTab && items[Number(activeTab)];
+    const missingAriaControls = items.some((item) => !item.ariaControls);
+    const missingFallback =
+      activeItem && !document.getElementById(activeItem.ariaControls || `${activeItem.id}-content`);
+
+    if (missingAriaControls || missingFallback) {
+      warning(
+        'TabItems must have an ariaControls field, otherwise, an element with fallback ID "{tab.id}-content" must exist in the DOM',
+      );
+    }
+
     return (
       <>
         <StyledTabs {...props} flexDirection="row" role="tablist">
-          {items.map(({ id, title, disabled }) => (
+          {items.map(({ ariaControls, id, title, disabled }) => (
             <StyledTab
               activeTab={activeTab}
+              aria-expanded={id === activeTab ? 'true' : 'false'}
+              aria-controls={ariaControls || `${id}-content`}
               id={id}
               key={id}
               onClick={handleOnTabClick}

diff --git a/packages/big-design/src/components/Tabs/__snapshots__/spec.tsx.snap b/packages/big-design/src/components/Tabs/__snapshots__/spec.tsx.snap
@@ -154,6 +154,8 @@ exports[`render Tabs 1`] = `
   role="tablist"
 >
   <button
+    aria-controls="content1"
+    aria-expanded="false"
     class="c3 c4"
     id="tab1"
     role="tab"
@@ -166,6 +168,8 @@ exports[`render Tabs 1`] = `
     </span>
   </button>
   <button
+    aria-controls="content2"
+    aria-expanded="false"
     class="c3 c4"
     id="tab2"
     role="tab"
@@ -334,6 +338,8 @@ exports[`render Tabs with disabled item 1`] = `
   role="tablist"
 >
   <button
+    aria-controls="content1"
+    aria-expanded="false"
     class="c3 c4"
     id="tab1"
     role="tab"
@@ -346,6 +352,8 @@ exports[`render Tabs with disabled item 1`] = `
     </span>
   </button>
   <button
+    aria-controls="content2"
+    aria-expanded="false"
     class="c3 c4"
     id="tab2"
     role="tab"
@@ -358,6 +366,8 @@ exports[`render Tabs with disabled item 1`] = `
     </span>
   </button>
   <button
+    aria-controls="tab3-content"
+    aria-expanded="false"
     class="c3 c4"
     disabled=""
     id="tab3"

diff --git a/packages/big-design/src/components/Tabs/spec.tsx b/packages/big-design/src/components/Tabs/spec.tsx
@@ -1,16 +1,24 @@
+import userEvent from '@testing-library/user-event';
 import React from 'react';
 
-import { fireEvent, render } from '@test/utils';
+import { render, screen } from '@test/utils';
 
 import 'jest-styled-components';
 
+import { warning } from '../../utils';
+
 import { TabItem, Tabs } from './Tabs';
 
 const items: TabItem[] = [
-  { id: 'tab1', title: 'Tab 1' },
-  { id: 'tab2', title: 'Tab 2' },
+  { ariaControls: 'content1', id: 'tab1', title: 'Tab 1' },
+  { ariaControls: 'content2', id: 'tab2', title: 'Tab 2' },
 ];
 
+jest.mock('../../utils', () => ({
+  ...jest.requireActual('../../utils'),
+  warning: jest.fn(),
+}));
+
 test('render Tabs', () => {
   const { container } = render(<Tabs items={items} />);
 
@@ -25,22 +33,22 @@ test('render Tabs with disabled item', () => {
 });
 
 test('Tabs has a role', () => {
-  const { getByRole } = render(<Tabs items={items} />);
+  render(<Tabs items={items} />);
 
-  expect(getByRole('tablist')).toBeInTheDocument();
+  expect(screen.getByRole('tablist')).toBeInTheDocument();
 });
 
 test('Tab items have a role', () => {
-  const { getAllByRole } = render(<Tabs items={items} />);
+  render(<Tabs items={items} />);
 
-  expect(getAllByRole('tab').length).toBe(2);
+  expect(screen.getAllByRole('tab').length).toBe(2);
 });
 
 test('active tab has a tabindex', () => {
   const activeTab = 'tab2';
-  const { getAllByRole } = render(<Tabs activeTab={activeTab} items={items} />);
+  render(<Tabs activeTab={activeTab} items={items} />);
 
-  const tabs = getAllByRole('tab');
+  const tabs = screen.getAllByRole('tab');
 
   tabs.forEach((tab) => {
     if (tab.id === activeTab) {
@@ -53,30 +61,30 @@ test('active tab has a tabindex', () => {
 
 test('onTabClick is called', () => {
   const onClick = jest.fn();
-  const { getByText } = render(<Tabs items={items} onTabClick={onClick} />);
-  const trigger = getByText('Tab 2');
+  render(<Tabs items={items} onTabClick={onClick} />);
+  const trigger = screen.getByText('Tab 2');
 
-  fireEvent.click(trigger);
+  userEvent.click(trigger);
 
   expect(onClick).toHaveBeenCalled();
 });
 
 test('active tab changes on tab click', () => {
   const onClick = jest.fn();
 
-  const { getByText, rerender } = render(<Tabs items={items} onTabClick={onClick} />);
+  const { rerender } = render(<Tabs items={items} onTabClick={onClick} />);
 
-  let trigger = getByText('Tab 2');
+  let trigger = screen.getByText('Tab 2');
 
-  fireEvent.click(trigger);
+  userEvent.click(trigger);
 
   expect(onClick).toHaveBeenCalled();
 
   rerender(<Tabs items={items} onTabClick={onClick} />);
 
-  trigger = getByText('Tab 1');
+  trigger = screen.getByText('Tab 1');
 
-  fireEvent.click(trigger);
+  userEvent.click(trigger);
 
   expect(onClick).toHaveBeenCalled();
 });
@@ -86,11 +94,11 @@ test("clicking a disabled tab doesn't set the active tab", () => {
   const disabledItems: TabItem[] = [...items, { id: 'tab3', title: 'Tab 3', disabled: true }];
   const setActiveTab = jest.fn((tabId: string) => (activeTab = tabId));
 
-  const { getByText } = render(<Tabs activeTab={activeTab} items={disabledItems} onTabClick={setActiveTab} />);
+  render(<Tabs activeTab={activeTab} items={disabledItems} onTabClick={setActiveTab} />);
 
-  const trigger = getByText('Tab 3');
+  const trigger = screen.getByText('Tab 3');
 
-  fireEvent.click(trigger);
+  userEvent.click(trigger);
 
   expect(setActiveTab).not.toHaveBeenCalled();
   expect(activeTab).toBe('tab1');
@@ -102,3 +110,60 @@ test('does not forward styles', () => {
   expect(container.getElementsByClassName('test').length).toBe(0);
   expect(container.firstChild).not.toHaveStyle('background: red');
 });
+
+test('passes the ariaControls prop to the tabs', () => {
+  render(<Tabs className="test" items={items} />);
+
+  const tabs = screen.getAllByRole('tab');
+
+  expect(tabs[0].getAttribute('aria-controls')).toBe('content1');
+  expect(tabs[1].getAttribute('aria-controls')).toBe('content2');
+});
+
+test('active tab has aria-expanded', () => {
+  render(<Tabs activeTab="tab1" items={items} />);
+
+  const tabs = screen.getAllByRole('tab');
+
+  expect(tabs[0].getAttribute('aria-expanded')).toBe('true');
+  expect(tabs[1].getAttribute('aria-expanded')).toBe('false');
+});
+
+test('shows a warning if ariaControls is missing or fallback id does not exist', () => {
+  const warningSpy = jest.fn();
+
+  jest.mocked(warning).mockImplementation(warningSpy);
+
+  const incompleteItems: TabItem[] = [
+    { id: 'tab1', title: 'Tab 1' },
+    { id: 'tab2', title: 'Tab 2' },
+  ];
+  render(<Tabs activeTab="tab1" items={incompleteItems} />);
+
+  expect(warningSpy).toHaveBeenCalled();
+});
+
+test('does not warn if ariaControls is present', () => {
+  const warningSpy = jest.fn();
+
+  jest.mocked(warning).mockImplementation(warningSpy);
+
+  render(<Tabs activeTab="tab1" items={items} />);
+
+  expect(warningSpy).not.toHaveBeenCalled();
+});
+
+test('does not warn if fallback id is valid', () => {
+  const warningSpy = jest.fn();
+
+  jest.mocked(warning).mockImplementation(warningSpy);
+
+  render(
+    <>
+      <Tabs activeTab="tab1" items={items} />
+      <div id="tab1-content">Hello world</div>
+    </>,
+  );
+
+  expect(warningSpy).not.toHaveBeenCalled();
+});
diff --git a/packages/docs/PropTables/TabsPropTable.tsx b/packages/docs/PropTables/TabsPropTable.tsx
@@ -29,6 +29,12 @@ export const TabsPropTable: React.FC<PropTableWrapper> = (props) => (
 );
 
 const tabItemProps: Prop[] = [
+  {
+    name: 'ariaControls',
+    types: 'string',
+    description: 'An ID corresponding to the HTML element where your content will be.',
+    required: false,
+  },
   {
     name: 'id',
     types: 'string',

diff --git a/packages/docs/pages/tabs.tsx b/packages/docs/pages/tabs.tsx
@@ -30,20 +30,26 @@ const TabsPage = () => {
             const [activeTab, setActiveTab] = useState('tab1');
 
             const items = [
-              { id: 'tab1', title: 'Example 1' },
-              { id: 'tab2', title: 'Example 2' },
-              { id: 'tab3', title: 'Example 3' },
-              { id: 'tab4', title: 'Example 4', disabled: true },
+              { ariaControls: 'content1', id: 'tab1', title: 'Example 1' },
+              { ariaControls: 'content2', id: 'tab2', title: 'Example 2' },
+              { ariaControls: 'content3', id: 'tab3', title: 'Example 3' },
+              { ariaControls: 'content4', id: 'tab4', title: 'Example 4', disabled: true },
             ];
 
             return (
               <>
-                <Tabs activeTab={activeTab} items={items} onTabClick={setActiveTab} />
+                <Tabs
+                  aria-label="Example Tab Content"
+                  activeTab={activeTab}
+                  items={items}
+                  id="tab-example"
+                  onTabClick={setActiveTab}
+                />
                 <Box marginTop="large">
-                  {activeTab === 'tab1' && <Text>Content 1</Text>}
-                  {activeTab === 'tab2' && <Text>Content 2</Text>}
-                  {activeTab === 'tab3' && <Text>Content 3</Text>}
-                  {activeTab === 'tab4' && <Text>Content 4</Text>}
+                  {activeTab === 'tab1' && <Text id="content1">Content 1</Text>}
+                  {activeTab === 'tab2' && <Text id="content2">Content 2</Text>}
+                  {activeTab === 'tab3' && <Text id="content3">Content 3</Text>}
+                  {activeTab === 'tab4' && <Text id="content4">Content 4</Text>}
                 </Box>
               </>
             );
@@ -87,6 +93,10 @@ const TabsPage = () => {
             <>
               Use <Code primary>Tabs</Code> to switch between related content.
             </>,
+            <>
+              Use the <Code primary>ariaControls</Code> prop to match your content with the <Code primary>TabItem</Code>{' '}
+              that displays it.
+            </>,
           ]}
           discouraged={[
             <>