Merge 350ab45 into d2ab5e2

primer · Jun 10, 2024 · a0574c6 · a0574c6
2 parents d2ab5e2 + 350ab45
commit a0574c6
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 104 deletions.
diff --git a/packages/react/src/PageHeader/PageHeader.dev.stories.tsx b/packages/react/src/PageHeader/PageHeader.dev.stories.tsx
@@ -18,11 +18,11 @@ export default meta
 
 export const LargeVariantWithMultilineTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
+    <PageHeader titleVariant="large">
       <PageHeader.LeadingAction>
         <IconButton aria-label="Edit" icon={PencilIcon} variant="invisible" />
       </PageHeader.LeadingAction>
-      <PageHeader.TitleArea variant="large">
+      <PageHeader.TitleArea>
         <PageHeader.LeadingVisual>
           <GitBranchIcon />
         </PageHeader.LeadingVisual>

diff --git a/packages/react/src/PageHeader/PageHeader.docs.json b/packages/react/src/PageHeader/PageHeader.docs.json
@@ -18,6 +18,12 @@
       "defaultValue": "false",
       "description": "Whether the content is hidden."
     },
+    {
+      "name": "titleVariant",
+      "type": "| 'subtitle' | 'medium' | 'large' | { narrow?: | 'subtitle' | 'medium' | 'large' regular?: | 'subtitle' | 'medium' | 'large' wide?: | 'subtitle' | 'medium' | 'large' }",
+      "defaultValue": "medium",
+      "description": "Handles the title size and styles as well as the other elements' height. 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"
@@ -104,12 +110,6 @@
           "defaultValue": "false",
           "description": "Whether the content is hidden."
         },
