[Navigation] Add aria attributes for sub nav items

UNRELEASED.md

@@ -23,6 +23,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 - Updated `Collapsible` to be a functional component ([#3779](https://github.com/Shopify/polaris-react/pull/3779))
 - Coverted `TooltipOverlay` to a functional component ([#3631](https://github.com/Shopify/polaris-react/pull/3631))
 - New `ariaDescribedBy` prop for `Button` ([#3664](https://github.com/Shopify/polaris-react/pull/3686))
+- Changed the way sub navigation menus are rendered for improved accessibility ([#3661](https://github.com/Shopify/polaris-react/pull/3661))
 
 ### Bug fixes
 

playground/DetailsPage.tsx

@@ -239,6 +239,35 @@ export function DetailsPage() {
             },
             matches: navItemActive === 'orders',
             url: '#',
+            subNavigationItems: [
+              {
+                label: 'All orders',
+                onClick: () => {
+                  toggleIsLoading();
+                  setNavItemActive('all-orders');
+                },
+                matches: navItemActive.includes('orders'),
+                url: '#',
+              },
+              {
+                url: '#',
+                label: 'Drafts',
+                onClick: () => {
+                  toggleIsLoading();
+                  setNavItemActive('drafts');
+                },
+                matches: navItemActive === 'drafts',
+              },
+              {
+                url: '#',
+                label: 'Abandoned checkouts',
+                onClick: () => {
+                  toggleIsLoading();
+                  setNavItemActive('abandoned');
+                },
+                matches: navItemActive === 'abandoned',
+              },
+            ],
           },
           {
             label: 'Products',
@@ -261,21 +290,21 @@ export function DetailsPage() {
               },
               {
                 url: '#',
-                label: 'Drafts',
+                label: 'Inventory',
                 onClick: () => {
                   toggleIsLoading();
-                  setNavItemActive('drafts');
+                  setNavItemActive('inventory');
                 },
-                matches: navItemActive === 'drafts',
+                matches: navItemActive === 'inventory',
               },
               {
                 url: '#',
-                label: 'Abandoned checkouts',
+                label: 'Transfers',
                 onClick: () => {
                   toggleIsLoading();
-                  setNavItemActive('abandoned');
+                  setNavItemActive('transfers');
                 },
-                matches: navItemActive === 'abandoned',
+                matches: navItemActive === 'transfers',
               },
             ],
           },

src/components/Navigation/Navigation.scss

@@ -265,10 +265,13 @@ $disabled-fade: 0.6;
 $secondary-item-font-size: rem(15px);
 .SecondaryNavigation {
   flex-basis: 100%;
-  margin-bottom: spacing(tight);
   margin-left: nav(icon-size) + spacing(loose);
   overflow-x: var(--p-override-visible, hidden);
 
+  &.isExpanded {
+    margin-bottom: spacing(tight);
+  }
+
   .Navigation-newDesignLanguage & {
     margin-left: 0;
   }

src/components/Navigation/components/Item/Item.tsx

@@ -16,6 +16,7 @@ import {Indicator} from '../../../Indicator';
 import {UnstyledLink} from '../../../UnstyledLink';
 import {useI18n} from '../../../../utilities/i18n';
 import {useMediaQuery} from '../../../../utilities/media-query';
+import {useUniqueId} from '../../../../utilities/unique-id';
 import styles from '../../Navigation.scss';
 
 import {Secondary} from './components';
@@ -83,6 +84,7 @@ export function Item({
 }: ItemProps) {
   const i18n = useI18n();
   const {isNavigationCollapsed} = useMediaQuery();
+  const secondaryNavigationId = useUniqueId('SecondaryNavigation');
   const {location, onNavigationDismiss} = useContext(NavigationContext);
   const [expanded, setExpanded] = useState(false);
   const [keyFocused, setKeyFocused] = useState(false);
@@ -230,19 +232,20 @@ export function Item({
 
   let secondaryNavigationMarkup: ReactNode = null;
 
-  if (subNavigationItems.length > 0 && showExpanded) {
+  if (subNavigationItems.length > 0) {
     const longestMatch = matchingSubNavigationItems.sort(
       ({url: firstUrl}, {url: secondUrl}) => secondUrl.length - firstUrl.length,
     )[0];
 
     const SecondaryNavigationClassName = classNames(
       styles.SecondaryNavigation,
+      showExpanded && styles.isExpanded,
       !icon && styles['SecondaryNavigation-noIcon'],
     );
 
     secondaryNavigationMarkup = (
       <div className={SecondaryNavigationClassName}>
-        <Secondary expanded={showExpanded}>
+        <Secondary expanded={showExpanded} id={secondaryNavigationId}>
           {subNavigationItems.map((item) => {
             const {label, ...rest} = item;
             return (
@@ -277,6 +280,11 @@ export function Item({
           onClick={getClickHandler(onClick)}
           onKeyUp={handleKeyUp}
           onBlur={handleBlur}
+          {...normalizeAriaAttributes(
+            secondaryNavigationId,
+            subNavigationItems.length > 0,
+            showExpanded,
+          )}
         >
           {itemContentMarkup}
         </UnstyledLink>
@@ -386,3 +394,16 @@ function matchStateForItem(
     : safeStartsWith(location, url);
   return matchesUrl ? MatchState.MatchUrl : MatchState.NoMatch;
 }
+
+function normalizeAriaAttributes(
+  controlId: string,
+  hasSubMenu: boolean,
+  expanded: boolean,
+) {
+  return hasSubMenu
+    ? {
+        'aria-expanded': expanded,
+        'aria-controls': controlId,
+      }
+    : undefined;
+}

src/components/Navigation/components/Item/components/Secondary/Secondary.tsx

@@ -7,12 +7,17 @@ import styles from '../../../../Navigation.scss';
 interface SecondaryProps {
   expanded: boolean;
   children?: React.ReactNode;
+  id?: string;
 }
 
-export function Secondary({children, expanded}: SecondaryProps) {
-  const id = useUniqueId('SecondaryNavigation');
+export function Secondary({id, children, expanded}: SecondaryProps) {
+  const uid = useUniqueId('SecondaryNavigation');
   return (
-    <Collapsible id={id} open={expanded}>
+    <Collapsible
+      id={id || uid}
+      open={expanded}
+      transition={{duration: '0ms', timingFunction: 'linear'}}
+    >
       <ul className={styles.List}>{children}</ul>
     </Collapsible>
   );

src/components/Navigation/components/Item/components/Secondary/tests/Secondary.test.tsx

@@ -1,12 +1,43 @@
 import React from 'react';
-// eslint-disable-next-line no-restricted-imports
-import {mountWithAppProvider} from 'test-utilities/legacy';
+import {mountWithApp} from 'test-utilities';
 
+import {Collapsible} from '../../../../../../Collapsible';
 import {Secondary} from '../Secondary';
 
 describe('Secondary()', () => {
-  it('mounts', () => {
-    const secondary = mountWithAppProvider(<Secondary expanded />);
-    expect(secondary.exists()).toBe(true);
+  it('passes a default id to Collapsible', () => {
+    const component = mountWithApp(<Secondary expanded />);
+    expect(component).toContainReactComponent(Collapsible, {
+      id: 'PolarisSecondaryNavigation1',
+    });
+  });
+
+  it('passes a custom id to Collapsible when provided', () => {
+    const component = mountWithApp(
+      <Secondary expanded id="CustomSecondaryId" />,
+    );
+    expect(component).toContainReactComponent(Collapsible, {
+      id: 'CustomSecondaryId',
+    });
+  });
+
+  it('adds custom transition props to Collapsible', () => {
+    const component = mountWithApp(<Secondary expanded />);
+    expect(component).toContainReactComponent(Collapsible, {
+      transition: {
+        duration: expect.any(String),
+        timingFunction: expect.any(String),
+      },
+    });
+  });
+
+  it('passes expanded to Collapsible', () => {
+    const component = mountWithApp(<Secondary expanded />);
+    expect(component).toContainReactComponent(Collapsible, {open: true});
+  });
+
+  it('renders an unorders list for its children', () => {
+    const component = mountWithApp(<Secondary expanded />);
+    expect(component).toContainReactComponent('ul');
   });
 });