Remove feature flag from Heading component

primer · Sep 24, 2024 · dc5485a · dc5485a
1 parent 954170b
commit dc5485a
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 126 deletions.
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
@@ -142,54 +142,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')
   })
 })