Skip to content

Commit

Permalink
fix(BaseStyles): remove unknown props from fallback when feature flag…
Browse files Browse the repository at this point in the history
… is enabled (#5606)

* refactor(storybook): add feature flags as outermost component

* fix(FeatureFlags): merge scope with parent context scope

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

* chore: add changeset

* chore: fix eslint violation

* test(vrt): update snapshots

* chore: add stylelint disables

* test(vrt): update snapshots

* Revert "test(vrt): update snapshots"

This reverts commit f127d0c.

* fix(BaseStyles): add data-color-mode support for color-scheme

* test(vrt): update snapshots

---------

Co-authored-by: joshblack <joshblack@users.noreply.github.com>
Co-authored-by: Jon Rohan <yes@jonrohan.codes>
  • Loading branch information
3 people authored Jan 28, 2025
1 parent 50ef6a4 commit f291e81
Show file tree
Hide file tree
Showing 32 changed files with 68 additions and 38 deletions.
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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
26 changes: 26 additions & 0 deletions packages/react/src/BaseStyles.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,26 @@ table {
border-collapse: collapse;
}

[data-color-mode='light'] input {
color-scheme: light;
}

[data-color-mode='dark'] input {
color-scheme: dark;
}

@media (prefers-color-scheme: light) {
[data-color-mode='auto'][data-light-theme*='light'] {
color-scheme: light;
}
}

@media (prefers-color-scheme: dark) {
[data-color-mode='auto'][data-dark-theme*='dark'] {
color-scheme: dark;
}
}

[role='button']:focus:not(:focus-visible):not(.focus-visible),
[role='tabpanel'][tabindex='0']:focus:not(:focus-visible):not(.focus-visible),
button:focus:not(:focus-visible):not(.focus-visible),
Expand All @@ -34,6 +54,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
8 changes: 4 additions & 4 deletions packages/react/src/FeatureFlags/FeatureFlags.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import React, {useMemo} from 'react'
import React, {useContext, useMemo} from 'react'
import {FeatureFlagContext} from './FeatureFlagContext'
import {FeatureFlagScope, type FeatureFlags} from './FeatureFlagScope'
import {DefaultFeatureFlags} from './DefaultFeatureFlags'

export type FeatureFlagsProps = React.PropsWithChildren<{
flags: FeatureFlags
}>

export function FeatureFlags({children, flags}: FeatureFlagsProps) {
const parentFeatureFlags = useContext(FeatureFlagContext)
const value = useMemo(() => {
const scope = FeatureFlagScope.merge(DefaultFeatureFlags, FeatureFlagScope.create(flags))
const scope = FeatureFlagScope.merge(parentFeatureFlags, FeatureFlagScope.create(flags))
return scope
}, [flags])
}, [parentFeatureFlags, flags])
return <FeatureFlagContext.Provider value={value}>{children}</FeatureFlagContext.Provider>
}

0 comments on commit f291e81

Please sign in to comment.