Merge 1fe209b into 31d9a05

primer · Oct 29, 2024 · c53c88d · c53c88d
2 parents 31d9a05 + 1fe209b
commit c53c88d
Show file tree

Hide file tree

Showing 6 changed files with 136 additions and 100 deletions.
diff --git a/.changeset/four-schools-grin.md b/.changeset/four-schools-grin.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Update BranchName to use CSS Modules behind feature flag
diff --git a/e2e/components/BranchName.test.ts b/e2e/components/BranchName.test.ts
@@ -2,110 +2,58 @@ import {test, expect} from '@playwright/test'
 import {visit} from '../test-helpers/storybook'
 import {themes} from '../test-helpers/themes'
 
-test.describe('BranchName', () => {
-  test.describe('Default', () => {
-    for (const theme of themes) {
-      test.describe(theme, () => {
-        test('default @vrt', async ({page}) => {
-          await visit(page, {
-            id: 'components-branchname--default',
-            globals: {
-              colorScheme: theme,
-            },
-          })
-
-          // Default state
-          expect(await page.screenshot()).toMatchSnapshot(`BranchName.Default.${theme}.png`)
-
-          // Focus state
-          await page.keyboard.press('Tab')
-          expect(await page.screenshot()).toMatchSnapshot(`BranchName.Default.${theme}.focus.png`)
-        })
+const stories = [
+  {
+    title: 'Default',
+    id: 'components-branchname--default',
+    focus: true,
+  },
+  {
+    title: 'Not A Link',
+    id: 'components-branchname-features--not-a-link',
+    focus: false,
+  },
+  {
+    title: 'With A Branch Icon',
+    id: 'components-branchname-features--with-branch-icon',
+    focus: false,
+  },
+] as const
 
-        test('axe @aat', async ({page}) => {
-          await visit(page, {
-            id: 'components-branchname--default',
-            globals: {
-              colorScheme: theme,
-            },
-          })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
+test.describe('BranchName', () => {
+  for (const story of stories) {
+    test.describe(story.title, () => {
+      for (const theme of themes) {
+        test.describe(theme, () => {
+          test('default @vrt', async ({page}) => {
+            await visit(page, {
+              id: story.id,
+              globals: {
+                colorScheme: theme,
               },
-            },
-          })
-        })
-      })
-    }
-  })
-
-  test.describe('Not A Link', () => {
-    for (const theme of themes) {
-      test.describe(theme, () => {
-        test('default @vrt', async ({page}) => {
-          await visit(page, {
-            id: 'components-branchname-features--not-a-link',
-            globals: {
-              colorScheme: theme,
-            },
-          })
-
-          // Default state
-          expect(await page.screenshot()).toMatchSnapshot(`BranchName.Not A Link.${theme}.png`)
-        })
+            })
 
-        test('axe @aat', async ({page}) => {
-          await visit(page, {
-            id: 'components-branchname-features--not-a-link',
-            globals: {
-              colorScheme: theme,
-            },
-          })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
-              },
-            },
-          })
-        })
-      })
-    }
-  })
+            // Default state
+            expect(await page.screenshot()).toMatchSnapshot(`BranchName.${story.title}.${theme}.png`)
 
-  test.describe('With A Branch Icon', () => {
-    for (const theme of themes) {
-      test.describe(theme, () => {
-        test('default @vrt', async ({page}) => {
-          await visit(page, {
-            id: 'components-branchname-features--with-branch-icon',
-            globals: {
-              colorScheme: theme,
-            },
+            // Focus state
+            if (story.focus) {
+              await page.keyboard.press('Tab')
+              expect(await page.screenshot()).toMatchSnapshot(`BranchName.${story.title}.${theme}.focus.png`)
+            }
           })
 
-          // Default state
-          expect(await page.screenshot()).toMatchSnapshot(`BranchName.With A Branch Icon.${theme}.png`)
-        })
-
-        test('axe @aat', async ({page}) => {
-          await visit(page, {
-            id: 'components-branchname-features--with-branch-icon',
-            globals: {
-              colorScheme: theme,
-            },
-          })
-          await expect(page).toHaveNoViolations({
-            rules: {
-              'color-contrast': {
-                enabled: theme !== 'dark_dimmed',
+          test('axe @aat', async ({page}) => {
+            await visit(page, {
+              id: story.id,
+              globals: {
+                colorScheme: theme,
               },
-            },
+            })
+            await expect(page).toHaveNoViolations()
           })
         })
-      })
-    }
-  })
+      }
+    })
+  }
 })
diff --git a/packages/react/src/BranchName/BranchName.module.css b/packages/react/src/BranchName/BranchName.module.css
@@ -0,0 +1,14 @@
+.BranchName {
+  display: inline-block;
+  padding: var(--base-size-2) var(--base-size-6);
+  font-family: var(--fontStack-monospace);
+  font-size: var(--text-body-size-small);
+  color: var(--fgColor-link);
+  text-decoration: none;
+  background-color: var(--bgColor-accent-muted);
+  border-radius: var(--borderRadius-medium);
+
+  &:is(:not(a)) {
+    color: var(--fgColor-muted);
+  }
+}
diff --git a/packages/react/src/BranchName/BranchName.tsx b/packages/react/src/BranchName/BranchName.tsx
@@ -1,10 +1,15 @@
+import React from 'react'
+import {clsx} from 'clsx'
 import styled from 'styled-components'
 import {get} from '../constants'
 import type {SxProp} from '../sx'
 import sx from '../sx'
-import type {ComponentProps} from '../utils/types'
+import {useFeatureFlag} from '../FeatureFlags'
+import Box from '../Box'
+import classes from './BranchName.module.css'
+import {forwardRef} from '../utils/fixedForwardRef'
 
-const BranchName = styled.a<SxProp>`
+const StyledBranchName = styled.a<SxProp>`
   display: inline-block;
   padding: 2px 6px;
   font-size: var(--text-body-size-small, ${get('fontSizes.0')});
@@ -19,5 +24,39 @@ const BranchName = styled.a<SxProp>`
   ${sx};
 `
 
-export type BranchNameProps = ComponentProps<typeof BranchName>
+type BranchNameProps<As extends React.ElementType> = {
+  as?: As
+} & DistributiveOmit<React.ComponentPropsWithRef<React.ElementType extends As ? 'a' : As>, 'as'> &
+  SxProp
+
+type DistributiveOmit<T, TOmitted extends PropertyKey> = T extends any ? Omit<T, TOmitted> : never
+
+const BranchName = forwardRef(function BranchName<As extends React.ElementType>(
+  props: BranchNameProps<As>,
+  ref: React.ForwardedRef<any>,
+) {
+  const {as: BaseComponent = 'a', className, children, sx, ...rest} = props
+  const enabled = useFeatureFlag('primer_react_css_modules_team')
+  if (enabled) {
+    if (sx) {
+      return (
+        <Box {...rest} ref={ref} as={BaseComponent} className={clsx(className, classes.BranchName)} sx={sx}>
+          {children}
+        </Box>
+      )
+    }
+    return (
+      <BaseComponent {...rest} ref={ref} className={clsx(className, classes.BranchName)}>
+        {children}
+      </BaseComponent>
+    )
+  }
+  return (
+    <StyledBranchName {...rest} ref={ref} as={BaseComponent} className={className} sx={sx}>
+      {children}
+    </StyledBranchName>
+  )
+})
+
+export type {BranchNameProps}
 export default BranchName
diff --git a/packages/react/src/BranchName/__tests__/BranchName.test.tsx b/packages/react/src/BranchName/__tests__/BranchName.test.tsx
@@ -3,6 +3,7 @@ import BranchName from '../BranchName'
 import {render, behavesAsComponent, checkExports} from '../../utils/testing'
 import {render as HTMLRender} from '@testing-library/react'
 import axe from 'axe-core'
+import {FeatureFlags} from '../../FeatureFlags'
 
 describe('BranchName', () => {
   behavesAsComponent({Component: BranchName})
@@ -20,4 +21,23 @@ describe('BranchName', () => {
   it('renders an <a> by default', () => {
     expect(render(<BranchName />).type).toEqual('a')
   })
+
+  it('should support `className` on the outermost element', () => {
+    const Element = () => <BranchName className={'test-class-name'} />
+    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(<Element />).container.firstChild).toHaveClass('test-class-name')
+    expect(HTMLRender(<FeatureFlagElement />).container.firstChild).toHaveClass('test-class-name')
+  })
 })
diff --git a/packages/react/src/utils/fixedForwardRef.ts b/packages/react/src/utils/fixedForwardRef.ts
@@ -0,0 +1,10 @@
+import {forwardRef} from 'react'
+import type React from 'react'
+
+export type FixedForwardRef = <T, P = {}>(
+  render: (props: P, ref: React.Ref<T>) => React.ReactNode,
+) => (props: P & React.RefAttributes<T>) => React.ReactNode
+
+const fixedForwardRef = forwardRef as FixedForwardRef
+
+export {fixedForwardRef as forwardRef}