Skip to content

Commit

Permalink
refactor(Heading): update component to CSS Modules (#4819)
Browse files Browse the repository at this point in the history
* refactor(Heading): update component to CSS Modules

* test(e2e): add feature flags for Heading test

* chore: fix stylelint errors

* test(e2e): update snapshots for Heading

* refactor(Heading): incorporate variant features into CSS Modules

* chore: add changeset

* test(e2e): remove theme testing

* Fix props here

* Remove examples/nextjs

* Clean up css

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
Co-authored-by: Jon Rohan <yes@jonrohan.codes>
  • Loading branch information
3 people authored Aug 14, 2024
1 parent 1a674f7 commit 0112347
Show file tree
Hide file tree
Showing 14 changed files with 156 additions and 62 deletions.
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')
})
})
})

0 comments on commit 0112347

Please sign in to comment.