diff --git a/.changeset/wet-mammals-feel.md b/.changeset/wet-mammals-feel.md new file mode 100644 index 00000000000..35fe4ef4ebd --- /dev/null +++ b/.changeset/wet-mammals-feel.md @@ -0,0 +1,5 @@ +--- +'@primer/react': major +--- + +Remove support for `sx` from `PageHeader` diff --git a/package-lock.json b/package-lock.json index 7b949b95ae7..9cc83269aba 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26422,6 +26422,7 @@ "@types/react": "18.3.11", "@types/react-dom": "18.3.1", "@vitejs/plugin-react": "^4.3.3", + "babel-plugin-styled-components": "2.1.4", "publint": "^0.3.12", "react": "18.3.1", "react-dom": "18.3.1", diff --git a/packages/react/src/PageHeader/PageHeader.dev.stories.tsx b/packages/react/src/PageHeader/PageHeader.dev.stories.tsx index 66fc9fdd471..ed30893f569 100644 --- a/packages/react/src/PageHeader/PageHeader.dev.stories.tsx +++ b/packages/react/src/PageHeader/PageHeader.dev.stories.tsx @@ -47,53 +47,3 @@ export const LargeVariantWithMultilineTitle = () => ( ) - -export const ArrayTypeFontSizeOnTitle = () => ( -
- - - - Issue Title - - - -
-) - -export const ThemeBaseFontSizeOnTitle = () => ( -
- - - - Issue Title - - - -
-) - -export const StringTypeFontSizeOnTitle = () => ( -
- - - - Issue Title - - - -
-) diff --git a/packages/react/src/PageHeader/PageHeader.docs.json b/packages/react/src/PageHeader/PageHeader.docs.json index 4ead3c802ed..448c021ce18 100644 --- a/packages/react/src/PageHeader/PageHeader.docs.json +++ b/packages/react/src/PageHeader/PageHeader.docs.json @@ -69,11 +69,6 @@ "type": "AriaRole", "description": "The ARIA role to assign to the top-level node of this component." }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "as", "type": "React.ElementType", @@ -100,11 +95,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -134,11 +124,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "`{ narrow: false regular: true wide: true }`", "description": "Whether the parent link is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -156,11 +141,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -178,11 +158,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -206,11 +181,6 @@ "type": "| 'subtitle' | 'medium' | 'large' | { narrow?: | 'subtitle' | 'medium' | 'large' regular?: | 'subtitle' | 'medium' | 'large' wide?: | 'subtitle' | 'medium' | 'large' }", "defaultValue": "medium", "description": "Default title (medium) is the most common page title size. Use for static titles in most situations.\nLarge variant should be used for user-generated content such as issues, pull requests, or discussions.\nSubtitle variant can be used when a PageHeader.Title is already present in the page, such as in a SplitPageLayout." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -228,11 +198,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -250,11 +215,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -273,11 +233,6 @@ "defaultValue": "false", "description": "Whether the content is hidden." }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true - }, { "name": "as", "type": "React.ElementType", @@ -299,11 +254,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -321,11 +271,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -343,11 +288,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -365,11 +305,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -387,11 +322,6 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] }, @@ -425,13 +355,9 @@ "type": "| boolean | { narrow?: boolean regular?: boolean wide?: boolean }", "defaultValue": "false", "description": "Whether the content is hidden." - }, - { - "name": "sx", - "type": "SystemStyleObject", - "deprecated": true } ] } ] -} \ No newline at end of file +} + diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index f78a6890b12..8fff2517360 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -1,7 +1,6 @@ import React, {useEffect} from 'react' import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {isResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' -import type {SxProp, CSSCustomProperties} from '../sx' import Heading from '../Heading' import {ArrowLeftIcon} from '@primer/octicons-react' import type {LinkProps as BaseLinkProps} from '../Link' @@ -15,14 +14,12 @@ import type {AriaRole} from '../utils/types' import {clsx} from 'clsx' import classes from './PageHeader.module.css' -import {defaultSxProp} from '../utils/defaultSxProp' -import {BoxWithFallback} from '../internal/components/BoxWithFallback' // Types that are shared between PageHeader children components export type ChildrenPropTypes = { className?: string hidden?: boolean | ResponsiveValue -} & SxProp +} // Default state for the `visible` prop when a sub component is only visible on narrow viewport const hiddenOnRegularAndWide = { @@ -46,10 +43,10 @@ export type PageHeaderProps = { className?: string role?: AriaRole hasBorder?: boolean -} & SxProp +} const Root = React.forwardRef>( - ({children, className, sx = defaultSxProp, as = 'div', 'aria-label': ariaLabel, role, hasBorder}, forwardedRef) => { + ({children, className, as: BaseComponent = 'div', 'aria-label': ariaLabel, role, hasBorder}, forwardedRef) => { const rootRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const isInteractive = (element: HTMLElement) => { @@ -105,17 +102,15 @@ const Root = React.forwardRef {children} - + ) }, ) as PolymorphicForwardRefComponent<'div', PageHeaderProps> @@ -128,12 +123,11 @@ const ContextArea: React.FC> = ({ children, className, hidden = hiddenOnRegularAndWide, - sx: sxProp = defaultSxProp, }) => { return ( - +
{children} - +
) } type LinkProps = Pick< @@ -146,18 +140,7 @@ export type ParentLinkProps = React.PropsWithChildren( - ( - { - children, - className, - sx: sxProp = defaultSxProp, - href, - 'aria-label': ariaLabel, - as = 'a', - hidden = hiddenOnRegularAndWide, - }, - ref, - ) => { + ({children, className, href, 'aria-label': ariaLabel, as = 'a', hidden = hiddenOnRegularAndWide}, ref) => { return ( <> ( aria-label={ariaLabel} muted className={clsx(classes.ParentLink, className)} - sx={sxProp} {...getHiddenDataAttributes(hidden)} href={href} > @@ -186,13 +168,12 @@ ParentLink.displayName = 'ParentLink' const ContextBar: React.FC> = ({ children, className, - sx: sxProp = defaultSxProp, hidden = hiddenOnRegularAndWide, }) => { return ( - +
{children} - +
) } @@ -201,18 +182,12 @@ const ContextBar: React.FC> = ({ const ContextAreaActions: React.FC> = ({ children, className, - sx: sxProp = defaultSxProp, hidden = hiddenOnRegularAndWide, }) => { return ( - +
{children} - +
) } @@ -224,20 +199,19 @@ type TitleAreaProps = { // --------------------------------------------------------------------- const TitleArea = React.forwardRef>( - ({children, className, sx: sxProp = defaultSxProp, hidden = false, variant = 'medium'}, forwardedRef) => { + ({children, className, hidden = false, variant = 'medium'}, forwardedRef) => { const titleAreaRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const currentVariant = useResponsiveValue(variant, 'medium') return ( - {children} - + ) }, ) as PolymorphicForwardRefComponent<'div', TitleAreaProps> @@ -248,66 +222,42 @@ TitleArea.displayName = 'TitleArea' const LeadingAction: React.FC> = ({ children, className, - sx: sxProp = defaultSxProp, hidden = hiddenOnNarrow, }) => { - const style: CSSCustomProperties = {} - // @ts-ignore sx has height attribute - const {height} = sxProp - if (height) style['--custom-height'] = height return ( - {children} - + ) } // This is reserved for only breadcrumbs. -const Breadcrumbs: React.FC> = ({ - children, - className, - sx: sxProp = defaultSxProp, - hidden = false, -}) => { +const Breadcrumbs: React.FC> = ({children, className, hidden = false}) => { return ( - {children} - + ) } // PageHeader.LeadingVisual and PageHeader.TrailingVisual should remain visible on narrow viewports. -const LeadingVisual: React.FC> = ({ - children, - className, - sx: sxProp = defaultSxProp, - hidden = false, -}) => { - const style: CSSCustomProperties = {} - // @ts-ignore sx has height attribute - const {height} = sxProp - if (height) style['--custom-height'] = height +const LeadingVisual: React.FC> = ({children, className, hidden = false}) => { return ( - {children} - + ) } @@ -315,28 +265,13 @@ export type TitleProps = { as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6' } & ChildrenPropTypes -const Title: React.FC> = ({ - children, - className, - sx: sxProp = defaultSxProp, - hidden = false, - as = 'h2', -}) => { - const style: CSSCustomProperties = {} - // @ts-ignore sxProp can have color attribute - const {fontSize, lineHeight, fontWeight} = sxProp - if (fontSize) style['--custom-font-size'] = fontSize - if (lineHeight) style['--custom-line-height'] = lineHeight - if (fontWeight) style['--custom-font-weight'] = fontWeight - +const Title: React.FC> = ({children, className, hidden = false, as = 'h2'}) => { return ( {children} @@ -348,83 +283,51 @@ const Title: React.FC> = ({ const TrailingVisual: React.FC> = ({ children, className, - sx: sxProp = defaultSxProp, hidden = false, }) => { - const style: CSSCustomProperties = {} - // @ts-ignore sx has height attribute - const {height} = sxProp - if (height) style['--custom-height'] = height return ( - {children} - + ) } const TrailingAction: React.FC> = ({ children, className, - sx: sxProp = defaultSxProp, hidden = hiddenOnNarrow, }) => { - const style: CSSCustomProperties = {} - // @ts-ignore sx has height attribute - const {height} = sxProp - if (height) style['--custom-height'] = height return ( - {children} - + ) } -const Actions: React.FC> = ({ - children, - className, - sx: sxProp = defaultSxProp, - hidden = false, -}) => { - const style: CSSCustomProperties = {} - // @ts-ignore sx has height attribute - const {height} = sxProp - if (height) style['--custom-height'] = height +export type ActionsProps = React.PropsWithChildren + +const Actions = ({children, className, hidden = false}: ActionsProps) => { return ( - +
{children} - +
) } // PageHeader.Description: The description area of the header. Visible on all viewports -const Description: React.FC> = ({ - children, - className, - sx: sxProp = defaultSxProp, - hidden = false, -}) => { +const Description: React.FC> = ({children, className, hidden = false}) => { return ( - +
{children} - +
) } @@ -438,30 +341,27 @@ export type NavigationProps = { const Navigation: React.FC> = ({ children, className, - sx: sxProp = defaultSxProp, hidden = false, - as, + as: BaseComponent = 'div', 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, }) => { warning( - as === 'nav' && !ariaLabel && !ariaLabelledBy, + BaseComponent === 'nav' && !ariaLabel && !ariaLabelledBy, 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', ) return ( - {children} - + ) } diff --git a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap index a043ac85029..ffe05ff87bf 100644 --- a/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap +++ b/packages/react/src/__tests__/__snapshots__/exports.test.ts.snap @@ -104,7 +104,9 @@ exports[`@primer/react > should not update exports without a semver change 1`] = "Overlay", "type OverlayProps", "PageHeader", + "type PageHeaderActionsProps", "type PageHeaderProps", + "type PageHeaderTitleProps", "PageLayout", "type PageLayoutContentProps", "type PageLayoutFooterProps", @@ -268,7 +270,6 @@ exports[`@primer/react/experimental > should not update exports without a semver "ButtonBase", "type ButtonBaseProps", "type CellAlignment", - "type ChildrenPropTypes", "type Column", "type ColumnWidth", "createColumnHelper", @@ -296,7 +297,6 @@ exports[`@primer/react/experimental > should not update exports without a semver "type IssueLabelProps", "KeybindingHint", "type KeybindingHintProps", - "type NavigationProps", "NavList", "type NavListDividerProps", "type NavListGroupProps", @@ -307,8 +307,9 @@ exports[`@primer/react/experimental > should not update exports without a semver "type NavListTrailingVisualProps", "type ObjectPaths", "PageHeader", + "type PageHeaderActionsProps", "type PageHeaderProps", - "type ParentLinkProps", + "type PageHeaderTitleProps", "ScrollableRegion", "type ScrollableRegionProps", "SelectPanel", diff --git a/packages/react/src/experimental/index.ts b/packages/react/src/experimental/index.ts index d4eca5848d6..a23fc344fcc 100644 --- a/packages/react/src/experimental/index.ts +++ b/packages/react/src/experimental/index.ts @@ -39,7 +39,13 @@ export * from '../Dialog/Dialog' export {InlineMessage} from '../InlineMessage' export type {InlineMessageProps} from '../InlineMessage' -export * from '../PageHeader' +export {PageHeader} from '../PageHeader' +export type { + PageHeaderProps, + TitleProps, + TitleProps as PageHeaderTitleProps, + ActionsProps as PageHeaderActionsProps, +} from '../PageHeader' export * from '../Hidden' diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index d7b4f74ac40..9800c792fd1 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -207,7 +207,11 @@ export {Stack} from './Stack' export type {StackProps, StackItemProps} from './Stack' export {PageHeader} from './PageHeader' -export type {PageHeaderProps} from './PageHeader' +export type { + PageHeaderProps, + TitleProps as PageHeaderTitleProps, + ActionsProps as PageHeaderActionsProps, +} from './PageHeader' export {default as sx, merge} from './sx' export type {BetterCssProperties, BetterSystemStyleObject, SxProp} from './sx' diff --git a/packages/styled-react/ARCHITECTURE.md b/packages/styled-react/ARCHITECTURE.md index a5ea328aaf8..04c93e99974 100644 --- a/packages/styled-react/ARCHITECTURE.md +++ b/packages/styled-react/ARCHITECTURE.md @@ -61,17 +61,21 @@ import { type ExampleComponentProps as PrimerExampleComponentProps, } from '@primer/react' import {forwardRef} from 'react' -import {PolymorphicForwardRef as ForwardRefComponent} from '../polymorphic' +import {ForwardRefComponent} from '../polymorphic' +import {sx} from '../sx' type ExampleComponentProps = PrimerExampleComponentProps & SxProp -const ExampleComponent = forwardRef(function ExampleComponent(props, ref) { - // @ts-expect-error the polymorphic component type is not inferred - // correctly - return -}) as ForwardRefComponent<'div', ExampleComponentProps> +const ExampleComponent: ForwardRefComponent<'div', ExampleComponentProps> = styled( + PrimerExampleComponent, +).withConfig({ + shouldForwardProp: prop => prop !== 'sx', +})` + ${sx} +` export {ExampleComponent} +export type {ExampleComponentProps} ``` ## Sub-components diff --git a/packages/styled-react/package.json b/packages/styled-react/package.json index 2cc99763e69..15b74a008bd 100644 --- a/packages/styled-react/package.json +++ b/packages/styled-react/package.json @@ -35,6 +35,7 @@ "@types/react": "18.3.11", "@types/react-dom": "18.3.1", "@vitejs/plugin-react": "^4.3.3", + "babel-plugin-styled-components": "2.1.4", "publint": "^0.3.12", "react": "18.3.1", "react-dom": "18.3.1", diff --git a/packages/styled-react/rollup.config.js b/packages/styled-react/rollup.config.js index f9a5266988b..8aae93d5045 100644 --- a/packages/styled-react/rollup.config.js +++ b/packages/styled-react/rollup.config.js @@ -22,6 +22,7 @@ export default defineConfig({ }), babel({ presets: ['@babel/preset-typescript', ['@babel/preset-react', {runtime: 'automatic'}]], + plugins: ['babel-plugin-styled-components'], extensions: ['.ts', '.tsx'], babelHelpers: 'bundled', }), diff --git a/packages/styled-react/src/components/PageHeader.tsx b/packages/styled-react/src/components/PageHeader.tsx new file mode 100644 index 00000000000..739c677c3e1 --- /dev/null +++ b/packages/styled-react/src/components/PageHeader.tsx @@ -0,0 +1,72 @@ +import { + PageHeader as PrimerPageHeader, + type PageHeaderProps as PrimerPageHeaderProps, + type PageHeaderTitleProps as PrimerPageHeaderTitleProps, + type PageHeaderActionsProps as PrimerPageHeaderActionsProps, +} from '@primer/react' +import styled from 'styled-components' +import {sx, type SxProp} from '../sx' +import type {ForwardRefComponent} from '../polymorphic' +import {Box} from './Box' + +type PageHeaderProps = PrimerPageHeaderProps & SxProp + +const PageHeaderImpl: ForwardRefComponent<'div', PageHeaderProps> = styled( + PrimerPageHeader, +).withConfig({ + shouldForwardProp: prop => prop !== 'sx', +})` + ${sx} +` + +type PageHeaderActionsProps = PrimerPageHeaderActionsProps & SxProp + +function PageHeaderActions({sx, ...rest}: PageHeaderActionsProps) { + const style: CSSCustomProperties = {} + if (sx) { + // @ts-ignore sx has height attribute + const {height} = sx + if (height) { + style['--custom-height'] = height + } + } + + // @ts-expect-error type mismatch between Box usage here and PrimerPageHeader.Actions + return +} + +type PageHeaderTitleProps = PrimerPageHeaderTitleProps & SxProp + +type CSSCustomProperties = { + [key: `--${string}`]: string | number +} + +function PageHeaderTitle({sx, ...rest}: PageHeaderTitleProps) { + const style: CSSCustomProperties = {} + if (sx) { + // @ts-ignore sx can have color attribute + const {fontSize, lineHeight, fontWeight} = sx + if (fontSize) { + style['--custom-font-size'] = fontSize + } + + if (lineHeight) { + style['--custom-line-height'] = lineHeight + } + + if (fontWeight) { + style['--custom-font-weight'] = fontWeight + } + } + + // @ts-expect-error type mismatch between Box usage here and PrimerPageHeader.Title + return +} + +const PageHeader = Object.assign(PageHeaderImpl, { + Actions: PageHeaderActions, + Title: PageHeaderTitle, +}) + +export {PageHeader} +export type {PageHeaderProps, PageHeaderActionsProps, PageHeaderTitleProps} diff --git a/packages/styled-react/src/experimental.tsx b/packages/styled-react/src/experimental.tsx index 5ed22b2d9c1..7b1764bbcb9 100644 --- a/packages/styled-react/src/experimental.tsx +++ b/packages/styled-react/src/experimental.tsx @@ -1 +1,7 @@ -export {Dialog, PageHeader, Table, Tooltip, UnderlinePanels} from '@primer/react/experimental' +export { + PageHeader, + type PageHeaderProps, + type PageHeaderActionsProps, + type PageHeaderTitleProps, +} from './components/PageHeader' +export {Dialog, Table, Tooltip, UnderlinePanels} from '@primer/react/experimental' diff --git a/packages/styled-react/src/index.tsx b/packages/styled-react/src/index.tsx index 621ffec54fd..e02e6fa1580 100644 --- a/packages/styled-react/src/index.tsx +++ b/packages/styled-react/src/index.tsx @@ -170,6 +170,13 @@ const ToggleSwitch = forwardRef(function T return }) +export { + PageHeader, + type PageHeaderProps, + type PageHeaderActionsProps, + type PageHeaderTitleProps, +} from './components/PageHeader' + type TruncateProps = PropsWithChildren & SxProp const Truncate: ForwardRefComponent<'div', TruncateProps> = styled(PrimerTruncate).withConfig({ @@ -235,7 +242,6 @@ export { Link, NavList, Overlay, - PageHeader, PageLayout, ProgressBar, Select, diff --git a/packages/styled-react/vitest.config.browser.ts b/packages/styled-react/vitest.config.browser.ts index 00b25150af3..dd9772c045c 100644 --- a/packages/styled-react/vitest.config.browser.ts +++ b/packages/styled-react/vitest.config.browser.ts @@ -1,3 +1,4 @@ +import path from 'node:path' import react from '@vitejs/plugin-react' import {defineConfig} from 'vitest/config' @@ -6,6 +7,22 @@ export default defineConfig({ define: { __DEV__: true, }, + resolve: { + alias: [ + { + find: '@primer/react/experimental', + replacement: path.resolve(import.meta.dirname, '..', 'react', 'src', 'experimental', 'index.ts'), + }, + { + find: '@primer/react/deprecated', + replacement: path.resolve(import.meta.dirname, '..', 'react', 'src', 'deprecated', 'index.ts'), + }, + { + find: '@primer/react', + replacement: path.resolve(import.meta.dirname, '..', 'react', 'src', 'index.ts'), + }, + ], + }, test: { name: '@primer/styled-react (browser)', include: ['src/**/*.browser.test.?(c|m)[jt]s?(x)'],