Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(BranchName): update BranchName to use CSS Modules #5040

Merged
merged 15 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/four-schools-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Update BranchName to use CSS Modules behind feature flag
142 changes: 45 additions & 97 deletions e2e/components/BranchName.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
})
}
})
}
})
}
})
14 changes: 14 additions & 0 deletions packages/react/src/BranchName/BranchName.module.css
Original file line number Diff line number Diff line change
@@ -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);
}
}
57 changes: 53 additions & 4 deletions packages/react/src/BranchName/BranchName.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import React, {type ForwardedRef} 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'

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')});
Expand All @@ -19,5 +23,50 @@ const BranchName = styled.a<SxProp>`
${sx};
`

export type BranchNameProps = ComponentProps<typeof BranchName>
export default BranchName
type BranchNameProps<As extends React.ElementType> = {
as?: As
} & DistributiveOmit<React.ComponentPropsWithRef<React.ElementType extends As ? 'a' : As>, 'as'> &
SxProp

// eslint-disable-next-line @typescript-eslint/no-explicit-any
function BranchName<As extends React.ElementType>(props: BranchNameProps<As>, ref: 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} as={BaseComponent} ref={ref} className={className} sx={sx}>
{children}
</StyledBranchName>
)
}

// eslint-disable-next-line @typescript-eslint/ban-types
type FixedForwardRef = <T, P = {}>(
render: (props: P, ref: React.Ref<T>) => React.ReactNode,
) => (props: P & React.RefAttributes<T>) => React.ReactNode

const fixedForwardRef = React.forwardRef as FixedForwardRef

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type DistributiveOmit<T, TOmitted extends PropertyKey> = T extends any ? Omit<T, TOmitted> : never

BranchName.displayName = 'BranchName'

export type {BranchNameProps}
export default fixedForwardRef(BranchName)
27 changes: 26 additions & 1 deletion packages/react/src/BranchName/__tests__/BranchName.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@ 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})
behavesAsComponent({
Component: BranchName,
options: {
skipDisplayName: true,
},
})

checkExports('BranchName', {
default: BranchName,
Expand All @@ -20,4 +26,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')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,43 @@ export function shouldNotAcceptSystemProps() {
// @ts-expect-error system props should not be accepted
return <BranchName backgroundColor="thistle" />
}

export function shouldAcceptAs() {
return (
<BranchName
as="button"
onClick={event => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
type test = Expect<Equal<typeof event, React.MouseEvent<HTMLButtonElement, MouseEvent>>>
}}
/>
)
}

export function defaultAsIsAnchor() {
return (
<BranchName
onClick={event => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
type test = Expect<Equal<typeof event, React.MouseEvent<HTMLAnchorElement, MouseEvent>>>
}}
/>
)
}

export function ShouldAcceptRef() {
const ref = React.useRef<HTMLButtonElement>(null)
return (
<BranchName
as="button"
ref={ref}
onClick={event => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
type test = Expect<Equal<typeof event, React.MouseEvent<HTMLButtonElement, MouseEvent>>>
}}
/>
)
}

type Expect<T extends true> = T
type Equal<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T extends Y ? 1 : 2 ? true : false
9 changes: 6 additions & 3 deletions packages/react/src/utils/testing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ export function unloadCSS(path: string) {
interface Options {
skipAs?: boolean
skipSx?: boolean
skipDisplayName?: boolean
}

interface BehavesAsComponent {
Expand Down Expand Up @@ -221,9 +222,11 @@ export function behavesAsComponent({Component, toRender, options}: BehavesAsComp
})
}

it('sets a valid displayName', () => {
expect(Component.displayName).toMatch(COMPONENT_DISPLAY_NAME_REGEX)
})
if (!options.skipDisplayName) {
it('sets a valid displayName', () => {
expect(Component.displayName).toMatch(COMPONENT_DISPLAY_NAME_REGEX)
})
}
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand Down
Loading