chore(a11yaudit): remove unneeded nav/aria-label from Popover, update tests (#7745)

hbuchel · web-flow · commit 2f5f24362637 · 2024-06-18T09:43:58.000-07:00
diff --git a/src/components/GenSwitcher/GenSwitcher.tsx b/src/components/GenSwitcher/GenSwitcher.tsx
@@ -24,11 +24,7 @@ export const GenSwitcher = ({ isGen1, testId }: GenSwitcherProps) => {
           {isGen1 ? 'Gen 1' : 'Gen 2'}
           <VisuallyHidden>Open Amplify generation navigation</VisuallyHidden>
         </Popover.Trigger>
-        <Popover.List
-          ariaLabel="Amplify generation navigation"
-          anchor="left"
-          className="gen-switcher__list"
-        >
+        <Popover.List anchor="left" className="gen-switcher__list">
           <Popover.ListItem href={`/${currentPlatform}`} current={!isGen1}>
             Gen 2 {isGen1 ? '' : <IconCheck className="gen-switcher__check" />}
           </Popover.ListItem>
diff --git a/src/components/GetStartedPopover/GetStartedPopover.tsx b/src/components/GetStartedPopover/GetStartedPopover.tsx
@@ -16,11 +16,13 @@ export type GetStartedLinksType = {
 type GetStartedPopoverType = {
   platform: Platform | typeof DEFAULT_PLATFORM;
   getStartedLinks: GetStartedLinksType[];
+  testId?: string;
 };
 
 export const GetStartedPopover = ({
   platform,
-  getStartedLinks
+  getStartedLinks,
+  testId
 }: GetStartedPopoverType) => {
   const isGen1Page = useIsGen1Page();
 
@@ -47,7 +49,7 @@ export const GetStartedPopover = ({
             Toggle getting started guides navigation
           </VisuallyHidden>
         </Popover.Trigger>
-        <Popover.List ariaLabel="Getting started guides for other platforms">
+        <Popover.List testId={testId ? `${testId}-popoverList` : ''}>
           {getStartedLinks.map((link, index) => {
             return (
               <Popover.ListItem
diff --git a/src/components/GetStartedPopover/__tests__/GetStartedPopover.test.tsx b/src/components/GetStartedPopover/__tests__/GetStartedPopover.test.tsx
@@ -27,6 +27,8 @@ jest.mock('next/router', () => routerMock);
 
 describe('GetStartedPopover', () => {
   const getStartedHref = '/[platform]/start/quickstart/';
+  const testId = 'getStartedTestId';
+  const popoverTestId = `${testId}-popoverList`;
   const getStartedLinks = [
     {
       title: 'React',
@@ -112,7 +114,11 @@ describe('GetStartedPopover', () => {
   ];
 
   const component = (
-    <GetStartedPopover platform={'react'} getStartedLinks={getStartedLinks} />
+    <GetStartedPopover
+      testId={testId}
+      platform={'react'}
+      getStartedLinks={getStartedLinks}
+    />
   );
 
   const componentWithGeneratedLinks = (
@@ -149,9 +155,7 @@ describe('GetStartedPopover', () => {
     const button = await screen.findByRole('button', {
       name: 'Toggle getting started guides navigation'
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: 'Getting started guides for other platforms'
-    });
+    const dropdown = screen.getByTestId(popoverTestId);
 
     expect(dropdown.classList).not.toContain('popover--expanded');
     userEvent.click(button);
@@ -180,9 +184,7 @@ describe('GetStartedPopover', () => {
     const button = await screen.findByRole('button', {
       name: 'Toggle getting started guides navigation'
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: 'Getting started guides for other platforms'
-    });
+    const dropdown = screen.getByTestId(popoverTestId);
 
     userEvent.click(button);
     expect(dropdown.classList).toContain('popover--expanded');
@@ -195,9 +197,7 @@ describe('GetStartedPopover', () => {
     const button = await screen.findByRole('button', {
       name: 'Toggle getting started guides navigation'
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: 'Getting started guides for other platforms'
-    });
+    const dropdown = screen.getByTestId(popoverTestId);
     const platformOptions =
       document.getElementsByClassName('popover-list__link');
 
diff --git a/src/components/PlatformNavigator/__tests__/PlatformNavigator.test.tsx b/src/components/PlatformNavigator/__tests__/PlatformNavigator.test.tsx
@@ -26,8 +26,15 @@ const flatDirectoryMock = {
 jest.mock('@/directory/flatDirectory.json', () => flatDirectoryMock);
 
 describe('PlatformNavigator', () => {
+  const testId = 'platformNavTestId';
+  const popoverTestId = `${testId}-popoverList`;
+
   const component = (
-    <PlatformNavigator currentPlatform={'react'} isGen1={false} />
+    <PlatformNavigator
+      testId={testId}
+      currentPlatform={'react'}
+      isGen1={false}
+    />
   );
 
   it('should render the PlatformNavigator component', async () => {
@@ -71,7 +78,7 @@ describe('PlatformNavigator', () => {
     render(component);
 
     const platformButton = await screen.getByRole('button');
-    const popover = await screen.getByRole('navigation');
+    const popover = screen.getByTestId(popoverTestId);
     const popoverFirstItem = popover.children[0].children[1];
     userEvent.click(platformButton);
     userEvent.tab();
@@ -84,19 +91,31 @@ describe('PlatformNavigator', () => {
   });
 
   it('should use current pathname when platform exists for that path', async () => {
-    render(<PlatformNavigator currentPlatform={'react'} isGen1={false} />);
+    render(
+      <PlatformNavigator
+        testId={testId}
+        currentPlatform={'react'}
+        isGen1={false}
+      />
+    );
 
-    const popover = await screen.getByRole('navigation');
+    const popover = screen.getByTestId(popoverTestId);
     const popoverFirstItem = popover.children[0].children[0];
     expect(popoverFirstItem.children[0].getAttribute('href')).toBe(
       '/react/build-ui/figma-to-code'
     );
   });
 
   it('should use platform root url when platform does not exist for current pathname', async () => {
-    render(<PlatformNavigator currentPlatform={'react'} isGen1={false} />);
+    render(
+      <PlatformNavigator
+        testId={testId}
+        currentPlatform={'react'}
+        isGen1={false}
+      />
+    );
 
-    const popover = await screen.getByRole('navigation');
+    const popover = screen.getByTestId(popoverTestId);
 
     // Flutter
     const popoverFirstItem = popover.children[0].children[6];
diff --git a/src/components/PlatformNavigator/index.tsx b/src/components/PlatformNavigator/index.tsx
@@ -14,11 +14,13 @@ import {
 type PlatformNavigatorProps = {
   currentPlatform: Platform;
   isGen1: boolean;
+  testId?: string;
 };
 
 export function PlatformNavigator({
   currentPlatform,
-  isGen1
+  isGen1,
+  testId
 }: PlatformNavigatorProps) {
   const { pathname } = useRouter();
 
@@ -57,7 +59,7 @@ export function PlatformNavigator({
               {platformTitle}
             </Popover.Trigger>
             <Popover.List
-              ariaLabel="Supported frameworks and languages"
+              testId={testId ? `${testId}-popoverList` : ''}
               anchor="left"
               fullWidth={true}
             >
diff --git a/src/components/Popover/PopoverList.tsx b/src/components/Popover/PopoverList.tsx
@@ -5,15 +5,9 @@ import { usePopover } from './usePopover';
 interface PopoverListProps extends ViewProps {
   anchor?: 'left' | 'right';
   fullWidth?: boolean;
-  /**
-   * @description
-   * Accessible label for the nav item. Required since we are using a <nav> element.
-   */
-  ariaLabel: string;
 }
 
 export const PopoverList = ({
-  ariaLabel,
   children,
   className,
   anchor = 'right',
@@ -33,8 +27,7 @@ export const PopoverList = ({
         },
         className
       )}
-      as="nav"
-      aria-label={ariaLabel}
+      as="div"
       id={navId}
       tabIndex={-1}
       ref={contentRef}
diff --git a/src/components/Popover/__tests__/Popover.test.tsx b/src/components/Popover/__tests__/Popover.test.tsx
@@ -5,12 +5,12 @@ import userEvent from '@testing-library/user-event';
 
 describe('Popover', () => {
   const popoverTestId = 'popoverTestId';
+  const popoverListTestId = 'popoverListTestId';
   const triggerLabel = 'Popover trigger';
-  const navLabel = 'Test list';
   const popover = (
     <Popover testId={popoverTestId}>
       <Popover.Trigger>{triggerLabel}</Popover.Trigger>
-      <Popover.List ariaLabel={navLabel}>
+      <Popover.List testId={popoverListTestId}>
         <Popover.ListItem href="">List item 1</Popover.ListItem>
         <Popover.ListItem href="">List item 2</Popover.ListItem>
         <Popover.ListItem href="">List item 3</Popover.ListItem>
@@ -25,7 +25,7 @@ describe('Popover', () => {
     const popoverTrigger = screen.getByRole('button', {
       name: triggerLabel
     });
-    const popoverList = screen.getByRole('navigation', { name: navLabel });
+    const popoverList = screen.getByTestId(popoverListTestId);
 
     expect(popoverWrapper).toBeInTheDocument();
     expect(popoverTrigger).toBeInTheDocument();
@@ -37,9 +37,7 @@ describe('Popover', () => {
     const button = await screen.findByRole('button', {
       name: triggerLabel
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: navLabel
-    });
+    const dropdown = screen.getByTestId(popoverListTestId);
 
     userEvent.click(button);
     expect(dropdown.classList).toContain('popover--expanded');
@@ -54,9 +52,7 @@ describe('Popover', () => {
     const button = await screen.findByRole('button', {
       name: triggerLabel
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: navLabel
-    });
+    const dropdown = screen.getByTestId(popoverListTestId);
 
     userEvent.click(document.body);
     expect(dropdown.classList).not.toContain('popover--expanded');
@@ -73,9 +69,7 @@ describe('Popover', () => {
     const button = await screen.findByRole('button', {
       name: triggerLabel
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: navLabel
-    });
+    const dropdown = screen.getByTestId(popoverListTestId);
     const externalButton = await screen.findByRole('button', {
       name: 'External button'
     });
@@ -106,9 +100,7 @@ describe('Popover', () => {
     const button = await screen.findByRole('button', {
       name: triggerLabel
     });
-    const dropdown = await screen.findByRole('navigation', {
-      name: navLabel
-    });
+    const dropdown = screen.getByTestId(popoverListTestId);
     const externalButton = await screen.findByRole('button', {
       name: 'External button'
     });
