feat(BaseStyles): Fix Typography and Common props when CSS modules fe…

…ature flag is enabled (#5444) * Fix Typography and Common props for BaseStyles behind feature flag * Update Text to use util includesSystemProps * Fix whiteSpace prop for BaseStyles * Add changeset * Fix color ts error * Updated color variable in AnchoredOverlay
primer · Dec 17, 2024 · 8e481ca · 8e481ca
1 parent f9e193b
commit 8e481ca
Show file tree

Hide file tree

Showing 8 changed files with 152 additions and 48 deletions.
diff --git a/.changeset/polite-books-sneeze.md b/.changeset/polite-books-sneeze.md
@@ -0,0 +1,5 @@
+---
+"@primer/react": patch
+---
+
+Fix Typography and Common props for BaseStyles when CSS modules feature flag is enabled
diff --git a/packages/react/src/BaseStyles.dev.stories.tsx b/packages/react/src/BaseStyles.dev.stories.tsx
@@ -1,11 +1,10 @@
 import {BaseStyles} from '.'
 import type {Meta} from '@storybook/react'
-import React from 'react'
 import type {ComponentProps} from './utils/types'
 
 export default {
   title: 'Behaviors/BaseStyles/Dev',
   component: BaseStyles,
 } as Meta<ComponentProps<typeof BaseStyles>>
 
-export const Default = () => <BaseStyles>Hello</BaseStyles>
+export const Default = () => 'Hello'
diff --git a/packages/react/src/BaseStyles.tsx b/packages/react/src/BaseStyles.tsx
@@ -1,12 +1,14 @@
-import React from 'react'
+import React, {type CSSProperties, type PropsWithChildren} from 'react'
 import {clsx} from 'clsx'
 import styled, {createGlobalStyle} from 'styled-components'
-import type {ComponentProps} from './utils/types'
 import type {SystemCommonProps, SystemTypographyProps} from './constants'
 import {COMMON, TYPOGRAPHY} from './constants'
 import {useTheme} from './ThemeProvider'
 import {useFeatureFlag} from './FeatureFlags'
-import {toggleStyledComponent} from './internal/utils/toggleStyledComponent'
+import Box from './Box'
+import type {SxProp} from './sx'
+import {includesSystemProps, getTypographyAndCommonProps} from './utils/includeSystemProps'
+
 import classes from './BaseStyles.module.css'
 
 // load polyfill for :focus-visible
@@ -35,45 +37,108 @@ const GlobalStyle = createGlobalStyle<{colorScheme?: 'light' | 'dark'}>`
   }
 `
 
-const Base = toggleStyledComponent(
-  CSS_MODULES_FEATURE_FLAG,
-  'div',
-  styled.div<SystemTypographyProps & SystemCommonProps>`
-    ${TYPOGRAPHY};
-    ${COMMON};
-  `,
-)
+const StyledDiv = styled.div<SystemTypographyProps & SystemCommonProps>`
+  ${TYPOGRAPHY};
+  ${COMMON};
+`
 
-export type BaseStylesProps = ComponentProps<typeof Base>
+export type BaseStylesProps = PropsWithChildren & {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  as?: React.ComponentType<any> | keyof JSX.IntrinsicElements
+  className?: string
+  style?: CSSProperties
+  color?: string // Fixes `color` ts-error
+} & SystemTypographyProps &
+  SystemCommonProps &
+  SxProp
 
 function BaseStyles(props: BaseStylesProps) {
-  const {children, color = 'fg.default', fontFamily = 'normal', lineHeight = 'default', className, ...rest} = props
+  const {
+    children,
+    color = 'var(--fgColor-default)',
+    fontFamily = 'normal',
+    lineHeight = 'default',
+    className,
+    as: Component = 'div',
+    ...rest
+  } = props
 
   const {colorScheme, dayScheme, nightScheme} = useTheme()
   const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
 
-  const stylingProps = enabled ? {className: clsx(classes.BaseStyles, className)} : {className}
+  if (enabled) {
+    const newClassName = clsx(classes.BaseStyles, className)
+
+    // If props includes TYPOGRAPHY or COMMON props, pass them to the Box component
+    if (includesSystemProps(props)) {
+      const systemProps = getTypographyAndCommonProps(props)
+      return (
+        // @ts-ignore shh
+        <Box
+          as={Component}
+          className={newClassName}
+          color={color}
+          fontFamily={fontFamily}
+          lineHeight={lineHeight}
+          data-portal-root
+          /**
+           * We need to map valid primer/react color modes onto valid color modes for primer/primitives
+           * valid color modes for primer/primitives: auto | light | dark
+           * valid color modes for primer/primer: auto | day | night | light | dark
+           */
+          data-color-mode={colorScheme?.includes('dark') ? 'dark' : 'light'}
+          data-light-theme={dayScheme}
+          data-dark-theme={nightScheme}
+          style={systemProps}
+          {...rest}
+        >
+          {children}
+        </Box>
+      )
+    }
+
+    return (
+      <Component
+        className={newClassName}
+        color={color}
+        fontFamily={fontFamily}
+        lineHeight={lineHeight}
+        data-portal-root
+        /**
+         * We need to map valid primer/react color modes onto valid color modes for primer/primitives
+         * valid color modes for primer/primitives: auto | light | dark
+         * valid color modes for primer/primer: auto | day | night | light | dark
+         */
+        data-color-mode={colorScheme?.includes('dark') ? 'dark' : 'light'}
+        data-light-theme={dayScheme}
+        data-dark-theme={nightScheme}
+        {...rest}
+      >
+        {children}
+      </Component>
+    )
+  }
 
-  /**
-   * We need to map valid primer/react color modes onto valid color modes for primer/primitives
-   * valid color modes for primer/primitives: auto | light | dark
-   * valid color modes for primer/primer: auto | day | night | light | dark
-   */
   return (
-    <Base
-      {...rest}
-      {...stylingProps}
+    <StyledDiv
+      className={className}
       color={color}
       fontFamily={fontFamily}
       lineHeight={lineHeight}
       data-portal-root
+      /**
+       * We need to map valid primer/react color modes onto valid color modes for primer/primitives
+       * valid color modes for primer/primitives: auto | light | dark
+       * valid color modes for primer/primer: auto | day | night | light | dark
+       */
       data-color-mode={colorScheme?.includes('dark') ? 'dark' : 'light'}
       data-light-theme={dayScheme}
       data-dark-theme={nightScheme}
+      {...rest}
     >
-      {!enabled && <GlobalStyle colorScheme={colorScheme?.includes('dark') ? 'dark' : 'light'} />}
+      <GlobalStyle colorScheme={colorScheme?.includes('dark') ? 'dark' : 'light'} />
       {children}
-    </Base>
+    </StyledDiv>
   )
 }
 

diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx
@@ -8,8 +8,9 @@ import sx from '../sx'
 import {useFeatureFlag} from '../FeatureFlags'
 import Box from '../Box'
 import {useRefObjectAsForwardedRef} from '../hooks'
-import classes from './Text.module.css'
 import type {ComponentProps} from '../utils/types'
+import {includesSystemProps} from '../utils/includeSystemProps'
+import classes from './Text.module.css'
 
 type StyledTextProps = {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -58,19 +59,6 @@ const StyledText = styled.span<StyledTextProps>`
   ${sx};
 `
 
-const COMMON_PROP_NAMES = new Set(Object.keys(COMMON))
-const TYPOGRAPHY_PROP_NAMES = new Set(Object.keys(TYPOGRAPHY))
-
-const includesSystemProps = (props: StyledTextProps) => {
-  if (props.sx) {
-    return true
-  }
-
-  return Object.keys(props).some(prop => {
-    return TYPOGRAPHY_PROP_NAMES.has(prop) || COMMON_PROP_NAMES.has(prop)
-  })
-}
-
 const Text = forwardRef(({as: Component = 'span', className, size, weight, ...props}, forwardedRef) => {
   const enabled = useFeatureFlag('primer_react_css_modules_ga')
 

diff --git a/packages/react/src/__tests__/BaseStyles.test.tsx b/packages/react/src/__tests__/BaseStyles.test.tsx
@@ -16,7 +16,7 @@ describe('BaseStyles', () => {
   })
 
   it('has default styles', () => {
-    const {container} = render(<BaseStyles></BaseStyles>)
+    const {container} = render(<BaseStyles>Hello</BaseStyles>)
     expect(container).toMatchSnapshot()
   })
 
@@ -27,18 +27,32 @@ describe('BaseStyles', () => {
       lineHeight: '3.5',
     }
 
-    const {container} = render(<BaseStyles {...styles}></BaseStyles>)
+    const {container} = render(<BaseStyles {...styles}>Hello</BaseStyles>)
     expect(container.children[0]).toHaveStyle({color: '#f00', 'font-family': 'Arial', 'line-height': '3.5'})
   })
 
+  it('respects system props', () => {
+    const {container} = render(
+      <BaseStyles display="contents" whiteSpace="pre-wrap" mr="2">
+        Hello
+      </BaseStyles>,
+    )
+
+    expect(container.children[0]).toHaveStyle({
+      display: 'contents',
+      'white-space': 'pre-wrap',
+      'margin-right': '8px',
+    })
+  })
+
   it('accepts className and style props', () => {
     const styles = {
       style: {margin: '10px'},
       className: 'test-classname',
       sx: {},
     }
 
-    const {container} = render(<BaseStyles {...styles}></BaseStyles>)
+    const {container} = render(<BaseStyles {...styles}>Hello</BaseStyles>)
     expect(container.children[0]).toHaveClass('test-classname')
     expect(container.children[0]).toHaveStyle({margin: '10px'})
   })

diff --git a/packages/react/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/AnchoredOverlay.test.tsx.snap
@@ -4,7 +4,7 @@ exports[`AnchoredOverlay should render consistently when open 1`] = `
 .c0 {
   font-family: -apple-system,BlinkMacSystemFont,"Segoe UI","Noto Sans",Helvetica,Arial,sans-serif,"Apple Color Emoji","Segoe UI Emoji";
   line-height: 1.5;
-  color: var(--fgColor-default,var(--color-fg-default,#1F2328));
+  color: var(--fgColor-default);
 }
 
 .c1 {
@@ -38,7 +38,7 @@ exports[`AnchoredOverlay should render consistently when open 1`] = `
 <div>
   <div
     class="c0"
-    color="fg.default"
+    color="var(--fgColor-default)"
     data-color-mode="light"
     data-dark-theme="dark"
     data-light-theme="light"

diff --git a/packages/react/src/__tests__/__snapshots__/BaseStyles.test.tsx.snap b/packages/react/src/__tests__/__snapshots__/BaseStyles.test.tsx.snap
@@ -4,16 +4,18 @@ exports[`BaseStyles has default styles 1`] = `
 .c0 {
   font-family: normal;
   line-height: default;
-  color: fg.default;
+  color: var(--fgColor-default);
 }
 
 <div>
   <div
     class="c0"
-    color="fg.default"
+    color="var(--fgColor-default)"
     data-color-mode="light"
     data-portal-root="true"
     font-family="normal"
-  />
+  >
+    Hello
+  </div>
 </div>
 `;
diff --git a/packages/react/src/utils/includeSystemProps.ts b/packages/react/src/utils/includeSystemProps.ts
@@ -0,0 +1,31 @@
+import {COMMON, TYPOGRAPHY, type SystemCommonProps, type SystemTypographyProps} from '../constants'
+import type {SxProp} from '../sx'
+
+const COMMON_PROP_NAMES = new Set(Object.keys(COMMON))
+const TYPOGRAPHY_PROP_NAMES = new Set(Object.keys(TYPOGRAPHY))
+
+const includesSystemProps = (props: SxProp) => {
+  if (props.sx) {
+    return true
+  }
+
+  return Object.keys(props).some(prop => {
+    return TYPOGRAPHY_PROP_NAMES.has(prop) || COMMON_PROP_NAMES.has(prop)
+  })
+}
+
+type TypographyAndCommonProp = SystemTypographyProps & SystemCommonProps
+
+const getTypographyAndCommonProps = (props: TypographyAndCommonProp): TypographyAndCommonProp => {
+  let typographyAndCommonProps = {} as TypographyAndCommonProp
+  for (const prop of Object.keys(props)) {
+    if (TYPOGRAPHY_PROP_NAMES.has(prop) || COMMON_PROP_NAMES.has(prop)) {
+      const p = prop as keyof TypographyAndCommonProp
+      typographyAndCommonProps = {...typographyAndCommonProps, [p]: props[p]}
+    }
+  }
+
+  return typographyAndCommonProps
+}
+
+export {includesSystemProps, getTypographyAndCommonProps}