NavList: Support linked sub-nav parents

.changeset/navlist-linked-subnav.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+NavList: Support linked sub-nav parent items with a separate toggle

packages/react/src/ActionList/ActionList.module.css

@@ -115,7 +115,7 @@
   border-radius: var(--borderRadius-medium);
 
   /* apply flex if trailing action exists as an immediate child */
-  &[data-has-trailing-action] {
+  &[data-has-trailing-action]:not([data-has-subitem='true']) {
     display: flex;
     flex-wrap: nowrap;
   }
@@ -497,6 +497,24 @@
 
 /* button or a tag */
 
+.ActionListItemRow {
+  display: flex;
+  border-radius: var(--borderRadius-medium);
+
+  @media (hover: hover) {
+    &:hover {
+      background-color: var(--control-transparent-bgColor-hover);
+      outline: solid var(--borderWidth-thin) transparent;
+      outline-offset: calc(-1 * var(--borderWidth-thin));
+      box-shadow: var(--boxShadow-thin) var(--control-transparent-borderColor-active);
+    }
+  }
+
+  &:active {
+    background-color: var(--control-transparent-bgColor-active);
+  }
+}
+
 /* [ [spacer] [leadingAction] [leadingVisual] [content] ] */
 .ActionListContent {
   --subitem-depth: 0px;

packages/react/src/ActionList/Item.tsx

@@ -334,67 +334,69 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
         data-trailing-action-loading={trailingActionRendered && slots.trailingAction?.props.loading ? true : undefined}
         className={clsx(classes.ActionListItem, className)}
       >
-        <ConditionalTooltip ref={forwardedRef} text={_PrivateTooltipText ?? truncatedText} enabled={buttonSemantics}>
-          <ItemWrapper
-            {...wrapperProps}
-            className={classes.ActionListContent}
-            data-size={size}
-            // @ts-ignore: ItemWrapper is polymorphic and the ref type depends on the rendered element ('button' or 'li')
-            ref={forwardedRef}
-          >
-            {/* Reset TooltipContext so that child components don't detect
-                the ConditionalTooltip and suppress their own internal tooltips. */}
-            <TooltipContext.Provider value={{}}>
-              <span className={classes.Spacer} />
-              <Selection selected={selected} className={classes.LeadingAction} />
-              <VisualOrIndicator
-                inactiveText={showInactiveIndicator ? inactiveText : undefined}
-                itemHasLeadingVisual={Boolean(slots.leadingVisual)}
-                labelId={labelId}
-                loading={loading}
-                position="leading"
-              >
-                {slots.leadingVisual}
-              </VisualOrIndicator>
-              {/* TODO: next-major: change to data-component="ActionList.Item.DividerContainer" next major version */}
-              <span className={classes.ActionListSubContent} data-component="ActionList.Item--DividerContainer">
-                <ConditionalWrapper
-                  if={!!slots.description}
-                  className={classes.ItemDescriptionWrap}
-                  data-description-variant={descriptionVariant}
-                >
-                  <span id={labelId} className={classes.ItemLabel} data-component="ActionList.Item.Label">
-                    {childrenWithoutSlots}
-                    {/* Loading message needs to be in here so it is read with the label */}
-                    {/* If the item is inactive, we do not simultaneously announce that it is loading */}
-                    {loading === true && !inactive && <VisuallyHidden>Loading</VisuallyHidden>}
-                  </span>
-                  {slots.description}
-                </ConditionalWrapper>
+        <ConditionalWrapper if={Boolean(slots.subItem && trailingActionRendered)} className={classes.ActionListItemRow}>
+          <ConditionalTooltip ref={forwardedRef} text={_PrivateTooltipText ?? truncatedText} enabled={buttonSemantics}>
+            <ItemWrapper
+              {...wrapperProps}
+              className={classes.ActionListContent}
+              data-size={size}
+              // @ts-ignore: ItemWrapper is polymorphic and the ref type depends on the rendered element ('button' or 'li')
+              ref={forwardedRef}
+            >
+              {/* Reset TooltipContext so that child components don't detect
+                  the ConditionalTooltip and suppress their own internal tooltips. */}
+              <TooltipContext.Provider value={{}}>
+                <span className={classes.Spacer} />
+                <Selection selected={selected} className={classes.LeadingAction} />
                 <VisualOrIndicator
                   inactiveText={showInactiveIndicator ? inactiveText : undefined}
                   itemHasLeadingVisual={Boolean(slots.leadingVisual)}
                   labelId={labelId}
                   loading={loading}
-                  position="trailing"
+                  position="leading"
                 >
-                  {trailingVisual}
+                  {slots.leadingVisual}
                 </VisualOrIndicator>
-
-                {
-                  // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel),
-                  // render the inactive warning message directly in the item.
-                  !showInactiveIndicator && inactiveText ? (
-                    <span className={classes.InactiveWarning} id={inactiveWarningId}>
-                      {inactiveText}
+                {/* TODO: next-major: change to data-component="ActionList.Item.DividerContainer" next major version */}
+                <span className={classes.ActionListSubContent} data-component="ActionList.Item--DividerContainer">
+                  <ConditionalWrapper
+                    if={!!slots.description}
+                    className={classes.ItemDescriptionWrap}
+                    data-description-variant={descriptionVariant}
+                  >
+                    <span id={labelId} className={classes.ItemLabel} data-component="ActionList.Item.Label">
+                      {childrenWithoutSlots}
+                      {/* Loading message needs to be in here so it is read with the label */}
+                      {/* If the item is inactive, we do not simultaneously announce that it is loading */}
+                      {loading === true && !inactive && <VisuallyHidden>Loading</VisuallyHidden>}
                     </span>
-                  ) : null
-                }
-              </span>
-            </TooltipContext.Provider>
-          </ItemWrapper>
-        </ConditionalTooltip>
-        {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction}
+                    {slots.description}
+                  </ConditionalWrapper>
+                  <VisualOrIndicator
+                    inactiveText={showInactiveIndicator ? inactiveText : undefined}
+                    itemHasLeadingVisual={Boolean(slots.leadingVisual)}
+                    labelId={labelId}
+                    loading={loading}
+                    position="trailing"
+                  >
+                    {trailingVisual}
+                  </VisualOrIndicator>
+
+                  {
+                    // If the item is inactive, but it's not in an overlay (e.g. ActionMenu, SelectPanel),
+                    // render the inactive warning message directly in the item.
+                    !showInactiveIndicator && inactiveText ? (
+                      <span className={classes.InactiveWarning} id={inactiveWarningId}>
+                        {inactiveText}
+                      </span>
+                    ) : null
+                  }
+                </span>
+              </TooltipContext.Provider>
+            </ItemWrapper>
+          </ConditionalTooltip>
+          {!inactive && !loading && !menuContext && Boolean(slots.trailingAction) && slots.trailingAction}
+        </ConditionalWrapper>
         {slots.subItem}
       </li>
     </ItemContext.Provider>

packages/react/src/NavList/NavList.docs.json

@@ -15,6 +15,7 @@
     {"id": "components-navlist-features--with-group-expand"},
     {"id": "components-navlist-features--with-group-heading-links"},
     {"id": "components-navlist-features--with-inactive-items"},
+    {"id": "components-navlist-features--with-linked-sub-nav-parent"},
     {"id": "components-navlist-features--with-nested-sub-items"},
     {"id": "components-navlist-features--with-next-js-link"},
     {"id": "components-navlist-features--with-react-router-link"},
@@ -95,7 +96,7 @@
           "name": "href",
           "type": "string",
           "defaultValue": "",
-          "description": "The URL that the item navigates to. `href` is passed to the underlying `<a>` element. If `as` is specified, the component may need different props. If the item contains a sub-nav, the item is rendered as a `<button>` and `href` is ignored."
+          "description": "The URL that the item navigates to. `href` is passed to the underlying `<a>` element. If `as` is specified, the component may need different props. If the item contains a sub-nav, the item is rendered as a `<button>` and `href` is ignored unless `subNavToggleLabel` is also set."
         },
         {
           "name": "aria-current",
@@ -108,6 +109,11 @@
           "type": "boolean",
           "description": "The open state of the item when it is initially rendered if the item has a SubNav."
         },
+        {
+          "name": "subNavToggleLabel",
+          "type": "string",
+          "description": "Renders an item with a SubNav as a link with a separate toggle button. The value is used as the toggle button's accessible name."
+        },
         {
           "name": "inactiveText",
           "type": "string",

packages/react/src/NavList/NavList.features.stories.tsx

@@ -53,6 +53,23 @@ export const WithSubItems: StoryFn = () => (
   </PageLayout>
 )
 
+export const WithLinkedSubNavParent: StoryFn = () => (
+  <PageLayout>
+    <PageLayout.Pane position="start">
+      <NavList>
+        <NavList.Item href="#" subNavToggleLabel="Toggle overview pages">
+          Overview
+          <NavList.SubNav>
+            <NavList.Item href="#">Review requested</NavList.Item>
+            <NavList.Item href="#">Your drafts</NavList.Item>
+          </NavList.SubNav>
+        </NavList.Item>
+      </NavList>
+    </PageLayout.Pane>
+    <PageLayout.Content></PageLayout.Content>
+  </PageLayout>
+)
+
 export const WithNestedSubItems: StoryFn = () => (
   <PageLayout>
     <PageLayout.Pane position="start">

packages/react/src/NavList/NavList.test.tsx

@@ -271,6 +271,63 @@ describe('NavList.Item with NavList.SubNav', () => {
     expect(queryByRole('list', {name: 'Item 2'})).toBeNull()
   })
 
+  it('supports a link with a separate SubNav toggle', () => {
+    const onClick = vi.fn((event: React.MouseEvent) => event.preventDefault())
+    const {getByRole, queryByRole} = render(
+      <NavList>
+        <NavList.Item
+          href="/overview"
+          id="overview-link"
+          aria-current="page"
+          className="custom-class"
+          style={{color: 'red'}}
+          subNavToggleLabel="Toggle overview pages"
+          onClick={onClick}
+        >
+          Overview
+          <NavList.SubNav>
+            <NavList.Item href="/overview/review-requested">Review requested</NavList.Item>
+          </NavList.SubNav>
+        </NavList.Item>
+      </NavList>,
+    )
+    const link = getByRole('link', {name: 'Overview'})
+    const toggle = getByRole('button', {name: 'Toggle overview pages'})
+
+    expect(link).toHaveAttribute('href', '/overview')
+    expect(link).toHaveAttribute('id', 'overview-link')
+    expect(link).toHaveAttribute('aria-current', 'page')
+    expect(link).toHaveStyle({color: 'rgb(255, 0, 0)'})
+    expect(link.closest('li')).toHaveClass('custom-class')
+    expect(toggle).toHaveAttribute('aria-expanded', 'false')
+    expect(queryByRole('list', {name: 'Overview'})).toBeNull()
+
+    fireEvent.click(link)
+    expect(onClick).toHaveBeenCalledOnce()
+    expect(toggle).toHaveAttribute('aria-expanded', 'false')
+
+    onClick.mockClear()
+    fireEvent.click(toggle)
+    expect(onClick).not.toHaveBeenCalled()
+    expect(toggle).toHaveAttribute('aria-expanded', 'true')
+    expect(getByRole('list', {name: 'Overview'})).toBeVisible()
+  })
+
+  it('forwards custom link props when using a separate SubNav toggle', () => {
+    const {getByRole} = render(
+      <NavList>
+        <NavList.Item as={ReactRouterLikeLink} to="/overview" subNavToggleLabel="Toggle overview pages">
+          Overview
+          <NavList.SubNav>
+            <NavList.Item href="/overview/review-requested">Review requested</NavList.Item>
+          </NavList.SubNav>
+        </NavList.Item>
+      </NavList>,
+    )
+
+    expect(getByRole('link', {name: 'Overview'})).toHaveAttribute('href', '/overview')
+  })
+
   it('prevents more than 4 levels of nested SubNavs', () => {
     const consoleSpy = vi
       .spyOn(console, 'error')