From 89e5a3a924fe3db1c595cd7f58b09c7d4dd2ee37 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Fri, 4 Apr 2025 02:35:19 +0000 Subject: [PATCH 1/3] Remove the CSS modules feature flag from the PageHeader component --- .changeset/tough-dodos-retire.md | 5 + .../react/src/PageHeader/PageHeader.test.tsx | 6 +- packages/react/src/PageHeader/PageHeader.tsx | 598 +++++------------- .../src/internal/utils/toggleSxComponent.tsx | 4 +- 4 files changed, 157 insertions(+), 456 deletions(-) create mode 100644 .changeset/tough-dodos-retire.md diff --git a/.changeset/tough-dodos-retire.md b/.changeset/tough-dodos-retire.md new file mode 100644 index 00000000000..6bca21f4bf4 --- /dev/null +++ b/.changeset/tough-dodos-retire.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Remove the CSS modules feature flag from the PageHeader component diff --git a/packages/react/src/PageHeader/PageHeader.test.tsx b/packages/react/src/PageHeader/PageHeader.test.tsx index 48dbf0f08f2..e1d03426666 100644 --- a/packages/react/src/PageHeader/PageHeader.test.tsx +++ b/packages/react/src/PageHeader/PageHeader.test.tsx @@ -45,7 +45,7 @@ describe('PageHeader', () => { * They are testing the internal implementation of the component and checking if the component * is rendering the correct styles.This approach was necessary due to the impracticality of CSS media queries testing with Jest. */ - it('respects default visibility of ContextArea and renders CSS media styles correctly', () => { + it.skip('respects default visibility of ContextArea and renders CSS media styles correctly', () => { const expectedStyles = { '-ms-flex-align': 'center', '-ms-flex-direction': 'row', @@ -71,7 +71,7 @@ describe('PageHeader', () => { expect.objectContaining(expectedStyles), ) }) - it('respects the hidden prop of ContextArea and renders CSS media styles correctly', () => { + it.skip('respects the hidden prop of ContextArea and renders CSS media styles correctly', () => { const expectedStyles = { '-ms-flex-align': 'center', '-ms-flex-direction': 'row', @@ -110,7 +110,7 @@ describe('PageHeader', () => { ), ).toEqual(expect.objectContaining(expectedStyles)) }) - it('respects default visibility of LeadingAction and TrailingAction and renders CSS media styles correctly', () => { + it.skip('respects default visibility of LeadingAction and TrailingAction and renders CSS media styles correctly', () => { const expectedStyles = { '-ms-flex-align': 'center', '-webkit-align-items': 'center', diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index d408c3380d3..6eb2447f05c 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -2,49 +2,22 @@ import React, {useEffect} from 'react' import Box from '../Box' import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {isResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' -import type {SxProp, BetterSystemStyleObject, CSSCustomProperties} from '../sx' -import {merge} from '../sx' +import type {SxProp, CSSCustomProperties} from '../sx' import Heading from '../Heading' import {ArrowLeftIcon} from '@primer/octicons-react' import type {LinkProps as BaseLinkProps} from '../Link' import Link from '../Link' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' -import { - areAllValuesTheSame, - getBreakpointDeclarations, - haveRegularAndWideSameValue, -} from '../utils/getBreakpointDeclarations' +import {areAllValuesTheSame, haveRegularAndWideSameValue} from '../utils/getBreakpointDeclarations' import {warning} from '../utils/warning' import {useProvidedRefOrCreate} from '../hooks' import type {AriaRole} from '../utils/types' -import {useFeatureFlag} from '../FeatureFlags' import {clsx} from 'clsx' import classes from './PageHeader.module.css' - -const GRID_ROW_ORDER = { - ContextArea: 1, - LeadingAction: 2, - Breadcrumbs: 2, - TitleArea: 2, - TrailingAction: 2, - Actions: 2, - Description: 3, - Navigation: 4, -} - -const TITLE_AREA_REGION_ORDER = { - LeadingVisual: 0, - Title: 1, - TrailingVisual: 2, -} - -const CONTEXT_AREA_REGION_ORDER = { - ParentLink: 0, - ContextBar: 1, - ContextAreaActions: 2, -} +import {toggleSxComponent} from '../internal/utils/toggleSxComponent' +import {defaultSxProp} from '../utils/defaultSxProp' // Types that are shared between PageHeader children components export type ChildrenPropTypes = { @@ -66,8 +39,6 @@ const hiddenOnNarrow = { wide: false, } -const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_ga' - // Root // ----------------------------------------------------------------------------- export type PageHeaderProps = { @@ -80,68 +51,6 @@ export type PageHeaderProps = { const Root = React.forwardRef>( ({children, className, sx = {}, as = 'div', 'aria-label': ariaLabel, role, hasBorder}, forwardedRef) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - - const rootStyles = { - display: 'grid', - // We have max 5 columns. - gridTemplateColumns: 'auto auto auto auto 1fr', - gridTemplateAreas: ` - 'context-area context-area context-area context-area context-area' - 'leading-action breadcrumbs title-area trailing-action actions' - 'description description description description description' - 'navigation navigation navigation navigation navigation' - `, - // line-height is calculated with calc(height/font-size) and the below numbers are from @primer/primitives - // --custom-font-size, --custom-line-height, --custom-font-weight are custom properties (passed by sx) that can be used to override the below values - // We don't want these values to be overridden but still want to allow consumers to override them if needed. - '&:has([data-component="TitleArea"][data-size-variant="large"])': { - fontSize: 'var(--custom-font-size, var(--text-title-size-large, 2rem))', - lineHeight: 'var(--custom-line-height, var(--text-title-lineHeight-large, 1.5))', // calc(48/32) - fontWeight: 'var(--custom-font-weight, var(--base-text-weight-normal, 400))', - '--title-line-height': 'var(--custom-line-height, var(--text-title-lineHeight-large, 1.5))', - }, - '&:has([data-component="TitleArea"][data-size-variant="medium"])': { - fontSize: 'var(--custom-font-size, var(--text-title-size-medium, 1.25rem))', - lineHeight: 'var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6))', // calc(32/20) - fontWeight: 'var(--custom-font-weight, var(--base-text-weight-semibold, 600))', - '--title-line-height': 'var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6))', - }, - '&:has([data-component="TitleArea"][data-size-variant="subtitle"])': { - fontSize: 'var(--custom-font-size, var(--text-title-size-medium, 1.25rem))', - lineHeight: 'var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6))', // calc(32/20) - fontWeight: 'var(--custom-font-weight, var(--base-text-weight-normal, 400))', - '--title-line-height': 'var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6))', - }, - '[data-component="PH_LeadingAction"], [data-component="PH_TrailingAction"],[data-component="PH_Actions"], [data-component="PH_LeadingVisual"], [data-component="PH_TrailingVisual"]': - { - height: 'calc(var(--title-line-height) * 1em)', - }, - '&[data-has-border="true"]:has([data-component="PH_Navigation"][data-hidden-all]), &[data-has-border="true"]:not(:has([data-component="PH_Navigation"]))': - { - borderBlockEnd: 'var(--borderWidth-thin) solid var(--borderColor-default)', - paddingBlockEnd: 'var(--base-size-8)', - }, - '@media screen and (max-width: 768px)': { - '&[data-has-border="true"]:has([data-component="PH_Navigation"][data-hidden-narrow])': { - borderBlockEnd: 'var(--borderWidth-thin) solid var(--borderColor-default)', - paddingBlockEnd: 'var(--base-size-8)', - }, - }, - '@media screen and (min-width: 768px)': { - '&[data-has-border="true"]:has([data-component="PH_Navigation"][data-hidden-regular])': { - borderBlockEnd: 'var(--borderWidth-thin) solid var(--borderColor-default)', - paddingBlockEnd: 'var(--base-size-8)', - }, - }, - '@media screen and (min-width: 1440px)': { - '&[data-has-border="true"]:has([data-component="PH_Navigation"][data-hidden-wide])': { - borderBlockEnd: 'var(--borderWidth-thin) solid var(--borderColor-default)', - paddingBlockEnd: 'var(--base-size-8)', - }, - }, - } - const rootRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const isInteractive = (element: HTMLElement) => { @@ -195,18 +104,21 @@ const Root = React.forwardRef> + > + return ( - (rootStyles, sx)} + sx={sx} aria-label={ariaLabel} role={role} > {children} - + ) }, ) as PolymorphicForwardRefComponent<'div', PageHeaderProps> @@ -219,33 +131,15 @@ const ContextArea: React.FC> = ({ children, className, hidden = hiddenOnRegularAndWide, - sx = {}, + sx: sxProp = defaultSxProp, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const contentNavStyles = { - gridRow: GRID_ROW_ORDER.ContextArea, - gridArea: 'context-area', - display: 'flex', - flexDirection: 'row', - alignItems: 'center', - paddingBottom: '0.5rem', - gap: '0.5rem', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - fontWeight: 'initial', - lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', - fontSize: 'var(--text-body-size-medium, 0.875rem)', - } - + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> + > return ( - (contentNavStyles, sx)} - {...getHiddenDataAttributes(enabled, hidden)} - > + {children} - + ) } type LinkProps = Pick< @@ -258,8 +152,18 @@ export type ParentLinkProps = React.PropsWithChildren( - ({children, className, sx = {}, href, 'aria-label': ariaLabel, as = 'a', hidden = hiddenOnRegularAndWide}, ref) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + ( + { + children, + className, + sx: sxProp = defaultSxProp, + href, + 'aria-label': ariaLabel, + as = 'a', + hidden = hiddenOnRegularAndWide, + }, + ref, + ) => { return ( <> ( as={as} aria-label={ariaLabel} muted - className={clsx(enabled && classes.ParentLink, className)} - sx={ - enabled - ? sx - : merge( - { - display: 'flex', - alignItems: 'center', - order: CONTEXT_AREA_REGION_ORDER.ParentLink, - gap: '0.5rem', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} + className={clsx(classes.ParentLink, className)} + sx={sxProp} + {...getHiddenDataAttributes(hidden)} href={href} > @@ -303,31 +192,16 @@ ParentLink.displayName = 'ParentLink' const ContextBar: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = hiddenOnRegularAndWide, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> + > return ( - ( - { - display: 'flex', - order: CONTEXT_AREA_REGION_ORDER.ContextBar, - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} - > + {children} - + ) } @@ -336,37 +210,21 @@ const ContextBar: React.FC> = ({ const ContextAreaActions: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = hiddenOnRegularAndWide, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> + > return ( - ( - { - display: 'flex', - flexDirection: 'row', - order: CONTEXT_AREA_REGION_ORDER.ContextAreaActions, - alignItems: 'center', - gap: '0.5rem', - flexGrow: '1', - justifyContent: 'right', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} + {children} - + ) } @@ -378,38 +236,23 @@ type TitleAreaProps = { // --------------------------------------------------------------------- const TitleArea = React.forwardRef>( - ({children, className, sx = {}, hidden = false, variant = 'medium'}, forwardedRef) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + ({children, className, sx: sxProp = defaultSxProp, hidden = false, variant = 'medium'}, forwardedRef) => { const titleAreaRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const currentVariant = useResponsiveValue(variant, 'medium') + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> + > return ( - ( - { - gridRow: GRID_ROW_ORDER.TitleArea, - gridArea: 'title-area', - display: 'flex', - gap: '0.5rem', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - flexDirection: 'row', - alignItems: 'flex-start', - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} + sx={sxProp} + {...getHiddenDataAttributes(hidden)} > {children} - + ) }, ) as PolymorphicForwardRefComponent<'div', TitleAreaProps> @@ -420,40 +263,27 @@ TitleArea.displayName = 'TitleArea' const LeadingAction: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = hiddenOnNarrow, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute - const {height} = sx + const {height} = sxProp if (height) style['--custom-height'] = height + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> & + React.HtmlHTMLAttributes + > return ( - ( - { - gridRow: GRID_ROW_ORDER.LeadingAction, - gridArea: 'leading-action', - paddingRight: '0.5rem', - display: 'flex', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - alignItems: 'center', - }, - sx, - ) - } + sx={sxProp} style={style} - {...getHiddenDataAttributes(enabled, hidden)} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } @@ -461,39 +291,21 @@ const LeadingAction: React.FC> = ({ const Breadcrumbs: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> + > return ( - ( - { - gridRow: GRID_ROW_ORDER.Breadcrumbs, - gridArea: 'breadcrumbs', - paddingRight: '0.5rem', - display: 'flex', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - alignItems: 'center', - fontWeight: 'initial', - lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', - fontSize: 'var(--text-body-size-medium, 0.875rem)', - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} + sx={sxProp} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } @@ -501,39 +313,28 @@ const Breadcrumbs: React.FC> = ({ const LeadingVisual: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute - const {height} = sx + const {height} = sxProp if (height) style['--custom-height'] = height + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > + > return ( - ( - { - // using flex and order to display the leading visual in the title area. - display: 'flex', - order: TITLE_AREA_REGION_ORDER.LeadingVisual, - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - alignItems: 'center', - }, - sx, - ) - } + sx={sxProp} style={style} - {...getHiddenDataAttributes(enabled, hidden)} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } @@ -544,43 +345,26 @@ export type TitleProps = { const Title: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, as = 'h2', }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} // @ts-ignore sxProp can have color attribute - const {fontSize, lineHeight, fontWeight} = sx + 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 return ( ( - { - // using flex and order to display the title in the title area. - display: 'flex', - order: TITLE_AREA_REGION_ORDER.Title, - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'block' - }), - fontSize: 'inherit', - fontWeight: 'inherit', - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} + sx={sxProp} + {...getHiddenDataAttributes(hidden)} > {children} @@ -591,123 +375,84 @@ const Title: React.FC> = ({ const TrailingVisual: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute - const {height} = sx + const {height} = sxProp if (height) style['--custom-height'] = height + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > + > return ( - ( - { - // using flex and order to display the trailing visual in the title area. - display: 'flex', - order: TITLE_AREA_REGION_ORDER.TrailingVisual, - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - alignItems: 'center', - }, - sx, - ) - } + sx={sxProp} style={style} - {...getHiddenDataAttributes(enabled, hidden)} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } const TrailingAction: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = hiddenOnNarrow, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute - const {height} = sx + const {height} = sxProp if (height) style['--custom-height'] = height + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > + > return ( - ( - { - gridRow: GRID_ROW_ORDER.TrailingAction, - gridArea: 'trailing-action', - paddingLeft: '0.5rem', - display: 'flex', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - alignItems: 'center', - }, - sx, - ) - } + sx={sxProp} style={style} - {...getHiddenDataAttributes(enabled, hidden)} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } const Actions: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute - const {height} = sx + const {height} = sxProp if (height) style['--custom-height'] = height + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > + > return ( - ( - { - gridRow: GRID_ROW_ORDER.Actions, - gridArea: 'actions', - display: 'flex', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - flexDirection: 'row', - paddingLeft: '0.5rem', - gap: '0.5rem', - minWidth: 'max-content', - justifyContent: 'right', - alignItems: 'center', - }, - sx, - ) - } + sx={sxProp} style={style} - {...getHiddenDataAttributes(enabled, hidden)} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } @@ -715,39 +460,16 @@ const Actions: React.FC> = ({ const Description: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const BaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> + > return ( - ( - { - gridRow: GRID_ROW_ORDER.Description, - gridArea: 'description', - display: 'flex', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - flexDirection: 'row', - alignItems: 'center', - paddingTop: '0.5rem', - gap: '0.5rem', - fontWeight: 'initial', - lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', - fontSize: 'var(--text-body-size-medium, 0.875rem)', - }, - sx, - ) - } - {...getHiddenDataAttributes(enabled, hidden)} - > + {children} - + ) } @@ -761,68 +483,42 @@ export type NavigationProps = { const Navigation: React.FC> = ({ children, className, - sx = {}, + sx: sxProp = defaultSxProp, hidden = false, as, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, }) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) warning( as === 'nav' && !ariaLabel && !ariaLabelledBy, 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', ) + const BaseComponent = toggleSxComponent(as) as React.ComponentType< + React.PropsWithChildren> + > return ( - ( - { - gridRow: GRID_ROW_ORDER.Navigation, - gridArea: 'navigation', - paddingTop: '0.5rem', - display: 'flex', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'block' - }), - fontWeight: 'initial', - lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', - fontSize: 'var(--text-body-size-medium, 0.875rem)', - }, - sx, - ) - } - // passing `true` always get the data attributes for the hidden prop, - // not just for CSS modules - {...getHiddenDataAttributes(true, hidden)} + sx={sxProp} + {...getHiddenDataAttributes(hidden)} > {children} - + ) } // Based on getBreakpointDeclarations, this function will return the // correct data attribute for the given hidden value for CSS modules. -function getHiddenDataAttributes( - isCssModules: boolean, - isHidden: boolean | ResponsiveValue, -): { +function getHiddenDataAttributes(isHidden: boolean | ResponsiveValue): { 'data-hidden-all'?: boolean 'data-hidden-narrow'?: boolean 'data-hidden-regular'?: boolean 'data-hidden-wide'?: boolean } { - if (!isCssModules) { - return {} - } - if (isResponsiveValue(isHidden)) { const responsiveValue = isHidden diff --git a/packages/react/src/internal/utils/toggleSxComponent.tsx b/packages/react/src/internal/utils/toggleSxComponent.tsx index 144361c9ee6..ac55615f43c 100644 --- a/packages/react/src/internal/utils/toggleSxComponent.tsx +++ b/packages/react/src/internal/utils/toggleSxComponent.tsx @@ -17,10 +17,10 @@ type CSSModulesProps = { */ export function toggleSxComponent( // eslint-disable-next-line @typescript-eslint/no-explicit-any - defaultAs: string | React.ComponentType, + defaultAs?: string | React.ComponentType, ) { const Wrapper = React.forwardRef(function Wrapper( - {as: BaseComponent = defaultAs, sx: sxProp = defaultSxProp, ...rest}, + {as: BaseComponent = defaultAs || 'div', sx: sxProp = defaultSxProp, ...rest}, ref, ) { if (sxProp !== defaultSxProp) { From 615da8de59ab538a536d1473e53c436ae058a361 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Tue, 8 Apr 2025 23:11:47 +0000 Subject: [PATCH 2/3] Pass in div instead of updating toggleSxComponent --- packages/react/src/PageHeader/PageHeader.tsx | 2 +- packages/react/src/internal/utils/toggleSxComponent.tsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 6eb2447f05c..95c5f10e4e8 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -493,7 +493,7 @@ const Navigation: React.FC> = ({ as === 'nav' && !ariaLabel && !ariaLabelledBy, 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', ) - const BaseComponent = toggleSxComponent(as) as React.ComponentType< + const BaseComponent = toggleSxComponent(as || 'div') as React.ComponentType< React.PropsWithChildren> > return ( diff --git a/packages/react/src/internal/utils/toggleSxComponent.tsx b/packages/react/src/internal/utils/toggleSxComponent.tsx index ac55615f43c..144361c9ee6 100644 --- a/packages/react/src/internal/utils/toggleSxComponent.tsx +++ b/packages/react/src/internal/utils/toggleSxComponent.tsx @@ -17,10 +17,10 @@ type CSSModulesProps = { */ export function toggleSxComponent( // eslint-disable-next-line @typescript-eslint/no-explicit-any - defaultAs?: string | React.ComponentType, + defaultAs: string | React.ComponentType, ) { const Wrapper = React.forwardRef(function Wrapper( - {as: BaseComponent = defaultAs || 'div', sx: sxProp = defaultSxProp, ...rest}, + {as: BaseComponent = defaultAs, sx: sxProp = defaultSxProp, ...rest}, ref, ) { if (sxProp !== defaultSxProp) { From f3e0c80ad31c06aaf9cf7ba4be511a0636fff3dc Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 10 Apr 2025 17:00:24 +0000 Subject: [PATCH 3/3] Move BaseComponents out of Component creations --- packages/react/src/PageHeader/PageHeader.tsx | 165 ++++++++++--------- 1 file changed, 91 insertions(+), 74 deletions(-) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 95c5f10e4e8..666250c04e4 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -49,6 +49,9 @@ export type PageHeaderProps = { hasBorder?: boolean } & SxProp +const RootBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> const Root = React.forwardRef>( ({children, className, sx = {}, as = 'div', 'aria-label': ariaLabel, role, hasBorder}, forwardedRef) => { const rootRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) @@ -104,12 +107,10 @@ const Root = React.forwardRef> - > return ( - {children} - + ) }, ) as PolymorphicForwardRefComponent<'div', PageHeaderProps> +const ContextAreaBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> // PageHeader.ContextArea : Only visible on narrow viewports by default to provide user context of where they are at their journey. `hidden` prop available // to manage their custom visibility but consumers should be careful if they choose to hide this on narrow viewports. // PageHeader.ContextArea Sub Components: PageHeader.ParentLink, PageHeader.ContextBar, PageHeader.ContextAreaActions @@ -133,13 +137,14 @@ const ContextArea: React.FC> = ({ hidden = hiddenOnRegularAndWide, sx: sxProp = defaultSxProp, }) => { - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> - > return ( - + {children} - + ) } type LinkProps = Pick< @@ -189,22 +194,29 @@ ParentLink.displayName = 'ParentLink' // Generic slot for any component above the title region. Use it for custom breadcrumbs and other navigation elements instead of ParentLink. // --------------------------------------------------------------------- +const ContextBarBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> const ContextBar: React.FC> = ({ children, className, sx: sxProp = defaultSxProp, hidden = hiddenOnRegularAndWide, }) => { - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> - > return ( - + {children} - + ) } +const ContextAreaActionsBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> // ContextAreaActions // --------------------------------------------------------------------- const ContextAreaActions: React.FC> = ({ @@ -213,18 +225,15 @@ const ContextAreaActions: React.FC> = sx: sxProp = defaultSxProp, hidden = hiddenOnRegularAndWide, }) => { - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> - > return ( - {children} - + ) } @@ -235,15 +244,15 @@ type TitleAreaProps = { // PageHeader.TitleArea Sub Components: PageHeader.LeadingVisual, PageHeader.Title, PageTitle.TrailingVisual // --------------------------------------------------------------------- +const TitleAreaBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> const TitleArea = React.forwardRef>( ({children, className, sx: sxProp = defaultSxProp, hidden = false, variant = 'medium'}, forwardedRef) => { const titleAreaRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const currentVariant = useResponsiveValue(variant, 'medium') - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> - > return ( - {children} - + ) }, ) as PolymorphicForwardRefComponent<'div', TitleAreaProps> TitleArea.displayName = 'TitleArea' +const LeadingActionBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> & + React.HtmlHTMLAttributes +> // PageHeader.LeadingAction and PageHeader.TrailingAction should only be visible on regular viewports. // So they come as hidden on narrow viewports by default and their visibility can be managed by their `hidden` prop. const LeadingAction: React.FC> = ({ @@ -270,12 +283,8 @@ const LeadingAction: React.FC> = ({ // @ts-ignore sx has height attribute const {height} = sxProp if (height) style['--custom-height'] = height - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> & - React.HtmlHTMLAttributes - > return ( - > = ({ {...getHiddenDataAttributes(hidden)} > {children} - + ) } +const BreadcrumbsBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> // This is reserved for only breadcrumbs. const Breadcrumbs: React.FC> = ({ children, @@ -294,21 +306,23 @@ const Breadcrumbs: React.FC> = ({ sx: sxProp = defaultSxProp, hidden = false, }) => { - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> - > return ( - {children} - + ) } +const LeadingVisualBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > +> // PageHeader.LeadingVisual and PageHeader.TrailingVisual should remain visible on narrow viewports. const LeadingVisual: React.FC> = ({ children, @@ -320,13 +334,8 @@ const LeadingVisual: React.FC> = ({ // @ts-ignore sx has height attribute const {height} = sxProp if (height) style['--custom-height'] = height - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren< - ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes - > - > return ( - > = ({ {...getHiddenDataAttributes(hidden)} > {children} - + ) } @@ -371,6 +380,11 @@ const Title: React.FC> = ({ ) } +const TrailingVisualBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > +> // PageHeader.LeadingVisual and PageHeader.TrailingVisual should remain visible on narrow viewports. const TrailingVisual: React.FC> = ({ children, @@ -382,13 +396,8 @@ const TrailingVisual: React.FC> = ({ // @ts-ignore sx has height attribute const {height} = sxProp if (height) style['--custom-height'] = height - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren< - ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes - > - > return ( - > = ({ {...getHiddenDataAttributes(hidden)} > {children} - + ) } +const TrailingActionBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > +> const TrailingAction: React.FC> = ({ children, className, @@ -410,13 +424,8 @@ const TrailingAction: React.FC> = ({ // @ts-ignore sx has height attribute const {height} = sxProp if (height) style['--custom-height'] = height - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren< - ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes - > - > return ( - > = ({ {...getHiddenDataAttributes(hidden)} > {children} - + ) } +const ActionsBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren< + ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes + > +> const Actions: React.FC> = ({ children, className, @@ -438,13 +452,8 @@ const Actions: React.FC> = ({ // @ts-ignore sx has height attribute const {height} = sxProp if (height) style['--custom-height'] = height - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren< - ChildrenPropTypes & React.RefAttributes & React.HtmlHTMLAttributes - > - > return ( - > = ({ {...getHiddenDataAttributes(hidden)} > {children} - + ) } +const DescriptionBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> + // PageHeader.Description: The description area of the header. Visible on all viewports const Description: React.FC> = ({ children, @@ -463,13 +476,14 @@ const Description: React.FC> = ({ sx: sxProp = defaultSxProp, hidden = false, }) => { - const BaseComponent = toggleSxComponent('div') as React.ComponentType< - React.PropsWithChildren> - > return ( - + {children} - + ) } @@ -479,6 +493,10 @@ export type NavigationProps = { 'aria-labelledby'?: React.AriaAttributes['aria-labelledby'] } & ChildrenPropTypes +const NavigationBaseComponent = toggleSxComponent('div') as React.ComponentType< + React.PropsWithChildren> +> + // PageHeader.Navigation: The local navigation area of the header. Visible on all viewports const Navigation: React.FC> = ({ children, @@ -493,11 +511,10 @@ const Navigation: React.FC> = ({ as === 'nav' && !ariaLabel && !ariaLabelledBy, 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', ) - const BaseComponent = toggleSxComponent(as || 'div') as React.ComponentType< - React.PropsWithChildren> - > + return ( - > = ({ {...getHiddenDataAttributes(hidden)} > {children} - + ) }