NavList: Fix group heading list item counts

.changeset/fuzzy-lists-count.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+NavList: Exclude group headings from navigation list item counts

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

@@ -3,15 +3,6 @@
 
   &:not(:first-child) {
     margin-block-start: var(--base-size-8);
-
-    /* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading */
-    /* stylelint-disable-next-line selector-max-specificity, selector-pseudo-class-disallowed-list -- scoped to CSS Module, audited (github/github-ui#17224) */
-    &:has(.GroupHeadingWrap + ul > .GroupHeadingWrap) {
-      /* stylelint-disable-next-line selector-max-specificity */
-      & > .GroupHeadingWrap {
-        display: none;
-      }
-    }
   }
 }
 

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

@@ -2,10 +2,11 @@ import {describe, it, expect, vi} from 'vitest'
 import {render, fireEvent, act} from '@testing-library/react'
 import React from 'react'
 import {renderToStaticMarkup} from 'react-dom/server'
-import {NavList} from './NavList'
+import {NavList, type NavListGroupHeadingProps} from './NavList'
 import {ReactRouterLikeLink} from '../Pagination/mocks/ReactRouterLink'
 import {implementsClassName} from '../utils/testing'
 import {FeatureFlags} from '../FeatureFlags'
+import {asSlot} from '../utils/as-slot'
 
 type NextJSLinkProps = {href: string; children: React.ReactNode}
 
