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(Heading): update component to CSS Modules #4819

Merged
merged 12 commits into from
Aug 14, 2024
5 changes: 5 additions & 0 deletions .changeset/nervous-llamas-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Update Heading component to use CSS Modules behind feature flag
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
113 changes: 54 additions & 59 deletions e2e/components/Heading.test.ts
Original file line number Diff line number Diff line change
@@ -1,74 +1,69 @@
import {test, expect} from '@playwright/test'
import {visit} from '../test-helpers/storybook'

test.describe('Heading', () => {
test.describe('Default', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-heading--default',
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Heading.Default.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-heading--default',
})
await expect(page).toHaveNoViolations()
})
})

test.describe('Small', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-heading-features--small',
})
const stories = [
{
title: 'Default',
id: 'components-heading--default',
},
{
title: 'Small',
id: 'components-heading-features--small',
},
{
title: 'Medium',
id: 'components-heading-features--medium',
},
{
title: 'Large',
id: 'components-heading-features--large',
},
] as const

expect(await page.screenshot()).toMatchSnapshot(`Heading.Small.png`)
})
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: true,
},
},
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-heading-features--small',
// Default state
expect(await page.screenshot()).toMatchSnapshot(`Heading.${story.title}.png`)
})
await expect(page).toHaveNoViolations()
})
})

test.describe('Medium', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-heading-features--medium',
})
test('default (styled-components) @vrt', async ({page}) => {
await visit(page, {
id: story.id,
})

expect(await page.screenshot()).toMatchSnapshot(`Heading.Medium.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-heading-features--medium',
// Default state
expect(await page.screenshot()).toMatchSnapshot(`Heading.${story.title}.png`)
})
await expect(page).toHaveNoViolations()
})
})

test.describe('Large', () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-heading-features--large',
test('axe @aat', async ({page}) => {
await visit(page, {
id: story.id,
globals: {
featureFlags: {
primer_react_css_modules: true,
},
},
})
await expect(page).toHaveNoViolations()
})

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Heading.Large.png`)
})

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-heading-features--large',
test('axe (styled-components) @aat', async ({page}) => {
await visit(page, {
id: story.id,
})
await expect(page).toHaveNoViolations()
})
await expect(page).toHaveNoViolations()
})
})
}
})
17 changes: 17 additions & 0 deletions packages/react/src/Heading/Heading.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
.Heading {
margin: 0;
font-size: var(--text-title-size-large);
font-weight: var(--base-text-weight-semibold);

&[data-variant='large'] {
font: var(--text-title-shorthand-large);
}

&[data-variant='medium'] {
font: var(--text-title-shorthand-medium);
}

&[data-variant='small'] {
font: var(--text-title-shorthand-small);
}
}
29 changes: 27 additions & 2 deletions packages/react/src/Heading/Heading.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import cx from 'clsx'
import React, {forwardRef, useEffect} from 'react'
import styled from 'styled-components'
import {get} from '../constants'
Expand All @@ -6,6 +7,9 @@ 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'
Expand All @@ -28,9 +32,12 @@ const StyledHeading = styled.h2<StyledHeadingProps>`
&:where([data-variant='small']) {
font: var(--text-title-shorthand-small, 600 16px / 1.5 ${get('fonts.normal')});
}

${sx};
`
const Heading = forwardRef(({as: Component = 'h2', variant, ...props}, forwardedRef) => {

const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}, forwardedRef) => {
const enabled = useFeatureFlag('primer_react_css_modules')
const innerRef = React.useRef<HTMLHeadingElement>(null)
useRefObjectAsForwardedRef(forwardedRef, innerRef)

Expand All @@ -50,13 +57,31 @@ const Heading = forwardRef(({as: Component = 'h2', variant, ...props}, forwarded
}, [innerRef])
}

if (enabled) {
if (props.sx) {
return (
<Box
as={Component}
className={cx(className, classes.Heading)}
data-variant={variant}
{...props}
// @ts-ignore shh
ref={innerRef}
/>
)
}
return <Component className={cx(className, classes.Heading)} data-variant={variant} {...props} ref={innerRef} />
}

return (
<StyledHeading
as={Component}
className={className}
data-variant={variant}
sx={sx}
{...props}
// @ts-ignore shh
ref={innerRef}
data-variant={variant}
/>
)
}) as PolymorphicForwardRefComponent<'h2', StyledHeadingProps>
Expand Down
54 changes: 53 additions & 1 deletion packages/react/src/Heading/__tests__/Heading.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import React from 'react'
import {Heading} from '../..'
import {render, behavesAsComponent, checkExports} from '../../utils/testing'
import {render as HTMLRender} from '@testing-library/react'
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'],
Expand Down Expand Up @@ -140,4 +141,55 @@ describe('Heading', () => {
),
).toHaveStyleRule('font-style', 'italic')
})

describe('with primer_react_css_modules enabled', () => {
it('should only include css modules class', () => {
HTMLRender(
<FeatureFlags
flags={{
primer_react_css_modules: 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: 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: true,
}}
>
<Heading
sx={{
fontWeight: '900',
}}
>
test
</Heading>
</FeatureFlags>,
)

expect(screen.getByText('test')).toHaveStyle('font-weight: 900')
})
})
})
Loading