diff --git a/.changeset/weak-papers-drive.md b/.changeset/weak-papers-drive.md new file mode 100644 index 00000000000..e9dcc746de1 --- /dev/null +++ b/.changeset/weak-papers-drive.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Remove support for `sx` from the `Heading` component diff --git a/packages/react/src/Header/Header.dev.module.css b/packages/react/src/Header/Header.dev.module.css index 4cc998f661c..cec71865407 100644 --- a/packages/react/src/Header/Header.dev.module.css +++ b/packages/react/src/Header/Header.dev.module.css @@ -11,3 +11,7 @@ .HeaderDevLink { color: var(--color-prettylights-syntax-carriage-return-text); } + +.Icon { + margin-right: var(--base-size-8); +} diff --git a/packages/react/src/Heading/Heading.docs.json b/packages/react/src/Heading/Heading.docs.json index eda0b578973..85f20e94203 100644 --- a/packages/react/src/Heading/Heading.docs.json +++ b/packages/react/src/Heading/Heading.docs.json @@ -6,11 +6,6 @@ "stories": [], "importPath": "@primer/react", "props": [ - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "as", "type": "React.ElementType", diff --git a/packages/react/src/Heading/Heading.features.stories.tsx b/packages/react/src/Heading/Heading.features.stories.tsx index 38486183286..5f8b4bb091c 100644 --- a/packages/react/src/Heading/Heading.features.stories.tsx +++ b/packages/react/src/Heading/Heading.features.stories.tsx @@ -5,17 +5,6 @@ export default { title: 'Components/Heading/Features', } -export const TestSx: StoryFn = () => ( - - Heading with sx override - -) - export const Small: StoryFn = () => Small heading export const Medium: StoryFn = () => Medium heading diff --git a/packages/react/src/Heading/Heading.tsx b/packages/react/src/Heading/Heading.tsx index 3064d1b82f3..56c7c7ed149 100644 --- a/packages/react/src/Heading/Heading.tsx +++ b/packages/react/src/Heading/Heading.tsx @@ -1,16 +1,16 @@ import {clsx} from 'clsx' import React, {forwardRef, useEffect} from 'react' import {useRefObjectAsForwardedRef} from '../hooks' -import type {SxProp} from '../sx' import type {ComponentProps} from '../utils/types' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import classes from './Heading.module.css' -import Box from '../Box' + +type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' type StyledHeadingProps = { - as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' + as?: HeadingLevels variant?: 'large' | 'medium' | 'small' -} & SxProp +} const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props}, forwardedRef) => { const innerRef = React.useRef(null) @@ -33,20 +33,8 @@ const Heading = forwardRef(({as: Component = 'h2', className, variant, ...props} }, [innerRef]) } - if (props.sx) { - return ( - - ) - } return -}) as PolymorphicForwardRefComponent<'h2', StyledHeadingProps> +}) as PolymorphicForwardRefComponent Heading.displayName = 'Heading' diff --git a/packages/react/src/Heading/__tests__/Heading.test.tsx b/packages/react/src/Heading/__tests__/Heading.test.tsx index 7b9836e3af6..573bc2405e9 100644 --- a/packages/react/src/Heading/__tests__/Heading.test.tsx +++ b/packages/react/src/Heading/__tests__/Heading.test.tsx @@ -1,30 +1,6 @@ import {describe, expect, it, vi} from 'vitest' import {Heading} from '../..' import {render, screen} from '@testing-library/react' -import ThemeProvider from '../../ThemeProvider' - -const theme = { - breakpoints: ['400px', '640px', '960px', '1280px'], - colors: { - green: ['#010', '#020', '#030', '#040', '#050', '#060'], - }, - fontSizes: ['12px', '14px', '16px', '20px', '24px', '32px', '40px', '48px'], - fonts: { - normal: 'Helvetica,sans-serif', - mono: 'Consolas,monospace', - }, - lineHeights: { - normal: 1.5, - condensed: 1.25, - condensedUltra: 1, - }, - fontWeights: { - light: '300', - normal: '400', - semibold: '500', - bold: '600', - }, -} describe('Heading', () => { it('should support `className` on the outermost element', () => { @@ -38,89 +14,6 @@ describe('Heading', () => { expect(heading.tagName).toBe('H2') }) - it('respects fontWeight', () => { - const {container} = render( - - - , - ) - const heading = container.firstChild as HTMLElement - expect(heading).toHaveStyle(`font-weight: ${theme.fontWeights.bold}`) - - const {container: container2} = render( - - - , - ) - const heading2 = container2.firstChild as HTMLElement - expect(heading2).toHaveStyle(`font-weight: ${theme.fontWeights.normal}`) - - const {container: container3} = render( - - - , - ) - const heading3 = container3.firstChild as HTMLElement - expect(heading3).toHaveStyle(`font-weight: ${theme.fontWeights.semibold}`) - - const {container: container4} = render( - - - , - ) - const heading4 = container4.firstChild as HTMLElement - expect(heading4).toHaveStyle(`font-weight: ${theme.fontWeights.light}`) - }) - - it('respects lineHeight', () => { - const {container} = render( - - - , - ) - const heading = container.firstChild as HTMLElement - ///These sx tests should go away right? - expect(heading).toHaveStyle(`line-height: 48px`) - - const {container: container2} = render( - - - , - ) - const heading2 = container2.firstChild as HTMLElement - expect(heading2).toHaveStyle(`line-height: 40px`) - - const {container: container3} = render( - - - , - ) - const heading3 = container3.firstChild as HTMLElement - expect(heading3).toHaveStyle(`line-height: 32px`) - }) - - it('respects fontFamily="mono"', () => { - const {container} = render( - - - , - ) - const heading = container.firstChild as HTMLElement - expect(heading).toHaveStyle(`font-family: ${theme.fonts.mono}`) - }) - - it('renders fontSize', () => { - for (const fontSize of theme.fontSizes) { - const {container} = render( - - - , - ) - const heading = container.firstChild as HTMLElement - expect(heading).toHaveStyle(`font-size: ${fontSize}`) - } - }) - it('logs a warning when trying to render invalid "as" prop', () => { const consoleSpy = vi.spyOn(globalThis.console, 'warn').mockImplementation(() => {}) @@ -130,15 +23,6 @@ describe('Heading', () => { consoleSpy.mockRestore() }) - it('respects the "fontStyle" prop', () => { - const {container} = render( - - - , - ) - const heading = container.firstChild as HTMLElement - expect(heading).toHaveStyle('font-style: italic') - }) // How can we test for generated class names? it.skip('should only include css modules class', () => { @@ -148,18 +32,4 @@ describe('Heading', () => { // for this component expect(screen.getByText('test')).not.toHaveClass(/^Heading__StyledHeading/) }) - - it('should support overrides with sx if provided', () => { - render( - - test - , - ) - - expect(screen.getByText('test')).toHaveStyle('font-weight: 900') - }) }) diff --git a/packages/styled-react/src/components/Heading.tsx b/packages/styled-react/src/components/Heading.tsx new file mode 100644 index 00000000000..92c364a449b --- /dev/null +++ b/packages/styled-react/src/components/Heading.tsx @@ -0,0 +1,17 @@ +import {Heading as PrimerHeading} from '@primer/react' +import {type HeadingProps as PrimerHeadingProps} from '@primer/react' +import type {ForwardRefComponent} from '../polymorphic' +import {sx, type SxProp} from '../sx' +import styled from 'styled-components' + +type HeadingLevels = 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' + +type HeadingProps = PrimerHeadingProps & SxProp + +const Heading: ForwardRefComponent = styled(PrimerHeading).withConfig({ + shouldForwardProp: prop => (prop as keyof HeadingProps) !== 'sx', +})` + ${sx} +` + +export {Heading, type HeadingProps} diff --git a/packages/styled-react/src/index.tsx b/packages/styled-react/src/index.tsx index f0b680cd843..5b2aa7c6030 100644 --- a/packages/styled-react/src/index.tsx +++ b/packages/styled-react/src/index.tsx @@ -7,7 +7,6 @@ export {Box, type BoxProps} from './components/Box' export {Button} from '@primer/react' export {CheckboxGroup} from '@primer/react' export {Details} from '@primer/react' -export {Heading} from '@primer/react' export {IconButton} from '@primer/react' export {Label} from '@primer/react' export {Overlay} from '@primer/react' @@ -36,6 +35,7 @@ export {Dialog, type DialogProps} from './components/Dialog' export {Flash} from './components/Flash' export {FormControl, type FormControlProps} from './components/FormControl' export {Header, type HeaderProps} from './components/Header' +export {Heading} from './components/Heading' export {Link, type LinkProps} from './components/Link' export {LinkButton, type LinkButtonProps} from './components/LinkButton' export {NavList, type NavListProps} from './components/NavList'