Move variant prop of Title to the parent PageHeader component to prev…

…ent layout shift
primer · Jun 10, 2024 · ed80257 · ed80257
1 parent 43dea59
commit ed80257
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 49 deletions.
diff --git a/packages/react/src/PageHeader/PageHeader.dev.stories.tsx b/packages/react/src/PageHeader/PageHeader.dev.stories.tsx
@@ -26,19 +26,11 @@ export const LargeVariantWithMultilineTitle = () => (
         <PageHeader.LeadingVisual>
           <GitBranchIcon />
         </PageHeader.LeadingVisual>
-        <PageHeader.Title
-          sx={{
-            // lineHeight: '1.25',
-            fontWeight: 'normal',
-            // fontSize: '32px',
-            fontSize: ['26px', '26px', '32px', '32px'],
-            // fontSize: ['26px', '26px', 'var(--text-title-size-large, 32px)', 'var(--text-title-size-large, 32px)'], // it doesn't support this format right now.
-          }}
-        >
+        <PageHeader.Title>
           Title long title some extra loooong looong words here some extra loooong looong words here some extra loooong
           looong words here some extra loooong looong words here some extra loooong looong words here
         </PageHeader.Title>
-        <PageHeader.TrailingVisual sx={{height: '120px'}}>
+        <PageHeader.TrailingVisual>
           <Label>Beta</Label>
         </PageHeader.TrailingVisual>
       </PageHeader.TitleArea>

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.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
@@ -71,6 +71,7 @@ export type PageHeaderProps = React.PropsWithChildren<
 
 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',
@@ -90,16 +91,7 @@ const Root = React.forwardRef(
             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))',
-          ],
-          fontSize: 'var(--custom-font-size, var(--text-title-size-medium, 1.25rem))',
-          // 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))',
-          // ],
+          fontSize: fontSize ? 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))',
         },
@@ -110,8 +102,7 @@ const Root = React.forwardRef(
             height: `var(--custom-height, ${MEDIUM_TITLE_HEIGHT})`,
           },
         '[data-component="PH_Title"]': {
-          // fontSize: 'var(--text-title-size-medium, 1.25rem)',
-          fontSize: 'var(--custom-font-size, var(--text-title-size-medium, 1.25rem))',
+          fontSize: fontSize ? 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))',
         },
@@ -122,8 +113,7 @@ const Root = React.forwardRef(
             height: `var(--custom-height, ${MEDIUM_TITLE_HEIGHT})`,
           },
         '[data-component="PH_Title"]': {
-          // fontSize: 'var(--text-title-size-medium, 1.25rem)',
-          fontSize: 'var(--custom-font-size, var(--text-title-size-medium, 1.25rem))',
+          fontSize: fontSize ? 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))',
         },
@@ -144,7 +134,6 @@ const Root = React.forwardRef(
     const [hasContextArea, setHasContextArea] = useState(false)
     const [hasLeadingAction, setHasLeadingAction] = useState(false)
 
-    // This useeffect doesn't have any impact on the layout, it is purely for dev env logs.
     useEffect(() => {
       if (!rootRef.current || rootRef.current.children.length <= 0) return
       const titleArea = Array.from(rootRef.current.children as HTMLCollection).find(child => {
@@ -154,6 +143,22 @@ const Root = React.forwardRef(
       // 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
 
+      // find the title element
+      const title = Array.from(titleArea.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)
+      }
+
       for (const child of React.Children.toArray(children)) {
         if (React.isValidElement(child) && child.type === ContextArea) {
           setHasContextArea(true)
@@ -326,11 +331,9 @@ type TitleAreaProps = {
 
 const TitleArea = React.forwardRef<HTMLDivElement, React.PropsWithChildren<TitleAreaProps>>(
   ({children, sx = {}, hidden = false}, forwardedRef) => {
-    const titleAreaRef = useProvidedRefOrCreate<HTMLDivElement>(forwardedRef as React.RefObject<HTMLDivElement>)
-
     return (
       <Box
-        ref={titleAreaRef}
+        ref={forwardedRef}
         data-component="TitleArea"
         sx={merge<BetterSystemStyleObject>(
           {
@@ -447,17 +450,7 @@ const Title: React.FC<React.PropsWithChildren<TitleProps>> = ({children, sx = {}
   const style: CSSCustomProperties = {}
   // @ts-ignore sxProp can have color attribute
   const {fontSize, lineHeight, fontWeight} = sx
-  console.log('fontsize:', fontSize)
-  if (fontSize) {
-    if (Array.isArray(fontSize)) {
-      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'] = fontSize
-    }
-  }
+  if (fontSize) style['--custom-font-size'] = fontSize
   if (lineHeight) style['--custom-line-height'] = lineHeight
   if (fontWeight) style['--custom-font-weight'] = fontWeight