-        {
-          "name": "variant",
-          "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"

diff --git a/packages/react/src/PageHeader/PageHeader.features.stories.tsx b/packages/react/src/PageHeader/PageHeader.features.stories.tsx
@@ -41,8 +41,8 @@ export const HasTitleOnly = () => (
 
 export const HasLargeTitle = () => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
-      <PageHeader.TitleArea variant="large">
+    <PageHeader titleVariant="large">
+      <PageHeader.TitleArea>
         <PageHeader.Title>Title</PageHeader.Title>
       </PageHeader.TitleArea>
     </PageHeader>

diff --git a/packages/react/src/PageHeader/PageHeader.stories.tsx b/packages/react/src/PageHeader/PageHeader.stories.tsx
@@ -189,14 +189,14 @@ export default meta
 
 export const Playground: Story = args => (
   <Box sx={{padding: 3}}>
-    <PageHeader>
-      <PageHeader.TitleArea
-        variant={{
-          narrow: args['Title.variant'],
-          regular: args['Title.variant'],
-          wide: args['Title.variant'],
-        }}
-      >
+    <PageHeader
+      titleVariant={{
+        narrow: args['Title.variant'],
+        regular: args['Title.variant'],
+        wide: args['Title.variant'],
+      }}
+    >
+      <PageHeader.TitleArea>
         <PageHeader.LeadingVisual hidden={!args.hasLeadingVisual}>{<args.LeadingVisual />}</PageHeader.LeadingVisual>
         <PageHeader.Title as={args['Title.as']} hidden={!args.hasTitle}>
           {args.Title}

diff --git a/packages/react/src/PageHeader/PageHeader.test.tsx b/packages/react/src/PageHeader/PageHeader.test.tsx
@@ -138,8 +138,8 @@ describe('PageHeader', () => {
   })
   it('respects the title variant prop', () => {
     const {getByText} = render(
-      <PageHeader>
-        <PageHeader.TitleArea variant="large">
+      <PageHeader titleVariant="large">
+        <PageHeader.TitleArea>
           <PageHeader.Title>Title</PageHeader.Title>
         </PageHeader.TitleArea>
         <PageHeader.ContextArea>ContextArea</PageHeader.ContextArea>

diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx
@@ -61,13 +61,18 @@ const hiddenOnNarrow = {
 
 // Root
 // -----------------------------------------------------------------------------
-export type PageHeaderProps = {
-  'aria-label'?: React.AriaAttributes['aria-label']
-  as?: React.ElementType | 'header' | 'div'
-} & SxProp
-
-const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeaderProps>>(
-  ({children, sx = {}, as = 'div'}, forwardedRef) => {
+export type PageHeaderProps = React.PropsWithChildren<
+  {
+    'aria-label'?: React.AriaAttributes['aria-label']
+    as?: React.ElementType | 'header' | 'div'
+    titleVariant?: 'subtitle' | 'medium' | 'large' | ResponsiveValue<'subtitle' | 'medium' | 'large'>
+  } & SxProp
+>
+
+const Root = React.forwardRef(
+  ({children, sx = {}, as = 'div', titleVariant = 'medium', ...rest}: PageHeaderProps, forwardedRef) => {
+    // const [fontSize, setFontSize] = useState<string | null | number | string[]>(null)
+    const currentVariant = useResponsiveValue(titleVariant, 'medium')
     const rootStyles = {
       display: 'grid',
       // We have max 5 columns.
@@ -85,18 +90,48 @@ const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeader
           {
             height: `var(--custom-height, ${LARGE_TITLE_HEIGHT})`,
           },
+        '[data-component="PH_Title"]': {
+          fontSize: [
+            'var(--custom-font-size-0, var(--text-title-size-large, 2rem))',
+            'var(--custom-font-size-1, var(--text-title-size-large, 2rem))',
+            'var(--custom-font-size-2, var(--text-title-size-large, 2rem))',
+            'var(--custom-font-size-3, 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))',
+        },
       },
       '&[data-size-variant="medium"]': {
         '[data-component="PH_LeadingAction"], [data-component="PH_TrailingAction"],[data-component="PH_Actions"], [data-component="PH_LeadingVisual"], [data-component="PH_TrailingVisual"]':
           {
             height: `var(--custom-height, ${MEDIUM_TITLE_HEIGHT})`,
           },
+        '[data-component="PH_Title"]': {
+          fontSize: [
+            'var(--custom-font-size-0, var(--text-title-size-medium, 1.25rem))',
+            'var(--custom-font-size-1, var(--text-title-size-medium, 1.25rem))',
+            'var(--custom-font-size-2, var(--text-title-size-medium, 1.25rem))',
+            'var(--custom-font-size-3, 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))',
+        },
       },
       '&[data-size-variant="subtitle"]': {
         '[data-component="PH_LeadingAction"], [data-component="PH_TrailingAction"],[data-component="PH_Actions"], [data-component="PH_LeadingVisual"], [data-component="PH_TrailingVisual"]':
           {
             height: `var(--custom-height, ${MEDIUM_TITLE_HEIGHT})`,
           },
+        '[data-component="PH_Title"]': {
+          fontSize: [
+            'var(--custom-font-size-0, var(--text-title-size-medium, 1.25rem))',
+            'var(--custom-font-size-1, var(--text-title-size-medium, 1.25rem))',
+            'var(--custom-font-size-2, var(--text-title-size-medium, 1.25rem))',
+            'var(--custom-font-size-3, 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))',
+        },
       },
     }
 
@@ -113,7 +148,6 @@ const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeader
 
     const [hasContextArea, setHasContextArea] = useState(false)
     const [hasLeadingAction, setHasLeadingAction] = useState(false)
-    const [titleVariant, setTitleVariant] = useState<string | undefined>('')
 
     useEffect(() => {
       if (!rootRef.current || rootRef.current.children.length <= 0) return
@@ -124,10 +158,6 @@ const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeader
       // It is very unlikely to have a PageHeader without a TitleArea, but we still want to make sure we don't break the page if that happens.
       if (!titleArea) return
 
-      // // grab the data-size-variant attribute from the titleArea
-      const sizeVariant = titleArea.getAttribute('data-size-variant')
-      setTitleVariant(sizeVariant as string)
-
       for (const child of React.Children.toArray(children)) {
         if (React.isValidElement(child) && child.type === ContextArea) {
           setHasContextArea(true)
@@ -156,7 +186,13 @@ const Root = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageHeader
       }
     }, [children, rootRef, hasContextArea, hasLeadingAction])
     return (
-      <Box ref={rootRef} as={as} sx={merge<BetterSystemStyleObject>(rootStyles, sx)} data-size-variant={titleVariant}>
+      <Box
+        ref={rootRef}
+        as={as}
+        sx={merge<BetterSystemStyleObject>(rootStyles, sx)}
+        data-size-variant={currentVariant}
+        {...rest}
+      >
         {children}
       </Box>
     )
@@ -284,80 +320,33 @@ const ContextAreaActions: React.FC<React.PropsWithChildren<ChildrenPropTypes>> =
     </Box>
   )
 }
-
-type TitleAreaProps = {
-  variant?: 'subtitle' | 'medium' | 'large' | ResponsiveValue<'subtitle' | 'medium' | 'large'>
-} & ChildrenPropTypes
 // PageHeader.TitleArea: The main title area of the page. Visible on all viewports.
 // PageHeader.TitleArea Sub Components: PageHeader.LeadingVisual, PageHeader.Title, PageTitle.TrailingVisual
 // ---------------------------------------------------------------------
 
-const TitleArea = React.forwardRef<HTMLDivElement, React.PropsWithChildren<TitleAreaProps>>(
-  ({children, sx = {}, hidden = false, variant = 'medium'}, forwardedRef) => {
-    const titleAreaRef = useProvidedRefOrCreate<HTMLDivElement>(forwardedRef as React.RefObject<HTMLDivElement>)
-    const currentVariant = useResponsiveValue(variant, 'medium')
-    const [fontSize, setFontSize] = useState<string | null | number | string[]>(null)
-
-    useEffect(() => {
-      if (!titleAreaRef.current || titleAreaRef.current.children.length <= 0) return
-      const title = Array.from(titleAreaRef.current.children as HTMLCollection).find(child => {
-        return child instanceof HTMLElement && child.getAttribute('data-component') === 'PH_Title'
-      })
-
-      const styles = getComputedStyle(title as HTMLHeadingElement)
-      const customfontSize = styles.getPropertyValue('--custom-font-size')
-      // This is cumbersome but needed to handle the array format of font-size
-      if (customfontSize.includes(',')) {
-        const values = customfontSize.split(',')
-        setFontSize(values)
-      } else {
-        setFontSize(customfontSize)
-      }
-      // We only need this on the pageload
-    }, [titleAreaRef])
-    return (
-      <Box
-        ref={titleAreaRef}
-        data-component="TitleArea"
-        data-size-variant={currentVariant}
-        sx={merge<BetterSystemStyleObject>(
-          {
-            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',
-            // 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 overriden but still want to allow consumers to override them if needed.
-            '&[data-size-variant="large"] [data-component="PH_Title"]': {
-              fontSize: fontSize ? fontSize : '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))',
-            },
-            '&[data-size-variant="medium"] [data-component="PH_Title"]': {
-              fontSize: fontSize ? fontSize : '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))',
-            },
-            '&[data-size-variant="subtitle"] [data-component="PH_Title"]': {
-              fontSize: fontSize ? fontSize : '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))',
-            },
-          },
-          sx,
-        )}
-      >
-        {children}
-      </Box>
-    )
-  },
-) as PolymorphicForwardRefComponent<'div', TitleAreaProps>
+const TitleArea: React.FC<React.PropsWithChildren<ChildrenPropTypes>> = ({children, sx = {}, hidden = false}) => {
+  return (
+    <Box
+      data-component="TitleArea"
+      sx={merge<BetterSystemStyleObject>(
+        {
+          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,
+      )}
+    >
+      {children}
+    </Box>
+  )
+}
 
 // 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.
@@ -453,7 +442,20 @@ const Title: React.FC<React.PropsWithChildren<TitleProps>> = ({children, sx = {}
   const style: CSSCustomProperties = {}
   // @ts-ignore sxProp can have color attribute
   const {fontSize, lineHeight, fontWeight} = sx
-  if (fontSize) style['--custom-font-size'] = fontSize
+  if (fontSize) {
+    if (Array.isArray(fontSize)) {
+      // This is to handle styled components responsive break points. This is temporary fix.
+      style['--custom-font-size-0'] = fontSize[0]
+      style['--custom-font-size-1'] = fontSize[1]
+      style['--custom-font-size-2'] = fontSize[2]
+      style['--custom-font-size-3'] = fontSize[3]
+    } else {
+      style['--custom-font-size-0'] = fontSize
+      style['--custom-font-size-1'] = fontSize
+      style['--custom-font-size-2'] = fontSize
+      style['--custom-font-size-3'] = fontSize
+    }
+  }
   if (lineHeight) style['--custom-line-height'] = lineHeight
   if (fontWeight) style['--custom-font-weight'] = fontWeight