feat(ActionList.Heading): Convert to CSS Modules (#5367)

* heading * lint * Create new-pans-shout.md * use size prop * test(vrt): update snapshots * Update e2e/components/ActionList.test.ts * test(vrt): update snapshots * Update packages/react/src/ActionList/Heading.test.tsx --------- Co-authored-by: langermank <[email protected]> Co-authored-by: Jon Rohan <[email protected]> Co-authored-by: jonrohan <[email protected]>
primer · Dec 6, 2024 · 844e41f · 844e41f
1 parent 3e0fc7c
commit 844e41f
Show file tree

Hide file tree

Showing 17 changed files with 196 additions and 58 deletions.
diff --git a/.changeset/new-pans-shout.md b/.changeset/new-pans-shout.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": minor
+---
+
+Convert ActionList.Heading to CSS Modules
diff --git a/...s/ActionList.test.ts-snapshots/Heading-with-Classname-dark-colorblind-linux.png b/...s/ActionList.test.ts-snapshots/Heading-with-Classname-dark-colorblind-linux.png
diff --git a/...nents/ActionList.test.ts-snapshots/Heading-with-Classname-dark-dimmed-linux.png b/...nents/ActionList.test.ts-snapshots/Heading-with-Classname-dark-dimmed-linux.png
diff --git a/...ctionList.test.ts-snapshots/Heading-with-Classname-dark-high-contrast-linux.png b/...ctionList.test.ts-snapshots/Heading-with-Classname-dark-high-contrast-linux.png
diff --git a/...s/components/ActionList.test.ts-snapshots/Heading-with-Classname-dark-linux.png b/...s/components/ActionList.test.ts-snapshots/Heading-with-Classname-dark-linux.png
diff --git a/...s/ActionList.test.ts-snapshots/Heading-with-Classname-dark-tritanopia-linux.png b/...s/ActionList.test.ts-snapshots/Heading-with-Classname-dark-tritanopia-linux.png
diff --git a/.../ActionList.test.ts-snapshots/Heading-with-Classname-light-colorblind-linux.png b/.../ActionList.test.ts-snapshots/Heading-with-Classname-light-colorblind-linux.png
diff --git a/...tionList.test.ts-snapshots/Heading-with-Classname-light-high-contrast-linux.png b/...tionList.test.ts-snapshots/Heading-with-Classname-light-high-contrast-linux.png
diff --git a/.../components/ActionList.test.ts-snapshots/Heading-with-Classname-light-linux.png b/.../components/ActionList.test.ts-snapshots/Heading-with-Classname-light-linux.png
diff --git a/.../ActionList.test.ts-snapshots/Heading-with-Classname-light-tritanopia-linux.png b/.../ActionList.test.ts-snapshots/Heading-with-Classname-light-tritanopia-linux.png
diff --git a/e2e/components/ActionList.test.ts b/e2e/components/ActionList.test.ts
@@ -712,4 +712,32 @@ test.describe('ActionList', () => {
       })
     }
   })
+
+  test.describe('Heading with Classname', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-actionlist-dev--heading-custom-classname',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`Heading with Classname.${theme}.png`)
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-actionlist-dev--heading-custom-classname',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations()
+        })
+      })
+    }
+  })
 })
diff --git a/packages/react/src/ActionList/ActionList.dev.stories.tsx b/packages/react/src/ActionList/ActionList.dev.stories.tsx
@@ -116,3 +116,16 @@ export const GroupHeadingCustomClassname = () => (
     </ActionList.Group>
   </ActionList>
 )
