fix(ui-react): Whitespace in Tab IDs (#5378)

* fix: whitespace in tabs, validate value & defaultValue types * chore: added changeset * fix: creates const idValue to avoid changing inputted values * fix: removed unused import, type-checked before .replace * fix: called functions in test * fix: updated ComposedTabsExample.tsx
aws-amplify · Aug 6, 2024 · e52db7b · e52db7b
1 parent ba17d38
commit e52db7b
Show file tree

Hide file tree

Showing 8 changed files with 87 additions and 10 deletions.
diff --git a/.changeset/tall-olives-beam.md b/.changeset/tall-olives-beam.md
@@ -0,0 +1,5 @@
+---
+'@aws-amplify/ui-react': patch
+---
+
+fixes invalid tab IDs
diff --git a/docs/src/pages/[platform]/components/tabs/examples/ComposedTabsExample.tsx b/docs/src/pages/[platform]/components/tabs/examples/ComposedTabsExample.tsx
@@ -11,9 +11,9 @@ export const ComposedTabsExample = () => {
         </Tabs.Item>
       </Tabs.List>
       <Tabs.Panel value="Tab 1">Tab 1 content</Tabs.Panel>
-      <Tabs.Panel value="Tab 2">Tab 1 content</Tabs.Panel>
+      <Tabs.Panel value="Tab 2">Tab 2 content</Tabs.Panel>
       <Tabs.Panel value="Tab 3" isDisabled>
-        Tab 1 content
+        Tab 3 content
       </Tabs.Panel>
     </Tabs.Container>
   );

diff --git a/packages/react/src/primitives/Tabs/TabsContainer.tsx b/packages/react/src/primitives/Tabs/TabsContainer.tsx
@@ -7,6 +7,7 @@ import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';
 import { BaseTabsProps, TabsProps } from './types';
 import { View } from '../View';
 import { TabsContext } from './TabsContext';
+import { useStableId } from '../utils/useStableId';
 
 const TabsContainerPrimitive: Primitive<TabsProps, 'div'> = (
   {
@@ -20,6 +21,7 @@ const TabsContainerPrimitive: Primitive<TabsProps, 'div'> = (
   }: BaseTabsProps,
   ref
 ) => {
+  const groupId = useStableId(); // groupId is used to ensure uniqueness between Tab Groups in IDs
   const isControlled = controlledValue !== undefined;
   const [localValue, setLocalValue] = React.useState(() =>
     isControlled ? controlledValue : defaultValue
@@ -44,8 +46,9 @@ const TabsContainerPrimitive: Primitive<TabsProps, 'div'> = (
       activeTab,
       isLazy,
       setActiveTab,
+      groupId,
     };
-  }, [activeTab, setActiveTab, isLazy]);
+  }, [activeTab, setActiveTab, isLazy, groupId]);
 
   return (
     <TabsContext.Provider value={_value}>

diff --git a/packages/react/src/primitives/Tabs/TabsContext.tsx b/packages/react/src/primitives/Tabs/TabsContext.tsx
@@ -3,10 +3,12 @@ import * as React from 'react';
 export interface TabsContextInterface {
   activeTab: string;
   isLazy?: boolean;
+  groupId: string;
   setActiveTab: (tab: string) => void;
 }
 
 export const TabsContext = React.createContext<TabsContextInterface>({
+  groupId: '',
   activeTab: '',
   setActiveTab: () => {},
 });
diff --git a/packages/react/src/primitives/Tabs/TabsItem.tsx b/packages/react/src/primitives/Tabs/TabsItem.tsx
@@ -12,28 +12,32 @@ import { View } from '../View';
 import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';
 import { BaseTabsItemProps, TabsItemProps } from './types';
 import { TabsContext } from './TabsContext';
+import { WHITESPACE_VALUE } from './constants';
 
 const TabsItemPrimitive: Primitive<TabsItemProps, 'button'> = (
   { className, value, children, onClick, as = 'button', role = 'tab', ...rest },
   ref
 ) => {
-  const { activeTab, setActiveTab } = React.useContext(TabsContext);
+  const { activeTab, setActiveTab, groupId } = React.useContext(TabsContext);
+  let idValue = value;
+  if (typeof idValue === 'string') {
+    idValue = idValue.replace(' ', WHITESPACE_VALUE);
+  }
   const isActive = activeTab === value;
   const handleOnClick = (e: React.MouseEvent<HTMLButtonElement>) => {
     if (isTypedFunction(onClick)) {
       onClick?.(e);
     }
     setActiveTab(value);
   };
-
   return (
     <View
       {...rest}
       role={role}
       as={as}
-      id={`${value}-tab`}
+      id={`${groupId}-tab-${idValue}`}
       aria-selected={isActive}
-      aria-controls={`${value}-panel`}
+      aria-controls={`${groupId}-panel-${idValue}`}
       tabIndex={!isActive ? -1 : undefined}
       className={classNames(
         ComponentClassName.TabsItem,

diff --git a/packages/react/src/primitives/Tabs/TabsPanel.tsx b/packages/react/src/primitives/Tabs/TabsPanel.tsx
@@ -8,21 +8,27 @@ import { View } from '../View';
 import { BaseTabsPanelProps, TabsPanelProps } from './types';
 import { primitiveWithForwardRef } from '../utils/primitiveWithForwardRef';
 import { TabsContext } from './TabsContext';
+import { WHITESPACE_VALUE } from './constants';
 
 const TabPanelPrimitive: Primitive<TabsPanelProps, 'div'> = (
   { className, value, children, role = 'tabpanel', ...rest },
   ref
 ) => {
-  const { activeTab, isLazy } = React.useContext(TabsContext);
+  const { activeTab, isLazy, groupId } = React.useContext(TabsContext);
 
   if (isLazy && activeTab !== value) return null;
 
+  let idValue = value;
+  if (typeof idValue === 'string') {
+    idValue = idValue.replace(' ', WHITESPACE_VALUE);
+  }
+
   return (
     <View
       {...rest}
       role={role}
-      id={`${value}-panel`}
-      aria-labelledby={`${value}-tab`}
+      id={`${groupId}-panel-${idValue}`}
+      aria-labelledby={`${groupId}-tab-${idValue}`}
       className={classNames(
         ComponentClassName.TabsPanel,
         classNameModifierByFlag(

diff --git a/packages/react/src/primitives/Tabs/__tests__/Tabs.test.tsx b/packages/react/src/primitives/Tabs/__tests__/Tabs.test.tsx
@@ -86,6 +86,61 @@ describe('Tabs', () => {
     expect(tabs[1]).toHaveAttribute('aria-selected', 'true');
   });
 
+  it('creates unique IDs for two tabs with same value in different tab groups"', async () => {
+    render(
+      <Tabs.Container>
+        <Tabs.List>
+          <Tabs.Item value="Tab 1">Tab 1</Tabs.Item>
+        </Tabs.List>
+      </Tabs.Container>
+    );
+    render(
+      <Tabs.Container>
+        <Tabs.List>
+          <Tabs.Item value="Tab 1">Tab 1</Tabs.Item>
+        </Tabs.List>
+      </Tabs.Container>
+    );
+    const tabs = await screen.findAllByRole('tab');
+    expect(tabs[0].id === tabs[1].id).toBeFalsy();
+  });
+
+  it('creates unique ids for each tab with a unique value', async () => {
+    render(
+      <Tabs.Container testId="tabsTest">
+        <Tabs.List>
+          <Tabs.Item value="1">Tab 1</Tabs.Item>
+          <Tabs.Item value="2">Tab 2</Tabs.Item>
+          <Tabs.Item value="3">Tab 3</Tabs.Item>
+        </Tabs.List>
+        <Tabs.Panel value="1">Tab 1</Tabs.Panel>
+        <Tabs.Panel value="2">Tab 2</Tabs.Panel>
+        <Tabs.Panel value="3">Tab 3</Tabs.Panel>
+      </Tabs.Container>
+    );
+    const tabs = await screen.findAllByRole('tab');
+    expect(tabs[0].id === tabs[1].id).toBeFalsy();
+    expect(tabs[0].id === tabs[2].id).toBeFalsy();
+  });
+
+  it('creates the same ids tabs with the same value in the same group', async () => {
+    render(
+      <Tabs.Container testId="tabsTest">
+        <Tabs.List>
+          <Tabs.Item value="1">Tab 1</Tabs.Item>
+          <Tabs.Item value="1">Tab 2</Tabs.Item>
+          <Tabs.Item value="1">Tab 3</Tabs.Item>
+        </Tabs.List>
+        <Tabs.Panel value="1">Tab 1</Tabs.Panel>
+        <Tabs.Panel value="1">Tab 2</Tabs.Panel>
+        <Tabs.Panel value="1">Tab 3</Tabs.Panel>
+      </Tabs.Container>
+    );
+    const tabs = await screen.findAllByRole('tab');
+    expect(tabs[0].id === tabs[1].id).toBeTruthy();
+    expect(tabs[0].id === tabs[2].id).toBeTruthy();
+  });
+
   describe('TabItem', () => {
     it('can render custom classnames', async () => {
       render(

diff --git a/packages/react/src/primitives/Tabs/constants.ts b/packages/react/src/primitives/Tabs/constants.ts
@@ -0,0 +1,2 @@
+/* WHITESPACE_VALUE is used to fill whitespace present in user-inputed `value` when creating id for TabsItem and TabsPanel */
+export const WHITESPACE_VALUE = '-';
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		/* WHITESPACE_VALUE is used to fill whitespace present in user-inputed `value` when creating id for TabsItem and TabsPanel */
		export const WHITESPACE_VALUE = '-';