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

Add variant prop to Heading #4806

Merged
merged 23 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from 15 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/clever-birds-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"@primer/react": patch
"@primer/react": minor

---

Add `variant` prop to Heading for small, medium and large styles
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange that these tests show up on this PR? Are these changes from a different PR committed here?

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
93 changes: 67 additions & 26 deletions e2e/components/Heading.test.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,74 @@
import {test, expect} from '@playwright/test'
import {visit} from '../test-helpers/storybook'
import {themes} from '../test-helpers/themes'

test.describe('Heading', () => {
test.describe('Default', () => {
for (const theme of themes) {
test.describe(theme, () => {
test('default @vrt', async ({page}) => {
await visit(page, {
id: 'components-heading--default',
globals: {
colorScheme: theme,
},
})

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

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

// Default state
expect(await page.screenshot()).toMatchSnapshot(`Heading.Default.png`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aww? we don't tests for all themes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its just testing for font-size so color doesn't matter 😄

})

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--small',
})

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

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

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

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

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

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

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

test('axe @aat', async ({page}) => {
await visit(page, {
id: 'components-heading--large',
})
await expect(page).toHaveNoViolations()
})
})
})
5 changes: 5 additions & 0 deletions packages/react/src/Heading/Heading.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
"name": "as",
"type": "React.ElementType",
"defaultValue": "\"h2\""
},
{
"name": "variant",
"type": "'large' | 'medium' | 'small'",
"defaultValue": ""
langermank marked this conversation as resolved.
Show resolved Hide resolved
}
]
}
24 changes: 24 additions & 0 deletions packages/react/src/Heading/Heading.featires.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in file name, would mess with automated examples in docs

- Heading.featires.stories.tsx
+ Heading.features.stories.tsx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tire 😄 thanks!

import type {StoryFn} from '@storybook/react'
import Heading from './Heading'

export default {
title: 'Components/Heading',
}

export const TestSx: StoryFn<typeof Heading> = () => (
<Heading
sx={{
fontSize: 2,
fontWeight: 'normal',
}}
>
Heading with sx override
</Heading>
)

export const Small: StoryFn<typeof Heading> = () => <Heading variant="small">Small heading</Heading>

export const Medium: StoryFn<typeof Heading> = () => <Heading variant="medium">Medium heading</Heading>

export const Large: StoryFn<typeof Heading> = () => <Heading variant="large">Large heading</Heading>
11 changes: 9 additions & 2 deletions packages/react/src/Heading/Heading.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export default {
title: 'Components/Heading',
component: Heading,
} as Meta<typeof Heading>

export const Default: StoryFn<typeof Heading> = () => <Heading>Default H2 Heading</Heading>

export const Playground: StoryFn<typeof Heading> = args => <Heading {...args}>Heading</Heading>
Expand All @@ -17,8 +18,14 @@ Playground.args = {
Playground.argTypes = {
as: {
control: {
type: 'select',
options: ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'],
type: 'radio',
},
options: ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'],
},
variant: {
control: {
type: 'radio',
},
options: ['large', 'medium', 'small'],
},
}
16 changes: 15 additions & 1 deletion packages/react/src/Heading/Heading.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,28 @@ import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../uti

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', ...props}, forwardedRef) => {
const Heading = forwardRef(({as: Component = 'h2', variant, ...props}, forwardedRef) => {
const innerRef = React.useRef<HTMLHeadingElement>(null)
useRefObjectAsForwardedRef(forwardedRef, innerRef)

Expand All @@ -43,6 +56,7 @@ const Heading = forwardRef(({as: Component = 'h2', ...props}, forwardedRef) => {
{...props}
// @ts-ignore shh
ref={innerRef}
data-variant={variant}
/>
)
}) as PolymorphicForwardRefComponent<'h2', StyledHeadingProps>
Expand Down
12 changes: 12 additions & 0 deletions packages/react/src/NavList/__snapshots__/NavList.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,18 @@ exports[`NavList renders with groups 1`] = `
color: var(--fgColor-muted,var(--color-fg-muted,#656d76));
}

.c3:where([data-variant='large']) {
font: var(--text-title-shorthand-large,600 32px / 1.5 -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji");
}

.c3:where([data-variant='medium']) {
font: var(--text-title-shorthand-medium,600 20px / 1.6 -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji");
}

.c3:where([data-variant='small']) {
font: var(--text-title-shorthand-small,600 16px / 1.5 -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji");
}

.c0 {
margin: 0;
padding-inline-start: 0;
Expand Down
Loading