+
+export const HeadingCustomClassname = () => (
+  <ActionList>
+    <ActionList.Heading className="testCustomClassnameColor" as="h2">
+      Filter by
+    </ActionList.Heading>
+    <ActionList.Group>
+      <ActionList.GroupHeading as="h3">Repositories</ActionList.GroupHeading>
+      <ActionList.Item onClick={() => {}}>app/assets/modules</ActionList.Item>
+      <ActionList.Item onClick={() => {}}>src/react/components</ActionList.Item>
+    </ActionList.Group>
+  </ActionList>
+)
diff --git a/packages/react/src/ActionList/ActionList.features.stories.tsx b/packages/react/src/ActionList/ActionList.features.stories.tsx
@@ -50,7 +50,9 @@ export const SimpleList = () => (
 
 export const WithVisualListHeading = () => (
   <ActionList>
-    <ActionList.Heading as="h2">Filter by</ActionList.Heading>
+    <ActionList.Heading as="h2" size="small">
+      Filter by
+    </ActionList.Heading>
     <ActionList.Group>
       <ActionList.GroupHeading as="h3">Repositories</ActionList.GroupHeading>
       <ActionList.Item onClick={() => {}}>

diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx
@@ -237,52 +237,6 @@ describe('ActionList', () => {
     expect(onClick).toHaveBeenCalled()
   })
 
-  it('should render the ActionList.Heading component as a heading with the given heading level', async () => {
-    const container = HTMLRender(
-      <ActionList>
-        <ActionList.Heading as="h1">Heading</ActionList.Heading>
-      </ActionList>,
-    )
-    const heading = container.getByRole('heading', {level: 1})
-    expect(heading).toBeInTheDocument()
-    expect(heading).toHaveTextContent('Heading')
-  })
-  it('should label the action list with the heading id', async () => {
-    const {container, getByRole} = HTMLRender(
-      <ActionList>
-        <ActionList.Heading as="h1">Heading</ActionList.Heading>
-        <ActionList.Item>Item</ActionList.Item>
-      </ActionList>,
-    )
-    const list = container.querySelector('ul')
-    const heading = getByRole('heading', {level: 1})
-    expect(list).toHaveAttribute('aria-labelledby', heading.id)
-  })
-  it('should throw an error when ActionList.Heading is used within ActionMenu context', async () => {
-    const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
-    expect(() =>
-      HTMLRender(
-        <ThemeProvider theme={theme}>
-          <BaseStyles>
-            <ActionMenu open={true}>
-              <ActionMenu.Button>Trigger</ActionMenu.Button>
-              <ActionMenu.Overlay>
-                <ActionList>
-                  <ActionList.Heading as="h1">Heading</ActionList.Heading>
-                  <ActionList.Item>Item</ActionList.Item>
-                </ActionList>
-              </ActionMenu.Overlay>
-            </ActionMenu>
-          </BaseStyles>
-        </ThemeProvider>,
-      ),
-    ).toThrow(
-      "ActionList.Heading shouldn't be used within an ActionMenu container. Menus are labelled by the menu button's name.",
-    )
-    expect(spy).toHaveBeenCalled()
-    spy.mockRestore()
-  })
-
   it('should throw an error when ActionList.GroupHeading has an `as` prop when it is used within ActionMenu context', async () => {
     const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
     expect(() =>

diff --git a/packages/react/src/ActionList/Heading.module.css b/packages/react/src/ActionList/Heading.module.css
@@ -0,0 +1,12 @@
+.ActionListHeader {
+  margin-block-end: var(--base-size-8);
+
+  &:where([data-list-variant='full']) {
+    margin-inline-start: var(--base-size-8);
+  }
+
+  &:where([data-list-variant='inset']) {
+    /* stylelint-disable-next-line primer/spacing */
+    margin-inline-start: calc(var(--control-medium-paddingInline-condensed) + var(--base-size-8));
+  }
+}
diff --git a/packages/react/src/ActionList/Heading.test.tsx b/packages/react/src/ActionList/Heading.test.tsx
@@ -0,0 +1,83 @@
+import {render as HTMLRender} from '@testing-library/react'
+import React from 'react'
+import theme from '../theme'
+import {ActionList} from '.'
+import {BaseStyles, ThemeProvider, ActionMenu} from '..'
+import {FeatureFlags} from '../FeatureFlags'
+
+describe('ActionList.Heading', () => {
+  it('should render the ActionList.Heading component as a heading with the given heading level', async () => {
+    const container = HTMLRender(
+      <ActionList>
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+      </ActionList>,
+    )
+    const heading = container.getByRole('heading', {level: 1})
+    expect(heading).toBeInTheDocument()
+    expect(heading).toHaveTextContent('Heading')
+  })
+
+  it('should label the action list with the heading id', async () => {
+    const {container, getByRole} = HTMLRender(
+      <ActionList>
+        <ActionList.Heading as="h1">Heading</ActionList.Heading>
+        <ActionList.Item>Item</ActionList.Item>
+      </ActionList>,
+    )
+    const list = container.querySelector('ul')
+    const heading = getByRole('heading', {level: 1})
+    expect(list).toHaveAttribute('aria-labelledby', heading.id)
+  })
+
+  it('should throw an error when ActionList.Heading is used within ActionMenu context', async () => {
+    const spy = jest.spyOn(console, 'error').mockImplementation(() => jest.fn())
+    expect(() =>
+      HTMLRender(
+        <ThemeProvider theme={theme}>
+          <BaseStyles>
+            <ActionMenu open={true}>
+              <ActionMenu.Button>Trigger</ActionMenu.Button>
+              <ActionMenu.Overlay>
+                <ActionList>
+                  <ActionList.Heading as="h1">Heading</ActionList.Heading>
+                  <ActionList.Item>Item</ActionList.Item>
+                </ActionList>
+              </ActionMenu.Overlay>
+            </ActionMenu>
+          </BaseStyles>
+        </ThemeProvider>,
+      ),
+    ).toThrow(
+      "ActionList.Heading shouldn't be used within an ActionMenu container. Menus are labelled by the menu button's name.",
+    )
+    expect(spy).toHaveBeenCalled()
+    spy.mockRestore()
+  })
+
+  it('should support a custom `className` on the outermost element', () => {
+    const Element = () => {
+      return (
+        <ActionList>
+          <ActionList.Heading as="h2" className="test-class-name">
+            Filter by
+          </ActionList.Heading>
+        </ActionList>
+      )
+    }
+    const FeatureFlagElement = () => {
+      return (
+        <FeatureFlags
+          flags={{
+            primer_react_css_modules_team: true,
+            primer_react_css_modules_staff: true,
+            primer_react_css_modules_ga: true,
+          }}
+        >
+          <Element />
+        </FeatureFlags>
+      )
+    }
+    expect(HTMLRender(<FeatureFlagElement />).container.querySelector('h2')).toHaveClass('test-class-name')
+    expect(HTMLRender(<Element />).container.querySelector('h2')).toHaveClass('test-class-name')
+  })
+})
diff --git a/packages/react/src/ActionList/Heading.tsx b/packages/react/src/ActionList/Heading.tsx
@@ -9,18 +9,26 @@ import {ListContext} from './shared'
 import VisuallyHidden from '../_VisuallyHidden'
 import {ActionListContainerContext} from './ActionListContainerContext'
 import {invariant} from '../utils/invariant'
+import {clsx} from 'clsx'
+import {useFeatureFlag} from '../FeatureFlags'
+import classes from './Heading.module.css'
 
 type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
+type HeadingVariants = 'large' | 'medium' | 'small'
 export type ActionListHeadingProps = {
   as: HeadingLevels
+  size?: HeadingVariants
   visuallyHidden?: boolean
+  className?: string
 } & SxProp
 
 export const Heading = forwardRef(
-  ({as, children, sx = defaultSxProp, visuallyHidden = false, ...props}, forwardedRef) => {
+  ({as, size, children, sx = defaultSxProp, visuallyHidden = false, className, ...props}, forwardedRef) => {
     const innerRef = React.useRef<HTMLHeadingElement>(null)
     useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
+    const enabled = useFeatureFlag('primer_react_css_modules_team')
+
     const {headingId: headingId, variant: listVariant} = React.useContext(ListContext)
     const {container} = React.useContext(ActionListContainerContext)
 
@@ -37,16 +45,49 @@ export const Heading = forwardRef(
 
     return (
       <VisuallyHidden isVisible={!visuallyHidden}>
-        <HeadingComponent
-          as={as}
-          ref={innerRef}
-          // use custom id if it is provided. Otherwise, use the id from the context
-          id={props.id ?? headingId}
-          sx={merge<BetterSystemStyleObject>(styles, sx)}
-          {...props}
-        >
-          {children}
-        </HeadingComponent>
+        {enabled ? (
+          sx !== defaultSxProp ? (
+            <HeadingComponent
+              as={as}
+              variant={size}
+              ref={innerRef}
+              // use custom id if it is provided. Otherwise, use the id from the context
+              id={props.id ?? headingId}
+              className={clsx(className, classes.ActionListHeader)}
+              data-list-variant={listVariant}
+              sx={sx}
+              {...props}
+            >
+              {children}
+            </HeadingComponent>
+          ) : (
+            <HeadingComponent
+              as={as}
+              variant={size}
+              ref={innerRef}
+              // use custom id if it is provided. Otherwise, use the id from the context
+              id={props.id ?? headingId}
+              className={clsx(className, classes.ActionListHeader)}
+              data-list-variant={listVariant}
+              {...props}
+            >
+              {children}
+            </HeadingComponent>
+          )
+        ) : (
+          <HeadingComponent
+            as={as}
+            variant={size}
+            ref={innerRef}
+            // use custom id if it is provided. Otherwise, use the id from the context
+            id={props.id ?? headingId}
+            sx={merge<BetterSystemStyleObject>(styles, sx)}
+            className={className}
+            {...props}
+          >
+            {children}
+          </HeadingComponent>
+        )}
       </VisuallyHidden>
     )
   },