Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(BaseStyles): remove unknown props from fallback when feature flag is enabled #5545

Closed
wants to merge 8 commits into from
5 changes: 5 additions & 0 deletions .changeset/angry-ties-push.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

Update BaseStyles to no longer pass system props when feature flag is enabled
28 changes: 14 additions & 14 deletions packages/react/.storybook/preview.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ const preview = {
['*', 'Playground', /Playground$/, 'Features', 'Examples'],
],
],
]
]
],
],
],
'Behaviors',
'Hooks',
Expand All @@ -82,8 +82,8 @@ const preview = {
['*', 'Playground', /Playground$/, 'Features', 'Examples'],
],
],
]
]
],
],
],
[
'Private',
Expand All @@ -100,8 +100,8 @@ const preview = {
['*', 'Playground', /Playground$/, 'Features', 'Examples'],
],
],
]
]
],
],
],
'*',
]
Expand Down Expand Up @@ -257,14 +257,6 @@ export const globalTypes = {
}

export const decorators = [
(Story, context) => {
const {featureFlags} = context.globals
return (
<FeatureFlags flags={featureFlags}>
<Story {...context} />
</FeatureFlags>
)
},
(Story, context) => {
const {colorScheme} = context.globals

Expand Down Expand Up @@ -318,6 +310,14 @@ export const decorators = [
</Profiler>
)
},
(Story, context) => {
const {featureFlags} = context.globals
return (
<FeatureFlags flags={featureFlags}>
<Story {...context} />
</FeatureFlags>
)
},
]

export default preview
6 changes: 6 additions & 0 deletions packages/react/src/BaseStyles.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ details-dialog:focus:not(:focus-visible):not(.focus-visible) {
/* -------------------------------------------------------------------------- */

.BaseStyles {
font-family: var(--BaseStyles-fontFamily, var(--fontStack-system));
/* stylelint-disable-next-line primer/typography */
line-height: var(--BaseStyles-lineHeight, 1.5);
/* stylelint-disable-next-line primer/colors */
color: var(--BaseStyles-fgColor, var(--fgColor-default));

/* Global styles for light mode */
&:has([data-color-mode='light']) {
input & {
Expand Down
39 changes: 19 additions & 20 deletions packages/react/src/BaseStyles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,17 @@ export type BaseStylesProps = PropsWithChildren & {
SxProp

function BaseStyles(props: BaseStylesProps) {
const {
children,
color = 'var(--fgColor-default)',
fontFamily = 'normal',
lineHeight = 'default',
className,
as: Component = 'div',
...rest
} = props

const {children, color, fontFamily, lineHeight, className, as: Component = 'div', style, ...rest} = props
const {colorScheme, dayScheme, nightScheme} = useTheme()
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)

if (enabled) {
const newClassName = clsx(classes.BaseStyles, className)
const baseStyles = {
['--BaseStyles-fgColor']: color,
['--BaseStyles-fontFamily']: fontFamily,
['--BaseStyles-lineHeight']: lineHeight,
}

// If props includes TYPOGRAPHY or COMMON props, pass them to the Box component
if (includesSystemProps(props)) {
Expand All @@ -77,9 +73,6 @@ function BaseStyles(props: BaseStylesProps) {
<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
Expand All @@ -89,7 +82,11 @@ function BaseStyles(props: BaseStylesProps) {
data-color-mode={colorScheme?.includes('dark') ? 'dark' : 'light'}
data-light-theme={dayScheme}
data-dark-theme={nightScheme}
style={systemProps}
style={{
...systemProps,
...baseStyles,
...style,
}}
{...rest}
>
{children}
Expand All @@ -100,9 +97,6 @@ function BaseStyles(props: BaseStylesProps) {
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
Expand All @@ -112,6 +106,10 @@ function BaseStyles(props: BaseStylesProps) {
data-color-mode={colorScheme?.includes('dark') ? 'dark' : 'light'}
data-light-theme={dayScheme}
data-dark-theme={nightScheme}
style={{
...baseStyles,
...style,
}}
{...rest}
>
{children}
Expand All @@ -122,9 +120,9 @@ function BaseStyles(props: BaseStylesProps) {
return (
<StyledDiv
className={className}
color={color}
fontFamily={fontFamily}
lineHeight={lineHeight}
color={color ?? 'var(--fgColor-default)'}
fontFamily={fontFamily ?? 'normal'}
lineHeight={lineHeight ?? 'default'}
data-portal-root
/**
* We need to map valid primer/react color modes onto valid color modes for primer/primitives
Expand All @@ -134,6 +132,7 @@ function BaseStyles(props: BaseStylesProps) {
data-color-mode={colorScheme?.includes('dark') ? 'dark' : 'light'}
data-light-theme={dayScheme}
data-dark-theme={nightScheme}
style={style}
{...rest}
>
<GlobalStyle colorScheme={colorScheme?.includes('dark') ? 'dark' : 'light'} />
Expand Down
Loading