Remove the CSS modules feature flag from Heading (#5018)

* Remove feature flag from Heading component * Create good-years-attack.md * Update tests and snapshots
primer · Sep 25, 2024 · 8e4beae · 8e4beae
1 parent 15cb90f
commit 8e4beae
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 154 deletions.
diff --git a/.changeset/good-years-attack.md b/.changeset/good-years-attack.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Remove the CSS modules feature flag from `Heading`
diff --git a/e2e/components/Heading.test.ts b/e2e/components/Heading.test.ts
@@ -24,20 +24,6 @@ test.describe('Heading', () => {
   for (const story of stories) {
     test.describe(story.title, () => {
       test('default @vrt', async ({page}) => {
-        await visit(page, {
-          id: story.id,
-          globals: {
-            featureFlags: {
-              primer_react_css_modules_ga: true,
-            },
-          },
-        })
-
-        // Default state
-        expect(await page.screenshot()).toMatchSnapshot(`Heading.${story.title}.png`)
-      })
-
-      test('default (styled-components) @vrt', async ({page}) => {
         await visit(page, {
           id: story.id,
         })
@@ -47,18 +33,6 @@ test.describe('Heading', () => {
       })
 
       test('axe @aat', async ({page}) => {
-        await visit(page, {
-          id: story.id,
-          globals: {
-            featureFlags: {
-              primer_react_css_modules_ga: true,
-            },
-          },
-        })
-        await expect(page).toHaveNoViolations()
-      })
-
-      test('axe (styled-components) @aat', async ({page}) => {
         await visit(page, {
           id: story.id,
         })

diff --git a/packages/react/src/Heading/Heading.tsx b/packages/react/src/Heading/Heading.tsx
@@ -1,43 +1,18 @@
 import {clsx} from 'clsx'
 import React, {forwardRef, useEffect} from 'react'
-import styled from 'styled-components'
-import {get} from '../constants'
 import {useRefObjectAsForwardedRef} from '../hooks'
 import type {SxProp} from '../sx'
-import sx from '../sx'
 import type {ComponentProps} from '../utils/types'
 import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import classes from './Heading.module.css'
-import {useFeatureFlag} from '../FeatureFlags'
 import Box from '../Box'
 
 type StyledHeadingProps = {
   as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
   variant?: 'large' | 'medium' | 'small'
 } & SxProp
 
-const StyledHeading = styled.h2<StyledHeadingProps>`
-  font-weight: ${get('fontWeights.bold')};
-  font-size: ${get('fontSizes.5')};
-  margin: 0;
-
-  &:where([data-variant='large']) {
-    font: var(--text-title-shorthand-large, 600 32px / 1.5 ${get('fonts.normal')});
-  }
-
-  &:where([data-variant='medium']) {
-    font: var(--text-title-shorthand-medium, 600 20px / 1.6 ${get('fonts.normal')});
-  }
-
-  &:where([data-variant='small']) {
-    font: var(--text-title-shorthand-small, 600 16px / 1.5 ${get('fonts.normal')});
-  }
-
-  ${sx};
-`
-
 const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}, forwardedRef) => {
-  const enabled = useFeatureFlag('primer_react_css_modules_ga')
   const innerRef = React.useRef<HTMLHeadingElement>(null)
   useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
@@ -57,33 +32,19 @@ const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}
     }, [innerRef])
   }
 
-  if (enabled) {
-    if (props.sx) {
-      return (
-        <Box
-          as={Component}
-          className={clsx(className, classes.Heading)}
-          data-variant={variant}
-          {...props}
-          // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
-          ref={innerRef}
-        />
-      )
-    }
-    return <Component className={clsx(className, classes.Heading)} data-variant={variant} {...props} ref={innerRef} />
+  if (props.sx) {
+    return (
+      <Box
+        as={Component}
+        className={clsx(className, classes.Heading)}
+        data-variant={variant}
+        {...props}
+        // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
+        ref={innerRef}
+      />
+    )
   }
-
-  return (
-    <StyledHeading
-      as={Component}
-      className={className}
-      data-variant={variant}
-      sx={sx}
-      {...props}
-      // @ts-ignore temporary disable as we migrate to css modules, until we remove PolymorphicForwardRefComponent
-      ref={innerRef}
-    />
-  )
+  return <Component className={clsx(className, classes.Heading)} data-variant={variant} {...props} ref={innerRef} />
 }) as PolymorphicForwardRefComponent<'h2', StyledHeadingProps>
 
 Heading.displayName = 'Heading'

diff --git a/packages/react/src/Heading/__tests__/Heading.test.tsx b/packages/react/src/Heading/__tests__/Heading.test.tsx
@@ -4,7 +4,6 @@ import {render, behavesAsComponent, checkExports} from '../../utils/testing'
 import {render as HTMLRender, screen} from '@testing-library/react'
 import axe from 'axe-core'
 import ThemeProvider from '../../ThemeProvider'
-import {FeatureFlags} from '../../FeatureFlags'
 
 const theme = {
   breakpoints: ['400px', '640px', '960px', '1280px'],
@@ -142,54 +141,30 @@ describe('Heading', () => {
     ).toHaveStyleRule('font-style', 'italic')
   })
 
-  describe('with primer_react_css_modules_ga enabled', () => {
-    it('should only include css modules class', () => {
-      HTMLRender(
-        <FeatureFlags
-          flags={{
-            primer_react_css_modules_ga: true,
-          }}
-        >
-          <Heading>test</Heading>
-        </FeatureFlags>,
-      )
-      expect(screen.getByText('test')).toHaveClass('Heading')
-      // Note: this is the generated class name when styled-components is used
-      // for this component
-      expect(screen.getByText('test')).not.toHaveClass(/^Heading__StyledHeading/)
-    })
-
-    it('should support `className` on the outermost element', () => {
-      const {container} = HTMLRender(
-        <FeatureFlags
-          flags={{
-            primer_react_css_modules_ga: true,
-          }}
-        >
-          <Heading className="test">test</Heading>
-        </FeatureFlags>,
-      )
-      expect(container.firstChild).toHaveClass('test')
-    })
-
-    it('should support overrides with sx if provided', () => {
-      HTMLRender(
-        <FeatureFlags
-          flags={{
-            primer_react_css_modules_ga: true,
-          }}
-        >
-          <Heading
-            sx={{
-              fontWeight: '900',
-            }}
-          >
-            test
-          </Heading>
-        </FeatureFlags>,
-      )
-
-      expect(screen.getByText('test')).toHaveStyle('font-weight: 900')
-    })
+  it('should only include css modules class', () => {
+    HTMLRender(<Heading>test</Heading>)
+    expect(screen.getByText('test')).toHaveClass('Heading')
+    // Note: this is the generated class name when styled-components is used
+    // for this component
+    expect(screen.getByText('test')).not.toHaveClass(/^Heading__StyledHeading/)
+  })
+
+  it('should support `className` on the outermost element', () => {
+    const {container} = HTMLRender(<Heading className="test">test</Heading>)
+    expect(container.firstChild).toHaveClass('test')
+  })
+
+  it('should support overrides with sx if provided', () => {
+    HTMLRender(
+      <Heading
+        sx={{
+          fontWeight: '900',
+        }}
+      >
+        test
+      </Heading>,
+    )
+
+    expect(screen.getByText('test')).toHaveStyle('font-weight: 900')
   })
 })
diff --git a/packages/react/src/NavList/__snapshots__/NavList.test.tsx.snap b/packages/react/src/NavList/__snapshots__/NavList.test.tsx.snap
@@ -464,6 +464,16 @@ exports[`NavList renders with groups 1`] = `
   margin-top: 8px;
 }
 
+.c3 {
+  padding-top: 6px;
+  padding-bottom: 6px;
+  padding-left: 16px;
+  padding-right: 16px;
+  font-size: 12px;
+  font-weight: 600;
+  color: var(--fgColor-muted,var(--color-fg-muted,#656d76));
+}
+
 .c4 {
   padding-inline-start: 0;
 }
@@ -501,31 +511,6 @@ exports[`NavList renders with groups 1`] = `
   word-break: break-word;
 }
 
-.c3 {
-  font-weight: 600;
-  font-size: 32px;
-  margin: 0;
-  padding-top: 6px;
-  padding-bottom: 6px;
-  padding-left: 16px;
-  padding-right: 16px;
-  font-size: 12px;
-  font-weight: 600;
-  color: var(--fgColor-muted,var(--color-fg-muted,#656d76));
-}
-
-.c3:where([data-variant='large']) {
-  font: var(--text-title-shorthand-large,600 32px / 1.5 -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji");
-}
-
-.c3:where([data-variant='medium']) {
-  font: var(--text-title-shorthand-medium,600 20px / 1.6 -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji");
-}
-
-.c3:where([data-variant='small']) {
-  font: var(--text-title-shorthand-small,600 16px / 1.5 -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji");
-}
-
 .c0 {
   margin: 0;
   padding-inline-start: 0;
@@ -869,7 +854,7 @@ exports[`NavList renders with groups 1`] = `
         class="c2"
       >
         <h3
-          class="c3"
+          class="c3 Heading"
           id=":r7:"
         >
           Overview
@@ -913,7 +898,7 @@ exports[`NavList renders with groups 1`] = `
         class="c2"
       >
         <h3
-          class="c3"
+          class="c3 Heading"
           id=":r9:"
         >
           Components