@@ -754,6 +755,93 @@ describe('NavList.ShowMoreItem with pages', () => {
   })
 
   describe('NavList.Group', () => {
+    it('renders the group heading outside of the nested list', () => {
+      const {container, getByRole} = render(
+        <NavList>
+          <NavList.Group aria-label="Project templates" data-testid="group">
+            <NavList.GroupHeading as="h2">Project templates</NavList.GroupHeading>
+            <NavList.Item href="#featured">Featured</NavList.Item>
+            <NavList.Item href="#recent">Recent</NavList.Item>
+            <NavList.Item href="#mine">Mine</NavList.Item>
+          </NavList.Group>
+        </NavList>,
+      )
+
+      const group = container.querySelector('[data-testid="group"]')
+      const list = group?.querySelector(':scope > ul')
+      const heading = getByRole('heading', {level: 2, name: 'Project templates'})
+
+      expect(group).not.toBeNull()
+      expect(list).not.toBeNull()
+      expect(list?.children).toHaveLength(3)
+      expect(list).not.toContainElement(heading)
+      expect(heading.parentElement?.parentElement).toBe(group)
+      expect(list).toHaveAttribute('aria-labelledby', heading.id)
+    })
+
+    it('prefers a fragment-wrapped group heading over the title prop', () => {
+      const {container, getByRole, queryByText} = render(
+        <NavList>
+          <NavList.Group title="Overview" data-testid="group">
+            <>
+              <NavList.GroupHeading>Project templates</NavList.GroupHeading>
+            </>
+            <NavList.Item href="#featured">Featured</NavList.Item>
+          </NavList.Group>
+        </NavList>,
+      )
+
+      const group = container.querySelector('[data-testid="group"]')
+      const list = container.querySelector('[data-testid="group"] > ul')
+      const heading = getByRole('heading', {level: 3, name: 'Project templates'})
+
+      expect(queryByText('Overview')).not.toBeInTheDocument()
+      expect(getByRole('heading')).toBe(heading)
+      expect(container.querySelectorAll(`#${CSS.escape(heading.id)}`)).toHaveLength(1)
+      expect(heading.parentElement?.parentElement).toBe(group)
+      expect(list).toHaveAttribute('aria-labelledby', heading.id)
+      expect(list?.children).toHaveLength(1)
+    })
+
+    it('prefers an explicit group heading over the title prop', () => {
+      const {getByRole, queryByText} = render(
+        <NavList>
+          <NavList.Group title="Overview">
+            <NavList.GroupHeading>Project templates</NavList.GroupHeading>
+            <NavList.Item href="#featured">Featured</NavList.Item>
+          </NavList.Group>
+        </NavList>,
+      )
+
+      expect(getByRole('heading', {level: 3, name: 'Project templates'})).toBeInTheDocument()
+      expect(queryByText('Overview')).not.toBeInTheDocument()
+    })
+
+    it('recognizes a group heading wrapper created with asSlot', () => {
+      const WrappedGroupHeading = asSlot(
+        ({children, ...props}: NavListGroupHeadingProps) => (
+          <NavList.GroupHeading {...props}>{children}</NavList.GroupHeading>
+        ),
+        NavList.GroupHeading,
+      )
+      const {container, getByRole, queryByText} = render(
+        <NavList>
+          <NavList.Group title="Overview" data-testid="group">
+            <WrappedGroupHeading id="project-templates-heading">Project templates</WrappedGroupHeading>
+            <NavList.Item href="#featured">Featured</NavList.Item>
+          </NavList.Group>
+        </NavList>,
+      )
+
+      const group = container.querySelector('[data-testid="group"]')
+      const list = group?.querySelector(':scope > ul')
+      const heading = getByRole('heading', {level: 3, name: 'Project templates'})
+
+      expect(queryByText('Overview')).not.toBeInTheDocument()
+      expect(heading.parentElement?.parentElement).toBe(group)
+      expect(list).toHaveAttribute('aria-labelledby', 'project-templates-heading')
+    })
+
     it('renders a divider before the group by default', () => {
       const {container} = render(
         <NavList>

packages/react/src/NavList/NavList.tsx

@@ -22,6 +22,7 @@ import {fixedForwardRef, type PolymorphicProps} from '../utils/modern-polymorphi
 import HeadingComponent from '../Heading'
 import visuallyHiddenClasses from '../_VisuallyHidden.module.css'
 import type {FCWithSlotMarker} from '../utils/types/Slots'
+import {asSlot} from '../utils/as-slot'
 
 type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
 
@@ -366,8 +367,39 @@ export type NavListGroupProps = React.HTMLAttributes<HTMLLIElement> & {
   hideDivider?: boolean
 }
 
+function flattenFragmentChildren(children: React.ReactNode): React.ReactNode[] {
+  const flattenedChildren: React.ReactNode[] = []
+  // eslint-disable-next-line github/array-foreach -- React.Children.toArray changes element identity, which is needed to remove the slotted child from its fragment.
+  React.Children.forEach(children, child => {
+    if (React.isValidElement(child) && child.type === React.Fragment) {
+      flattenedChildren.push(...flattenFragmentChildren((child.props as {children?: React.ReactNode}).children))
+    } else {
+      flattenedChildren.push(child)
+    }
+  })
+  return flattenedChildren
+}
+
+function removeChildFromFragments(children: React.ReactNode, childToRemove: React.ReactElement): React.ReactNode {
+  return React.Children.map(children, child => {
+    if (child === childToRemove) return null
+    if (React.isValidElement(child) && child.type === React.Fragment) {
+      return React.cloneElement(
+        child,
+        undefined,
+        removeChildFromFragments((child.props as {children?: React.ReactNode}).children, childToRemove),
+      )
+    }
+    return child
+  })
+}
+
 const Group: React.FC<NavListGroupProps> = ({title, children, hideDivider, ...props}) => {
   const headingLevel = React.useContext(NavListHeadingLevelContext)
+  const [slots] = useSlots(flattenFragmentChildren(children), {
+    groupHeading: GroupHeading,
+  })
+  const childrenWithoutHeading = slots.groupHeading ? removeChildFromFragments(children, slots.groupHeading) : children
   // Default the group heading to one level below the NavList.Heading (h3 under an
   // h2, h4 under an h3), falling back to h3 when there is no NavList.Heading. To
   // use a different level, pass NavList.GroupHeading with an explicit `as` instead.
@@ -376,12 +408,14 @@ const Group: React.FC<NavListGroupProps> = ({title, children, hideDivider, ...pr
     <>
       {!hideDivider && <ActionList.Divider />}
       <ActionList.Group {...props}>
-        {title ? (
+        {slots.groupHeading ? (
+          React.cloneElement(slots.groupHeading, {headingWrapElement: 'div'})
+        ) : title ? (
           <ActionList.GroupHeading as={groupHeadingAs} data-component="ActionList.GroupHeading">
             {title}
           </ActionList.GroupHeading>
         ) : null}
-        {children}
+        {childrenWithoutHeading}
       </ActionList.Group>
     </>
   )
@@ -495,7 +529,7 @@ export type NavListGroupHeadingProps = ActionListGroupHeadingProps
  * This is an alternative to the `title` prop on `NavList.Group`.
  * It was primarily added to allow links in group headings.
  */
-const GroupHeading: React.FC<NavListGroupHeadingProps> = ({as, className, ...rest}) => {
+const GroupHeadingImpl: React.FC<NavListGroupHeadingProps> = ({as, className, ...rest}) => {
   const headingLevel = React.useContext(NavListHeadingLevelContext)
   const resolvedAs = as ?? (headingLevel ? levelToHeadingTag(headingLevel + 1) : 'h3')
   return (
@@ -509,6 +543,8 @@ const GroupHeading: React.FC<NavListGroupHeadingProps> = ({as, className, ...res
   )
 }
 
+const GroupHeading = asSlot(GroupHeadingImpl, ActionList.GroupHeading)
+
 // ----------------------------------------------------------------------------
 // Export