From fe495edd84a36ab84846428ff1a6f255a3d95f9c Mon Sep 17 00:00:00 2001 From: Hussam Ghazzi Date: Mon, 25 Nov 2024 18:58:13 +0000 Subject: [PATCH 1/3] feat(PageHeader): Convert PageHeader to CSS modules behind feature flag --- .changeset/friendly-tomatoes-yell.md | 5 + .../src/PageHeader/PageHeader.module.css | 198 +++++++ packages/react/src/PageHeader/PageHeader.tsx | 557 +++++++++++------- .../src/utils/getBreakpointDeclarations.ts | 3 +- 4 files changed, 552 insertions(+), 211 deletions(-) create mode 100644 .changeset/friendly-tomatoes-yell.md create mode 100644 packages/react/src/PageHeader/PageHeader.module.css diff --git a/.changeset/friendly-tomatoes-yell.md b/.changeset/friendly-tomatoes-yell.md new file mode 100644 index 00000000000..01a1c79543e --- /dev/null +++ b/.changeset/friendly-tomatoes-yell.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Convert PageHeader to CSS modules behind feature flag diff --git a/packages/react/src/PageHeader/PageHeader.module.css b/packages/react/src/PageHeader/PageHeader.module.css new file mode 100644 index 00000000000..cdae3110906 --- /dev/null +++ b/packages/react/src/PageHeader/PageHeader.module.css @@ -0,0 +1,198 @@ +:root { + /* Grid Row Order */ + --grid-row-order-context-area: 1; + --grid-row-order-leading-action: 2; + --grid-row-order-breadcrumbs: 2; + --grid-row-order-title-area: 2; + --grid-row-order-trailing-action: 2; + --grid-row-order-actions: 2; + --grid-row-order-description: 3; + --grid-row-order-navigation: 4; + + /* Title Area Region Order */ + --title-area-region-order-leading-visual: 0; + --title-area-region-order-title: 1; + --title-area-region-order-trailing-visual: 2; + + /* Context Area Region Order */ + --context-area-region-order-parent-link: 0; + --context-area-region-order-context-bar: 1; + --context-area-region-order-context-area-actions: 2; +} + +.PageHeader { + display: grid; + + /* We have max 5 columns. */ + grid-template-columns: auto auto auto auto 1fr; + grid-template-areas: + '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 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']) { + font-size: var(--custom-font-size, var(--text-title-size-large, 2rem)); + font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); + line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); /* calc(48/32) */ + + --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-large, 1.5)); + } + + &:has([data-component='TitleArea'][data-size-variant='medium']) { + font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); + font-weight: var(--custom-font-weight, var(--base-text-weight-semibold, 600)); + line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ + + --title-line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); + } + + &:has([data-component='TitleArea'][data-size-variant='subtitle']) { + font-size: var(--custom-font-size, var(--text-title-size-medium, 1.25rem)); + font-weight: var(--custom-font-weight, var(--base-text-weight-normal, 400)); + line-height: var(--custom-line-height, var(--text-title-lineHeight-medium, 1.6)); /* calc(32/20) */ + + --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); + } +} + +.ContextArea { + display: flex; + padding-bottom: var(--base-size-8); + font-size: var(--text-body-size-medium, 0.875rem); + font-weight: var(--base-text-weight-light); + line-height: var(--text-body-lineHeight-medium, 1.4285); + flex-direction: row; + grid-row: var(--grid-row-order-context-area); + grid-area: context-area; + align-items: center; + gap: var(--stack-gap-condensed); +} + +.ParentLink { + display: flex; + align-items: center; + order: var(--context-area-region-order-parent-link); + gap: var(--stack-gap-condensed); +} + +.ContextBar { + display: flex; + order: var(--context-area-region-order-context-bar); +} + +.ContextAreaActions { + display: flex; + flex-direction: row; + order: var(--context-area-region-order-context-area-actions); + align-items: center; + gap: var(--stack-gap-condensed); + flex-grow: 1; + justify-content: flex-end; +} + +.TitleArea { + grid-row: var(--grid-row-order-title-area); + grid-area: title-area; + display: flex; + gap: var(--stack-gap-condensed); + flex-direction: row; + align-items: flex-start; +} + +.LeadingAction { + display: flex; + padding-right: var(--base-size-8); + grid-row: var(--grid-row-order-leading-action); + grid-area: leading-action; + align-items: center; +} + +.Breadcrumbs { + display: flex; + padding-right: var(--base-size-8); + font-size: var(--text-body-size-medium, 0.875rem); + font-weight: var(--base-text-weight-light); + line-height: var(--text-body-lineHeight-medium, 1.4285); + grid-row: var(--grid-row-order-breadcrumbs); + grid-area: breadcrumbs; + align-items: center; +} + +.LeadingVisual { + /* using flex and order to display the leading visual in the title area. */ + display: flex; + order: var(--title-area-region-order-leading-visual); + align-items: center; +} + +.Title { + /* using flex and order to display the title in the title area. */ + display: flex; + order: var(--title-area-region-order-title); + font-size: inherit; + font-weight: inherit; +} + +.TrailingVisual { + /* using flex and order to display the trailing visual in the title area. */ + display: flex; + order: var(--title-area-region-order-trailing-visual); + align-items: center; +} + +.TrailingAction { + display: flex; + padding-left: var(--base-size-8); + grid-row: var(--grid-row-order-trailing-action); + grid-area: trailing-action; + align-items: center; +} + +.Actions { + display: flex; + min-width: max-content; + padding-left: var(--base-size-8); + flex-direction: row; + grid-row: var(--grid-row-order-actions); + grid-area: actions; + gap: var(--stack-gap-condensed); + justify-content: flex-end; + align-items: center; +} + +.Description { + display: flex; + padding-top: var(--base-size-8); + font-size: var(--text-body-size-medium, 0.875rem); + font-weight: var(--base-text-weight-light); + line-height: var(--text-body-lineHeight-medium, 1.4285); + flex-direction: row; + grid-row: var(--grid-row-order-description); + grid-area: description; + align-items: center; + gap: var(--stack-gap-condensed); +} + +.Navigation { + display: flex; + padding-top: var(--base-size-8); + font-size: var(--text-body-size-medium, 0.875rem); + font-weight: var(--base-text-weight-light); + line-height: var(--text-body-lineHeight-medium, 1.4285); + grid-row: var(--grid-row-order-navigation); + grid-area: navigation; +} diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index d1ec83c2bcd..7376682cea6 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -14,6 +14,10 @@ import {getBreakpointDeclarations} 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, @@ -58,6 +62,8 @@ const hiddenOnNarrow = { wide: false, } +const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_team' + // Root // ----------------------------------------------------------------------------- export type PageHeaderProps = { @@ -69,6 +75,8 @@ export type PageHeaderProps = { const Root = React.forwardRef>( ({children, className, sx = {}, as = 'div', 'aria-label': ariaLabel, role}, forwardedRef) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const rootStyles = { display: 'grid', // We have max 5 columns. @@ -81,7 +89,7 @@ const Root = React.forwardRef(rootStyles, sx)} + className={clsx(enabled && classes.PageHeader, className)} + sx={enabled ? sx : merge(rootStyles, sx)} aria-label={ariaLabel} role={role} > @@ -184,6 +192,10 @@ const ContextArea: React.FC> = ({ hidden = hiddenOnRegularAndWide, sx = {}, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) const contentNavStyles = { gridRow: GRID_ROW_ORDER.ContextArea, gridArea: 'context-area', @@ -192,17 +204,18 @@ const ContextArea: React.FC> = ({ alignItems: 'center', paddingBottom: '0.5rem', gap: '0.5rem', - - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), + ...displayBreakpoints, fontWeight: 'initial', lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', fontSize: 'var(--text-body-size-medium, 0.875rem)', } return ( - (contentNavStyles, sx)}> + (contentNavStyles, sx)} + style={enabled ? displayBreakpoints : undefined} + > {children} ) @@ -218,6 +231,10 @@ export type ParentLinkProps = React.PropsWithChildren( ({children, className, sx = {}, href, 'aria-label': ariaLabel, as = 'a', hidden = hiddenOnRegularAndWide}, ref) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) return ( <> ( as={as} aria-label={ariaLabel} muted - className={className} - sx={merge( - { - display: 'flex', - alignItems: 'center', - order: CONTEXT_AREA_REGION_ORDER.ParentLink, - gap: '0.5rem', - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - }, - sx, - )} + className={clsx(enabled && classes.ParentLink, className)} + sx={ + enabled + ? sx + : merge( + { + display: 'flex', + alignItems: 'center', + order: CONTEXT_AREA_REGION_ORDER.ParentLink, + gap: '0.5rem', + ...displayBreakpoints, + }, + sx, + ) + } + style={enabled ? displayBreakpoints : undefined} href={href} > @@ -259,19 +279,26 @@ const ContextBar: React.FC> = ({ sx = {}, hidden = hiddenOnRegularAndWide, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) return ( ( - { - display: 'flex', - order: CONTEXT_AREA_REGION_ORDER.ContextBar, - ...getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }), - }, - sx, - )} + className={clsx(enabled && classes.ContextBar, className)} + sx={ + enabled + ? sx + : merge( + { + display: 'flex', + order: CONTEXT_AREA_REGION_ORDER.ContextBar, + ...displayBreakpoints, + }, + sx, + ) + } + style={enabled ? displayBreakpoints : undefined} > {children} @@ -286,24 +313,31 @@ const ContextAreaActions: React.FC> = sx = {}, hidden = hiddenOnRegularAndWide, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) 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, - )} + className={clsx(enabled && classes.ContextAreaActions, className)} + sx={ + enabled + ? sx + : merge( + { + display: 'flex', + flexDirection: 'row', + order: CONTEXT_AREA_REGION_ORDER.ContextAreaActions, + alignItems: 'center', + gap: '0.5rem', + flexGrow: '1', + justifyContent: 'right', + ...displayBreakpoints, + }, + sx, + ) + } + style={enabled ? displayBreakpoints : undefined} > {children} @@ -319,28 +353,35 @@ type TitleAreaProps = { const TitleArea = React.forwardRef>( ({children, className, sx = {}, hidden = false, variant = 'medium'}, forwardedRef) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const titleAreaRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const currentVariant = useResponsiveValue(variant, 'medium') + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) 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, - )} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.TitleArea, + gridArea: 'title-area', + display: 'flex', + gap: '0.5rem', + ...displayBreakpoints, + flexDirection: 'row', + alignItems: 'flex-start', + }, + sx, + ) + } + style={enabled ? displayBreakpoints : undefined} > {children} @@ -357,28 +398,41 @@ const LeadingAction: React.FC> = ({ sx = {}, hidden = hiddenOnNarrow, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height 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, - )} - style={style} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.LeadingAction, + gridArea: 'leading-action', + paddingRight: '0.5rem', + display: 'flex', + ...displayBreakpoints, + alignItems: 'center', + }, + sx, + ) + } + style={ + enabled + ? { + ...displayBreakpoints, + ...style, + } + : style + } > {children} @@ -392,26 +446,33 @@ const Breadcrumbs: React.FC> = ({ sx = {}, hidden = false, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) 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, - )} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.Breadcrumbs, + gridArea: 'breadcrumbs', + paddingRight: '0.5rem', + display: 'flex', + ...displayBreakpoints, + alignItems: 'center', + fontWeight: 'initial', + lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', + fontSize: 'var(--text-body-size-medium, 0.875rem)', + }, + sx, + ) + } + style={enabled ? displayBreakpoints : undefined} > {children} @@ -425,27 +486,40 @@ const LeadingVisual: React.FC> = ({ sx = {}, hidden = false, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height 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, - )} - style={style} + sx={ + enabled + ? sx + : merge( + { + // using flex and order to display the leading visual in the title area. + display: 'flex', + order: TITLE_AREA_REGION_ORDER.LeadingVisual, + ...displayBreakpoints, + alignItems: 'center', + }, + sx, + ) + } + style={ + enabled + ? { + ...displayBreakpoints, + ...style, + } + : style + } > {children} @@ -463,7 +537,11 @@ const Title: React.FC> = ({ hidden = false, as = 'h2', }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) // @ts-ignore sxProp can have color attribute const {fontSize, lineHeight, fontWeight} = sx if (fontSize) style['--custom-font-size'] = fontSize @@ -472,23 +550,32 @@ const Title: React.FC> = ({ 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, - )} + style={ + enabled + ? { + ...displayBreakpoints, + ...style, + } + : style + } + sx={ + enabled + ? sx + : merge( + { + // using flex and order to display the title in the title area. + display: 'flex', + order: TITLE_AREA_REGION_ORDER.Title, + ...displayBreakpoints, + fontSize: 'inherit', + fontWeight: 'inherit', + }, + sx, + ) + } > {children} @@ -502,27 +589,40 @@ const TrailingVisual: React.FC> = ({ sx = {}, hidden = false, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height 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, - )} - style={style} + sx={ + enabled + ? sx + : merge( + { + // using flex and order to display the trailing visual in the title area. + display: 'flex', + order: TITLE_AREA_REGION_ORDER.TrailingVisual, + ...displayBreakpoints, + alignItems: 'center', + }, + sx, + ) + } + style={ + enabled + ? { + ...displayBreakpoints, + ...style, + } + : style + } > {children} @@ -535,28 +635,41 @@ const TrailingAction: React.FC> = ({ sx = {}, hidden = hiddenOnNarrow, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height 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, - )} - style={style} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.TrailingAction, + gridArea: 'trailing-action', + paddingLeft: '0.5rem', + display: 'flex', + ...displayBreakpoints, + alignItems: 'center', + }, + sx, + ) + } + style={ + enabled + ? { + ...displayBreakpoints, + ...style, + } + : style + } > {children} @@ -569,32 +682,45 @@ const Actions: React.FC> = ({ sx = {}, hidden = false, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height 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, - )} - style={style} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.Actions, + gridArea: 'actions', + display: 'flex', + ...displayBreakpoints, + flexDirection: 'row', + paddingLeft: '0.5rem', + gap: '0.5rem', + minWidth: 'max-content', + justifyContent: 'right', + alignItems: 'center', + }, + sx, + ) + } + style={ + enabled + ? { + ...displayBreakpoints, + ...style, + } + : style + } > {children} @@ -608,27 +734,34 @@ const Description: React.FC> = ({ sx = {}, hidden = false, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }) 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, - )} + className={clsx(enabled && classes.Description, className)} + style={enabled ? displayBreakpoints : undefined} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.Description, + gridArea: 'description', + display: 'flex', + ...displayBreakpoints, + 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, + ) + } > {children} @@ -646,38 +779,44 @@ const Navigation: React.FC> = ({ children, className, sx = {}, - hidden = false, + hidden = hiddenOnRegularAndWide, as, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, }) => { + const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) + const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'block' + }) warning( as === 'nav' && !ariaLabel && !ariaLabelledBy, 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', ) - 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, - )} + className={clsx(enabled && classes.Navigation, className)} + style={enabled ? displayBreakpoints : undefined} + sx={ + enabled + ? sx + : merge( + { + gridRow: GRID_ROW_ORDER.Navigation, + gridArea: 'navigation', + paddingTop: '0.5rem', + display: 'flex', + ...displayBreakpoints, + fontWeight: 'initial', + lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', + fontSize: 'var(--text-body-size-medium, 0.875rem)', + }, + sx, + ) + } > {children} diff --git a/packages/react/src/utils/getBreakpointDeclarations.ts b/packages/react/src/utils/getBreakpointDeclarations.ts index 2f75f738ead..2b57536267f 100644 --- a/packages/react/src/utils/getBreakpointDeclarations.ts +++ b/packages/react/src/utils/getBreakpointDeclarations.ts @@ -1,6 +1,5 @@ import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {isResponsiveValue} from '../hooks/useResponsiveValue' -import type {BetterSystemStyleObject} from '../sx' import type {Properties as CSSProperties} from 'csstype' import {mediaQueries} from './layout' @@ -106,7 +105,7 @@ export function getBreakpointDeclarations( value: TInput | ResponsiveValue, cssProperty: keyof CSSProperties, mapFn: (value: TInput) => TOutput, -): BetterSystemStyleObject { +) { if (isResponsiveValue(value)) { // eslint-disable-next-line @typescript-eslint/no-explicit-any const responsiveValue = value as Extract> From d272fba4fcb3bf4dc58bd958a904d2043c63963d Mon Sep 17 00:00:00 2001 From: Hussam Ghazzi Date: Mon, 25 Nov 2024 19:00:04 +0000 Subject: [PATCH 2/3] revert default value --- packages/react/src/PageHeader/PageHeader.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 7376682cea6..2d3214055eb 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -779,7 +779,7 @@ const Navigation: React.FC> = ({ children, className, sx = {}, - hidden = hiddenOnRegularAndWide, + hidden = false, as, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, From 35ee6763da5fea0cc810fd815964231d6427bd43 Mon Sep 17 00:00:00 2001 From: Hussam Ghazzi Date: Mon, 25 Nov 2024 20:00:09 +0000 Subject: [PATCH 3/3] use data attributes for hidden --- .../src/PageHeader/PageHeader.module.css | 24 +- packages/react/src/PageHeader/PageHeader.tsx | 247 ++++++++++-------- .../src/utils/getBreakpointDeclarations.ts | 7 +- 3 files changed, 160 insertions(+), 118 deletions(-) diff --git a/packages/react/src/PageHeader/PageHeader.module.css b/packages/react/src/PageHeader/PageHeader.module.css index cdae3110906..3252339ffd2 100644 --- a/packages/react/src/PageHeader/PageHeader.module.css +++ b/packages/react/src/PageHeader/PageHeader.module.css @@ -67,6 +67,28 @@ & [data-component='PH_TrailingVisual'] { height: calc(var(--title-line-height) * 1em); } + + & [data-is-hidden-all] { + display: none; + } + + & [data-is-hidden-narrow] { + @media screen and (max-width: 768px) { + display: none; + } + } + + & [data-is-hidden-regular] { + @media screen and (min-width: 768px) { + display: none; + } + } + + & [data-is-hidden-wide] { + @media screen and (min-width: 1440px) { + display: none; + } + } } .ContextArea { @@ -188,7 +210,7 @@ } .Navigation { - display: flex; + display: block; padding-top: var(--base-size-8); font-size: var(--text-body-size-medium, 0.875rem); font-weight: var(--base-text-weight-light); diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index 2d3214055eb..7c3b7489014 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -1,7 +1,7 @@ import React, {useEffect} from 'react' import Box from '../Box' import type {ResponsiveValue} from '../hooks/useResponsiveValue' -import {useResponsiveValue} from '../hooks/useResponsiveValue' +import {isResponsiveValue, useResponsiveValue} from '../hooks/useResponsiveValue' import type {SxProp, BetterSystemStyleObject, CSSCustomProperties} from '../sx' import {merge} from '../sx' import Heading from '../Heading' @@ -10,7 +10,11 @@ import type {LinkProps as BaseLinkProps} from '../Link' import Link from '../Link' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' -import {getBreakpointDeclarations} from '../utils/getBreakpointDeclarations' +import { + areAllValuesTheSame, + getBreakpointDeclarations, + haveRegularAndWideSameValue, +} from '../utils/getBreakpointDeclarations' import {warning} from '../utils/warning' import {useProvidedRefOrCreate} from '../hooks' import type {AriaRole} from '../utils/types' @@ -193,9 +197,6 @@ const ContextArea: React.FC> = ({ sx = {}, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) const contentNavStyles = { gridRow: GRID_ROW_ORDER.ContextArea, gridArea: 'context-area', @@ -204,7 +205,9 @@ const ContextArea: React.FC> = ({ alignItems: 'center', paddingBottom: '0.5rem', gap: '0.5rem', - ...displayBreakpoints, + ...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)', @@ -214,7 +217,7 @@ const ContextArea: React.FC> = ({ (contentNavStyles, sx)} - style={enabled ? displayBreakpoints : undefined} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -232,9 +235,6 @@ export type ParentLinkProps = React.PropsWithChildren( ({children, className, sx = {}, href, 'aria-label': ariaLabel, as = 'a', hidden = hiddenOnRegularAndWide}, ref) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) return ( <> ( alignItems: 'center', order: CONTEXT_AREA_REGION_ORDER.ParentLink, gap: '0.5rem', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), }, sx, ) } - style={enabled ? displayBreakpoints : undefined} + {...getHiddenDataAttributes(enabled, hidden)} href={href} > @@ -280,9 +282,6 @@ const ContextBar: React.FC> = ({ hidden = hiddenOnRegularAndWide, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) return ( > = ({ { display: 'flex', order: CONTEXT_AREA_REGION_ORDER.ContextBar, - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), }, sx, ) } - style={enabled ? displayBreakpoints : undefined} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -314,12 +315,10 @@ const ContextAreaActions: React.FC> = hidden = hiddenOnRegularAndWide, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) return ( > = gap: '0.5rem', flexGrow: '1', justifyContent: 'right', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), }, sx, ) } - style={enabled ? displayBreakpoints : undefined} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -356,9 +357,6 @@ const TitleArea = React.forwardRef(forwardedRef as React.RefObject) const currentVariant = useResponsiveValue(variant, 'medium') - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) return ( { + return value ? 'none' : 'flex' + }), flexDirection: 'row', alignItems: 'flex-start', }, sx, ) } - style={enabled ? displayBreakpoints : undefined} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -400,9 +400,6 @@ const LeadingAction: React.FC> = ({ }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height @@ -419,20 +416,16 @@ const LeadingAction: React.FC> = ({ gridArea: 'leading-action', paddingRight: '0.5rem', display: 'flex', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), alignItems: 'center', }, sx, ) } - style={ - enabled - ? { - ...displayBreakpoints, - ...style, - } - : style - } + style={style} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -447,9 +440,7 @@ const Breadcrumbs: React.FC> = ({ hidden = false, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) + return ( > = ({ gridArea: 'breadcrumbs', paddingRight: '0.5rem', display: 'flex', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), alignItems: 'center', fontWeight: 'initial', lineHeight: 'var(--text-body-lineHeight-medium, 1.4285)', @@ -472,7 +465,7 @@ const Breadcrumbs: React.FC> = ({ sx, ) } - style={enabled ? displayBreakpoints : undefined} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -488,9 +481,6 @@ const LeadingVisual: React.FC> = ({ }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height @@ -506,20 +496,16 @@ const LeadingVisual: React.FC> = ({ // using flex and order to display the leading visual in the title area. display: 'flex', order: TITLE_AREA_REGION_ORDER.LeadingVisual, - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), alignItems: 'center', }, sx, ) } - style={ - enabled - ? { - ...displayBreakpoints, - ...style, - } - : style - } + style={style} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -539,9 +525,6 @@ const Title: React.FC> = ({ }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) // @ts-ignore sxProp can have color attribute const {fontSize, lineHeight, fontWeight} = sx if (fontSize) style['--custom-font-size'] = fontSize @@ -553,14 +536,7 @@ const Title: React.FC> = ({ className={clsx(enabled && classes.Title, className)} data-component="PH_Title" as={as} - style={ - enabled - ? { - ...displayBreakpoints, - ...style, - } - : style - } + style={style} sx={ enabled ? sx @@ -569,13 +545,16 @@ const Title: React.FC> = ({ // using flex and order to display the title in the title area. display: 'flex', order: TITLE_AREA_REGION_ORDER.Title, - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), fontSize: 'inherit', fontWeight: 'inherit', }, sx, ) } + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -591,9 +570,6 @@ const TrailingVisual: React.FC> = ({ }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height @@ -609,20 +585,16 @@ const TrailingVisual: React.FC> = ({ // using flex and order to display the trailing visual in the title area. display: 'flex', order: TITLE_AREA_REGION_ORDER.TrailingVisual, - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), alignItems: 'center', }, sx, ) } - style={ - enabled - ? { - ...displayBreakpoints, - ...style, - } - : style - } + style={style} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -636,9 +608,6 @@ const TrailingAction: React.FC> = ({ hidden = hiddenOnNarrow, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) const style: CSSCustomProperties = {} // @ts-ignore sx has height attribute const {height} = sx @@ -656,20 +625,16 @@ const TrailingAction: React.FC> = ({ gridArea: 'trailing-action', paddingLeft: '0.5rem', display: 'flex', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), alignItems: 'center', }, sx, ) } - style={ - enabled - ? { - ...displayBreakpoints, - ...style, - } - : style - } + style={style} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -684,9 +649,6 @@ const Actions: React.FC> = ({ }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) const style: CSSCustomProperties = {} - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) // @ts-ignore sx has height attribute const {height} = sx if (height) style['--custom-height'] = height @@ -702,7 +664,9 @@ const Actions: React.FC> = ({ gridRow: GRID_ROW_ORDER.Actions, gridArea: 'actions', display: 'flex', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), flexDirection: 'row', paddingLeft: '0.5rem', gap: '0.5rem', @@ -713,14 +677,8 @@ const Actions: React.FC> = ({ sx, ) } - style={ - enabled - ? { - ...displayBreakpoints, - ...style, - } - : style - } + style={style} + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -735,13 +693,9 @@ const Description: React.FC> = ({ hidden = false, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'flex' - }) return ( > = ({ gridRow: GRID_ROW_ORDER.Description, gridArea: 'description', display: 'flex', - ...displayBreakpoints, + ...getBreakpointDeclarations(hidden, 'display', value => { + return value ? 'none' : 'flex' + }), flexDirection: 'row', alignItems: 'center', paddingTop: '0.5rem', @@ -762,6 +718,7 @@ const Description: React.FC> = ({ sx, ) } + {...getHiddenDataAttributes(enabled, hidden)} > {children} @@ -785,9 +742,6 @@ const Navigation: React.FC> = ({ 'aria-labelledby': ariaLabelledBy, }) => { const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - const displayBreakpoints = getBreakpointDeclarations(hidden, 'display', value => { - return value ? 'none' : 'block' - }) warning( as === 'nav' && !ariaLabel && !ariaLabelledBy, 'Use `aria-label` or `aria-labelledby` prop to provide an accessible label to the `nav` landmark for assistive technology', @@ -799,7 +753,6 @@ const Navigation: React.FC> = ({ aria-label={as === 'nav' ? ariaLabel : undefined} aria-labelledby={as === 'nav' ? ariaLabelledBy : undefined} className={clsx(enabled && classes.Navigation, className)} - style={enabled ? displayBreakpoints : undefined} sx={ enabled ? sx @@ -809,7 +762,9 @@ const Navigation: React.FC> = ({ gridArea: 'navigation', paddingTop: '0.5rem', display: 'flex', - ...displayBreakpoints, + ...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)', @@ -817,12 +772,76 @@ const Navigation: React.FC> = ({ sx, ) } + {...getHiddenDataAttributes(enabled, 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, +): { + 'data-is-hidden-all'?: boolean + 'data-is-hidden-narrow'?: boolean + 'data-is-hidden-regular'?: boolean + 'data-is-hidden-wide'?: boolean +} { + if (!isCssModules) { + return {} + } + + if (isResponsiveValue(isHidden)) { + const responsiveValue = isHidden + + // Build media queries with the giving cssProperty and mapFn + const narrowMediaQuery = + 'narrow' in responsiveValue + ? { + 'data-is-hidden-narrow': responsiveValue.narrow || undefined, + } + : {} + + const regularMediaQuery = + 'regular' in responsiveValue + ? { + 'data-is-hidden-regular': responsiveValue.regular || undefined, + } + : {} + + const wideMediaQuery = + 'wide' in responsiveValue + ? { + 'data-is-hidden-wide': responsiveValue.wide || undefined, + } + : {} + + // check if all values are the same - this is not a recommended practice but we still should check for it + if (areAllValuesTheSame(responsiveValue)) { + // if all the values are the same, we can just use one of the value to determine the CSS property's value + return {'data-is-hidden-all': responsiveValue.narrow || undefined} + // check if regular and wide have the same value, if so we can just return the narrow and regular media queries + } else if (haveRegularAndWideSameValue(responsiveValue)) { + return { + ...narrowMediaQuery, + ...regularMediaQuery, + } + } else { + return { + ...narrowMediaQuery, + ...regularMediaQuery, + ...wideMediaQuery, + } + } + } else { + // If the given value is not a responsive value + return {'data-is-hidden-all': isHidden || undefined} + } +} + export const PageHeader = Object.assign(Root, { ContextArea, ParentLink, diff --git a/packages/react/src/utils/getBreakpointDeclarations.ts b/packages/react/src/utils/getBreakpointDeclarations.ts index 2b57536267f..23e603cea02 100644 --- a/packages/react/src/utils/getBreakpointDeclarations.ts +++ b/packages/react/src/utils/getBreakpointDeclarations.ts @@ -1,16 +1,17 @@ import type {ResponsiveValue} from '../hooks/useResponsiveValue' import {isResponsiveValue} from '../hooks/useResponsiveValue' +import type {BetterSystemStyleObject} from '../sx' import type {Properties as CSSProperties} from 'csstype' import {mediaQueries} from './layout' -function areAllValuesTheSame(responsiveValue: ResponsiveValue): boolean { +export function areAllValuesTheSame(responsiveValue: ResponsiveValue): boolean { if ('narrow' in responsiveValue && 'regular' in responsiveValue && 'wide' in responsiveValue) { const values = Object.values(responsiveValue) return values.every(value => value === values[0]) } return false } -function haveRegularAndWideSameValue(responsiveValue: ResponsiveValue): boolean { +export function haveRegularAndWideSameValue(responsiveValue: ResponsiveValue): boolean { if ('regular' in responsiveValue && 'wide' in responsiveValue) { return responsiveValue.regular === responsiveValue.wide } @@ -105,7 +106,7 @@ export function getBreakpointDeclarations( value: TInput | ResponsiveValue, cssProperty: keyof CSSProperties, mapFn: (value: TInput) => TOutput, -) { +): BetterSystemStyleObject { if (isResponsiveValue(value)) { // eslint-disable-next-line @typescript-eslint/no-explicit-any const responsiveValue = value as Extract>