Update heading levels in the FAQ component (#1053)

joshfarrant · web-flow · commit fe19fe9cde2f · 2025-06-25T13:33:08.000+01:00
* add tests asserting desired heading structure

* update default FAQ.Subheading heading level to h4

* render FAQ.Question as a h5 if there is a FAQ.Subheading present

* update FAQGroup to respect all provided FAQ.Heading props, not just children

* render FAQ.Question as a h5 if nested inside a FAQ.Group

* fix incorrect FAQ.Question heading levels FeaturePreviewLevelTwo playground

* add changeset

* update default FAQ.Subheading level in docs

* update changeset

* ensure FAQ.Question props are respected when there is a FAQ.Subheading

* extract mockSubheading out to own variable in tests

* handle FAQ.Items being wrapped in fragments

* handle the case where FAQ.Items are wrapped in a fragment and an array
diff --git a/.changeset/purple-plants-search.md b/.changeset/purple-plants-search.md
@@ -0,0 +1,9 @@
+---
+'@primer/react-brand': patch
+---
+
+Improved default heading levels in the `FAQ` component.
+
+- Updated `FAQ.Subheading` heading level from `h3` to `h4`.
+- `FAQ.Question` now renders as a `h5` if there is a `FAQ.Subheading` present, or if rendered inside a `FAQGroup`.
+- `FAQGroup` now respects all provided `FAQ.Heading` props, not just children.
diff --git a/apps/docs/content/components/FAQ/react.mdx b/apps/docs/content/components/FAQ/react.mdx
@@ -409,7 +409,7 @@ render(<App />)
 | `children`  | `string`                                                        |         | Sub-heading text                         |
 | `className` | `string`                                                        |         | Sub-heading custom class                 |
 | `id`        | `string`                                                        |         | Sets a custom id                         |
-| `as`        | <PropTableValues values={HeadingTags.slice(1)} addLineBreaks /> | `'h3'`  | Applies the underlying HTML element      |
+| `as`        | <PropTableValues values={HeadingTags.slice(1)} addLineBreaks /> | `'h4'`  | Applies the underlying HTML element      |
 | `ref`       | `React.RefObject`                                               |         | Forward a Ref to the underlying DOM node |
 
 <h3 id="faq-item">FAQ.Item</h3>
diff --git a/apps/next-docs/content/components/FAQ/react.mdx b/apps/next-docs/content/components/FAQ/react.mdx
@@ -386,7 +386,7 @@ render(<App />)
 | `children`  | `string`                |         | Sub-heading text                         |
 | `className` | `string`                |         | Sub-heading custom class                 |
 | `id`        | `string`                |         | Sets a custom id                         |
-| `as`        | <FAQSubheadingAsProp /> | `'h3'`  | Applies the underlying HTML element      |
+| `as`        | <FAQSubheadingAsProp /> | `'h4'`  | Applies the underlying HTML element      |
 | `ref`       | `React.RefObject`       |         | Forward a Ref to the underlying DOM node |
 
 <h3 id="faq-item">FAQ.Item</h3>
diff --git a/packages/react/src/FAQ/FAQ.test.tsx b/packages/react/src/FAQ/FAQ.test.tsx
@@ -10,6 +10,7 @@ describe('FAQ', () => {
   const questionRoot = 'question-root'
 
   const mockHeading = 'This is a mock heading'
+  const mockSubheading = 'this is a mock subheading'
   const mockQuestion = 'What is a mock question?'
   const mockFAQAnswer = 'mock answer'
 
@@ -137,7 +138,6 @@ describe('FAQ', () => {
   })
 
   it('can render groups of FAQs using an optional sub-heading', () => {
-    const mockSubheading = 'this is a mock subheading'
     const invalidChild = <div>This is an invalid child</div>
     const {getByRole} = render(
       <FAQ>
@@ -153,13 +153,12 @@ describe('FAQ', () => {
       </FAQ>,
     )
 
-    const subheadingEl = getByRole('heading', {level: 3, name: mockSubheading})
+    const subheadingEl = getByRole('heading', {level: 4, name: mockSubheading})
 
     expect(subheadingEl).toBeInTheDocument()
   })
 
-  it('renders alternative headling levels', () => {
-    const mockSubheading = 'this is a mock subheading'
+  it('renders alternative heading levels', () => {
     const {getByRole} = render(
       <FAQ>
         <FAQ.Heading as="h3">{mockHeading}</FAQ.Heading>
@@ -181,4 +180,96 @@ describe('FAQ', () => {
     expect(subheadingEl).toBeInTheDocument()
     expect(questionheadingEl).toBeInTheDocument()
   })
+
+  it('renders the heading as a h3 and the question as a h4 by default', () => {
+    const {getByRole} = render(
+      <FAQ>
+        <FAQ.Heading>{mockHeading}</FAQ.Heading>
+        <FAQ.Item>
+          <FAQ.Question>{mockQuestion}</FAQ.Question>
+          <FAQ.Answer>
+            <p>{mockFAQAnswer}</p>
+          </FAQ.Answer>
+        </FAQ.Item>
+      </FAQ>,
+    )
+
+    const mainheadingEl = getByRole('heading', {level: 3, name: mockHeading})
+    const questionheadingEl = getByRole('heading', {level: 4, name: mockQuestion})
+
+    expect(mainheadingEl).toBeInTheDocument()
+    expect(questionheadingEl).toBeInTheDocument()
+  })
+
+  it('renders the heading as a h3, the subheading as a h4, and the question as a h5 when there is a subheading present', () => {
+    const {getByRole} = render(
+      <FAQ>
+        <FAQ.Heading>{mockHeading}</FAQ.Heading>
+        <FAQ.Subheading>{mockSubheading}</FAQ.Subheading>
+        <FAQ.Item>
+          <FAQ.Question>{mockQuestion}</FAQ.Question>
+          <FAQ.Answer>
+            <p>{mockFAQAnswer}</p>
+          </FAQ.Answer>
+        </FAQ.Item>
+      </FAQ>,
+    )
+
+    const mainheadingEl = getByRole('heading', {level: 3, name: mockHeading})
+    const subheadingEl = getByRole('heading', {level: 4, name: mockSubheading})
+    const questionheadingEl = getByRole('heading', {level: 5, name: mockQuestion})
+
+    expect(mainheadingEl).toBeInTheDocument()
+    expect(subheadingEl).toBeInTheDocument()
+    expect(questionheadingEl).toBeInTheDocument()
+  })
+
+  it('renders the heading as a h3, the subheading as a h4, and the question as a h5 when there is a subheading present, and the FAQ.Item is wrapped in a Fragment', () => {
+    const {getByRole} = render(
+      <FAQ>
+        <FAQ.Heading>{mockHeading}</FAQ.Heading>
+        <FAQ.Subheading>{mockSubheading}</FAQ.Subheading>
+        <>
+          <FAQ.Item key="item-1">
+            <FAQ.Question>{mockQuestion}</FAQ.Question>
+            <FAQ.Answer>
+              <p>{mockFAQAnswer}</p>
+            </FAQ.Answer>
+          </FAQ.Item>
+        </>
+      </FAQ>,
+    )
+
+    const mainheadingEl = getByRole('heading', {level: 3, name: mockHeading})
+    const subheadingEl = getByRole('heading', {level: 4, name: mockSubheading})
+    const questionheadingEl = getByRole('heading', {level: 5, name: mockQuestion})
+
+    expect(mainheadingEl).toBeInTheDocument()
+    expect(subheadingEl).toBeInTheDocument()
+    expect(questionheadingEl).toBeInTheDocument()
+  })
+
+  it('renders the heading as a h3, the subheading as a h4, and the question as a h5 when there is a subheading present, FAQ.Items are wrapped in a Fragment, and all FAQ.Items are wrapped in an array', () => {
+    const {getByRole} = render(
+      <FAQ>
+        <FAQ.Heading>{mockHeading}</FAQ.Heading>
+        <FAQ.Subheading>{mockSubheading}</FAQ.Subheading>
+        <>
+          {[1, 2].map(item => {
+            return (
+              <FAQ.Item key={`item-${item}`}>
+                <FAQ.Question>Question {item}</FAQ.Question>
+                <FAQ.Answer>Answer {item}</FAQ.Answer>
+              </FAQ.Item>
+            )
+          })}
+        </>
+      </FAQ>,
+    )
+
+    expect(getByRole('heading', {level: 3, name: mockHeading})).toBeInTheDocument()
+    expect(getByRole('heading', {level: 4, name: mockSubheading})).toBeInTheDocument()
+    expect(getByRole('heading', {level: 5, name: 'Question 1'})).toBeInTheDocument()
+    expect(getByRole('heading', {level: 5, name: 'Question 2'})).toBeInTheDocument()
+  })
 })
diff --git a/packages/react/src/FAQ/FAQ.tsx b/packages/react/src/FAQ/FAQ.tsx
@@ -22,19 +22,30 @@ export type FAQRootProps = PropsWithChildren<BaseProps<HTMLElement>> & React.HTM
 const FAQRoot = forwardRef<HTMLElement, FAQRootProps>(({children, style, animate, className, ...rest}, ref) => {
   const {classes: animationClasses, styles: animationInlineStyles} = useAnimation(animate)
 
-  const filteredChildren = React.Children.toArray(children).filter(child => {
-    if (React.isValidElement(child) && typeof child.type !== 'string') {
-      if (
-        isFragment(child) ||
-        (child as React.ReactElement).type === FAQHeading ||
-        (child as React.ReactElement).type === FAQSubheading ||
-        (child as React.ReactElement).type === AccordionRoot
-      ) {
-        return true
+  const filteredChildren = React.Children.toArray(children)
+    .filter(child => {
+      if (React.isValidElement(child) && typeof child.type !== 'string') {
+        if (
+          isFragment(child) ||
+          (child as React.ReactElement).type === FAQHeading ||
+          (child as React.ReactElement).type === FAQSubheading ||
+          (child as React.ReactElement).type === AccordionRoot
+        ) {
+          return true
+        }
       }
-    }
-    return false
-  })
+      return false
+    })
+    .map(childMaybeFragment => {
+      /**
+       * If children are wrapped in a Fragment, extract the children.
+       * Children may also be an array of FAQ.Item components, so we call
+       * `toArray` to ensure we always have an array.
+       */
+      return React.Children.toArray(
+        isFragment(childMaybeFragment) ? childMaybeFragment.props.children : childMaybeFragment,
+      )
+    })
 
   const hasSubheading = React.Children.toArray(children).some(
     child => React.isValidElement(child) && typeof child.type !== 'string' && child.type === FAQSubheading,
@@ -47,17 +58,39 @@ const FAQRoot = forwardRef<HTMLElement, FAQRootProps>(({children, style, animate
       style={{...animationInlineStyles, ...style}}
       {...rest}
     >
-      {React.Children.toArray(filteredChildren).map(child => {
-        if (React.isValidElement(child) && typeof child.type !== 'string') {
+      {filteredChildren.map(childArr => {
+        return childArr.map(child => {
+          if (!React.isValidElement(child) || typeof child.type === 'string') {
+            return child
+          }
+
           if (child.type === FAQHeading) {
             return React.cloneElement(child as React.ReactElement, {
               align: hasSubheading ? 'start' : child.props.align,
               size: hasSubheading ? '3' : child.props.size,
               weight: hasSubheading ? 'semibold' : child.props.weight,
             })
           }
-        }
-        return child
+
+          // If there is a subheading, ensure that the FAQ.Question is rendered as a h5
+          if (hasSubheading && child.type === FAQ.Item) {
+            const grandChildren = React.Children.map(child.props.children, grandChild => {
+              if (grandChild.type === FAQ.Question) {
+                return React.cloneElement(grandChild as React.ReactElement, {
+                  as: 'h5',
+                  ...grandChild.props,
+                })
+              }
+              return grandChild
+            })
+
+            return React.cloneElement(child as React.ReactElement, {
+              children: grandChildren,
+            })
+          }
+
+          return child
+        })
       })}
     </section>
   )
@@ -95,7 +128,7 @@ export type FAQSubheadingProps = BaseProps<HTMLHeadingElement> & {
 function FAQSubheading({
   children,
   className,
-  as = 'h3',
+  as = 'h4',
   size = 'subhead-large',
   weight = 'medium',
   ...rest
diff --git a/packages/react/src/FAQ/FAQGroup.test.tsx b/packages/react/src/FAQ/FAQGroup.test.tsx
@@ -163,4 +163,23 @@ describe('FAQGroup', () => {
     expect(getByRole('tab', {name: 'mock heading 2'})).toHaveAttribute('data-tab-heading', 'mock heading 2')
     expect(getByRole('tab', {name: 'mock heading 3'})).toHaveAttribute('data-tab-heading', 'mock heading 3')
   })
+
+  it('renders the FAQGroup.Heading as a h3, the FAQ.Heading as a h4, and the question as a h5', () => {
+    const {getByRole, getAllByRole} = render(<Component />)
+
+    const faqGroupHeading = getByRole('heading', {level: 3, name: 'Frequently asked questions'})
+    const questionHeading = getByRole('heading', {level: 5, name: 'mock question 1'})
+
+    expect(faqGroupHeading).toBeInTheDocument()
+    expect(questionHeading).toBeInTheDocument()
+
+    /**
+     * We use getAllByRole here as we render two h4 elements with the same text content and hide one with media queries.
+     * Since our test suite doesn't render styles, Testing Library finds both of them, so we check both even though one is hidden.
+     */
+    const faqHeadings = getAllByRole('heading', {level: 4, name: 'mock heading 1'})
+    for (const faqHeading of faqHeadings) {
+      expect(faqHeading).toBeInTheDocument()
+    }
+  })
 })
diff --git a/packages/react/src/FAQ/FAQGroup.tsx b/packages/react/src/FAQ/FAQGroup.tsx
@@ -106,13 +106,30 @@ function _FAQGroup({children, id, defaultSelectedIndex = 0, tabAttributes, ...re
 
   const TabPanels = React.Children.map(faqChildren, (faqChild, index) => {
     if (React.isValidElement<FAQRootProps>(faqChild) && faqChild.props.children) {
-      const FAQItemChild = React.Children.map(faqChild.props.children, child =>
-        React.isValidElement(child) && child.type !== FAQ.Heading ? child : null,
-      )
+      const FAQItemChild = React.Children.map(faqChild.props.children, child => {
+        if (!React.isValidElement(child) || child.type === FAQ.Heading) {
+          return null
+        }
+
+        // Make sure that the FAQ.Question is rendered as a h5
+        const grandChildren = React.Children.map(child.props.children, grandChild => {
+          if (grandChild.type === FAQ.Question) {
+            return React.cloneElement(grandChild as React.ReactElement, {
+              as: 'h5',
+              ...grandChild.props,
+            })
+          }
+          return grandChild
+        })
 
-      const FAQItemHeadingText = React.Children.map(faqChild.props.children, child =>
-        React.isValidElement(child) && child.type === FAQ.Heading ? child.props.children : null,
-      )
+        return React.cloneElement(child as React.ReactElement, {
+          children: grandChildren,
+        })
+      })
+
+      const FAQItemHeading = React.Children.toArray(faqChild.props.children).find(
+        child => React.isValidElement(child) && child.type === FAQ.Heading,
+      ) as ReactElement | undefined
 
       return (
         <div
@@ -123,13 +140,12 @@ function _FAQGroup({children, id, defaultSelectedIndex = 0, tabAttributes, ...re
           key={index}
           data-testid={`FAQGroup-tab-panel-${index + 1}`}
         >
-          {FAQItemHeadingText && (
+          {FAQItemHeading && (
             <FAQ.Subheading
+              {...FAQItemHeading.props}
               data-testid={`FAQGroup-tab-panel-heading-${index + 1}`}
-              className={clsx(styles['FAQGroup__panel-subHeading'])}
-            >
-              {FAQItemHeadingText}
-            </FAQ.Subheading>
+              className={clsx(styles['FAQGroup__panel-subHeading'], FAQItemHeading.props.className)}
+            />
           )}
           {FAQItemChild}
         </div>
diff --git a/packages/react/src/recipes/FeaturePreviewLPs/FeaturePreviewLevelTwo/FeaturePreviewLevelTwo.tsx b/packages/react/src/recipes/FeaturePreviewLPs/FeaturePreviewLevelTwo/FeaturePreviewLevelTwo.tsx
