From 8e481cadd1fc45119f28428f97b75817179473d5 Mon Sep 17 00:00:00 2001 From: Stephanie Hong <41085564+JelloBagel@users.noreply.github.com> Date: Tue, 17 Dec 2024 10:22:41 -0800 Subject: [PATCH] feat(BaseStyles): Fix Typography and Common props when CSS modules feature 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 --- .changeset/polite-books-sneeze.md | 5 + packages/react/src/BaseStyles.dev.stories.tsx | 3 +- packages/react/src/BaseStyles.tsx | 113 ++++++++++++++---- packages/react/src/Text/Text.tsx | 16 +-- .../react/src/__tests__/BaseStyles.test.tsx | 20 +++- .../AnchoredOverlay.test.tsx.snap | 4 +- .../__snapshots__/BaseStyles.test.tsx.snap | 8 +- .../react/src/utils/includeSystemProps.ts | 31 +++++ 8 files changed, 152 insertions(+), 48 deletions(-) create mode 100644 .changeset/polite-books-sneeze.md create mode 100644 packages/react/src/utils/includeSystemProps.ts diff --git a/.changeset/polite-books-sneeze.md b/.changeset/polite-books-sneeze.md new file mode 100644 index 00000000000..e56d828b817 --- /dev/null +++ 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 index d313f089009..c778d9f0259 100644 --- a/packages/react/src/BaseStyles.dev.stories.tsx +++ b/packages/react/src/BaseStyles.dev.stories.tsx @@ -1,6 +1,5 @@ import {BaseStyles} from '.' import type {Meta} from '@storybook/react' -import React from 'react' import type {ComponentProps} from './utils/types' export default { @@ -8,4 +7,4 @@ export default { component: BaseStyles, } as Meta> -export const Default = () => Hello +export const Default = () => 'Hello' diff --git a/packages/react/src/BaseStyles.tsx b/packages/react/src/BaseStyles.tsx index 2a5ffb24347..f44cc58564c 100644 --- 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` - ${TYPOGRAPHY}; - ${COMMON}; - `, -) +const StyledDiv = styled.div` + ${TYPOGRAPHY}; + ${COMMON}; +` -export type BaseStylesProps = ComponentProps +export type BaseStylesProps = PropsWithChildren & { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + as?: React.ComponentType | 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 + + {children} + + ) + } + + return ( + + {children} + + ) + } - /** - * 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 ( - - {!enabled && } + {children} - + ) } diff --git a/packages/react/src/Text/Text.tsx b/packages/react/src/Text/Text.tsx index 9bad8d5d923..a6cd88e276f 100644 --- 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` ${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 index 8895f46d3d8..c6d1270b348 100644 --- 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() + const {container} = render(Hello) expect(container).toMatchSnapshot() }) @@ -27,10 +27,24 @@ describe('BaseStyles', () => { lineHeight: '3.5', } - const {container} = render() + const {container} = render(Hello) expect(container.children[0]).toHaveStyle({color: '#f00', 'font-family': 'Arial', 'line-height': '3.5'}) }) + it('respects system props', () => { + const {container} = render( + + Hello + , + ) + + 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'}, @@ -38,7 +52,7 @@ describe('BaseStyles', () => { sx: {}, } - const {container} = render() + const {container} = render(Hello) 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 index b09b34fe96f..1c8391c7328 100644 --- 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`] = `
+ > + Hello +
`; diff --git a/packages/react/src/utils/includeSystemProps.ts b/packages/react/src/utils/includeSystemProps.ts new file mode 100644 index 00000000000..c958e79144e --- /dev/null +++ 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}