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

feat(Header): Convert Header to CSS modules behind team feature flag #5192

Merged
merged 11 commits into from
Oct 30, 2024
5 changes: 5 additions & 0 deletions .changeset/odd-frogs-listen.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

Update `Header` component to use CSS modules behind the feature flag primer_react_css_modules_team
11 changes: 11 additions & 0 deletions packages/react/src/Header/Header.dev.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
.HeaderDev {
background-color: var(--label-olive-bgColor-active);
}

.HeaderDevItem {
padding-left: var(--base-size-24);
}

.HeaderDevLink {
color: var(--color-prettylights-syntax-markup-inserted-text);
}
76 changes: 76 additions & 0 deletions packages/react/src/Header/Header.dev.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import React from 'react'
import type {Meta} from '@storybook/react'
import {MarkGithubIcon} from '@primer/octicons-react'

import Header from './Header'
import Avatar from '../Avatar'
import Octicon from '../Octicon'

import classes from './Header.dev.module.css'
import {FeatureFlags} from '../FeatureFlags'

export default {
title: 'Components/Header/Dev',
component: Header,
} as Meta<typeof Header>

export const WithCss = () => (
<FeatureFlags
flags={{
primer_react_css_modules_team: true,
primer_react_css_modules_staff: true,
primer_react_css_modules_ga: true,
}}
>
<Header as="summary" className={classes.HeaderDev}>
<Header.Item id="github">
<Header.Link href="#" className={classes.HeaderDevLink}>
<Octicon icon={MarkGithubIcon} size={32} sx={{mr: 2}} />
<span>GitHub</span>
</Header.Link>
</Header.Item>
<Header.Item full>Menu</Header.Item>
<Header.Item className={classes.HeaderDevItem}>
<Avatar src="https://github.com/octocat.png" size={20} square alt="@octocat" />
</Header.Item>
</Header>
</FeatureFlags>
)

export const WithSx = () => (
<Header as="summary" sx={{backgroundColor: 'blue'}}>
<Header.Item id="github">
<Header.Link href="#" sx={{fontSize: 3}}>
<Octicon icon={MarkGithubIcon} size={32} sx={{mr: 2}} />
<span>GitHub</span>
</Header.Link>
</Header.Item>
<Header.Item full>Menu</Header.Item>
<Header.Item sx={{mr: 2}}>
<Avatar src="https://github.com/octocat.png" size={20} square alt="@octocat" />
</Header.Item>
</Header>
)

export const WithSxAndCSS = () => (
<FeatureFlags
flags={{
primer_react_css_modules_team: true,
primer_react_css_modules_staff: true,
primer_react_css_modules_ga: true,
}}
>
<Header as="summary" className={classes.HeaderDev} sx={{backgroundColor: 'orange'}}>
<Header.Item id="github">
<Header.Link href="#" className={classes.HeaderDevLink} sx={{p: 0, color: 'black'}}>
<Octicon icon={MarkGithubIcon} size={32} sx={{mr: 2}} />
<span>GitHub</span>
</Header.Link>
</Header.Item>
<Header.Item full>Menu</Header.Item>
<Header.Item className={classes.HeaderDevItem} sx={{m: 0}}>
<Avatar src="https://github.com/octocat.png" size={20} square alt="@octocat" />
</Header.Item>
</Header>
</FeatureFlags>
)
39 changes: 39 additions & 0 deletions packages/react/src/Header/Header.module.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
.Header {
z-index: 32;
display: flex;
padding: var(--base-size-16);
overflow: auto;
font-size: var(--text-body-size-medium);
line-height: var(--text-title-lineHeight-large);
color: var(--header-fgColor-default);
background-color: var(--header-bgColor);
align-items: center;
flex-wrap: nowrap;
}

.HeaderItem {
display: flex;
margin-right: var(--base-size-16);
align-self: stretch;
align-items: center;
flex-wrap: nowrap;

&:where([data-full]) {
flex: auto;
}
}

.HeaderLink {
display: flex;
font-weight: var(--text-title-weight-large);
color: var(--header-fgColor-logo);
text-decoration: none;
white-space: nowrap;
cursor: pointer;
align-items: center;

&:hover,
&:focus {
color: var(--header-fgColor-default);
}
}
178 changes: 121 additions & 57 deletions packages/react/src/Header/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,68 +4,132 @@ import {get} from '../constants'
import type {SxProp} from '../sx'
import sx from '../sx'
import type {ComponentProps} from '../utils/types'
import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent'
import {useFeatureFlag} from '../FeatureFlags'
import React from 'react'
import {clsx} from 'clsx'
import classes from './Header.module.css'
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'

type StyledHeaderItemProps = {full?: boolean} & SxProp
type StyledHeaderProps = SxProp
type StyledHeaderLinkProps = {to?: Location | Pathname} & SxProp

const Header = styled.header<StyledHeaderProps>`
z-index: 32;
display: flex;
padding: ${get('space.3')};
font-size: ${get('fontSizes.1')};
line-height: ${get('lineHeights.default')};
color: ${get('colors.header.text')};
background-color: ${get('colors.header.bg')};
align-items: center;
flex-wrap: nowrap;
overflow: auto;

${sx};
`
const HeaderItem = styled.div<StyledHeaderItemProps>`
display: flex;
margin-right: ${get('space.3')};
align-self: stretch;
align-items: center;
flex-wrap: nowrap;

${({full}) =>
full &&
css`
flex: auto;
`};

${sx};
`
type StyledHeaderProps = React.ComponentProps<'header'> & SxProp
type StyledHeaderItemProps = React.ComponentProps<'div'> & SxProp & {full?: boolean}
type StyledHeaderLinkProps = React.ComponentProps<'a'> & SxProp & {to?: Location | Pathname}

HeaderItem.displayName = 'Header.Item'
const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_team'

const HeaderLink = styled.a.attrs<StyledHeaderLinkProps>(({to}) => {
const isReactRouter = typeof to === 'string'
if (isReactRouter) {
// according to their docs, NavLink supports aria-current:
// https://reacttraining.com/react-router/web/api/NavLink/aria-current-string
return {'aria-current': 'page'}
} else {
return {}
}
})<StyledHeaderLinkProps>`
font-weight: ${get('fontWeights.bold')};
color: ${get('colors.header.logo')};
white-space: nowrap;
cursor: pointer;
text-decoration: none;
display: flex;
align-items: center;

&:hover,
&:focus {
const StyledHeader = toggleStyledComponent(
CSS_MODULES_FEATURE_FLAG,
'header',
styled.header<StyledHeaderProps>`
z-index: 32;
display: flex;
padding: ${get('space.3')};
font-size: ${get('fontSizes.1')};
line-height: ${get('lineHeights.default')};
color: ${get('colors.header.text')};
}
background-color: ${get('colors.header.bg')};
align-items: center;
flex-wrap: nowrap;
overflow: auto;

${sx};
`,
)

const Header = React.forwardRef<HTMLElement, StyledHeaderProps>(function Header(
{children, className, ...rest},
forwardRef,
) {
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
return (
<StyledHeader ref={forwardRef} className={clsx(className, {[classes.Header]: enabled})} {...rest}>
{children}
</StyledHeader>
)
}) as PolymorphicForwardRefComponent<'header', StyledHeaderProps>

Header.displayName = 'Header'

const StyledHeaderItem = toggleStyledComponent(
CSS_MODULES_FEATURE_FLAG,
'div',
styled.div<StyledHeaderItemProps>`
display: flex;
margin-right: ${get('space.3')};
align-self: stretch;
align-items: center;
flex-wrap: nowrap;

${({full}) =>
full &&
css`
flex: auto;
`};

${sx};
`,
)

const HeaderItem = React.forwardRef<HTMLElement, StyledHeaderItemProps>(function HeaderItem(
{children, className, ...rest},
forwardRef,
) {
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
return (
<StyledHeaderItem
ref={forwardRef}
className={clsx(className, enabled && classes.HeaderItem)}
data-full={rest.full}
{...rest}
Comment on lines +82 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a challenge with this is that full will be an attribute when the flag is enabled and will give us an unexpected attribute error, e.g.

image

I think we should either update the styled component to use the data attribute pattern or only pass along full when the flag is enabled, if that makes sense.

>
{children}
</StyledHeaderItem>
)
})

HeaderItem.displayName = 'Header.Item'

const StyledHeaderLink = toggleStyledComponent(
CSS_MODULES_FEATURE_FLAG,
'a',
styled.a.attrs<StyledHeaderLinkProps>(({to}) => {
const isReactRouter = typeof to === 'string'
if (isReactRouter) {
// according to their docs, NavLink supports aria-current:
// https://reacttraining.com/react-router/web/api/NavLink/aria-current-string
return {'aria-current': 'page'}
} else {
return {}
}
Comment on lines +96 to +103
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic will need to be ported over into the HeaderLink so that it works with the flag enabled/disabled. Right now, it will only be applied when the styled component is used if I understand right 👀

})<StyledHeaderLinkProps>`
font-weight: ${get('fontWeights.bold')};
color: ${get('colors.header.logo')};
white-space: nowrap;
cursor: pointer;
text-decoration: none;
display: flex;
align-items: center;

&:hover,
&:focus {
color: ${get('colors.header.text')};
}

${sx};
`,
)

${sx};
`
const HeaderLink = React.forwardRef<HTMLElement, StyledHeaderLinkProps>(function HeaderLink(
{children, className, ...rest},
forwardRef,
) {
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
return (
<StyledHeaderLink ref={forwardRef} className={clsx(className, enabled && classes.HeaderLink)} {...rest}>
{children}
</StyledHeaderLink>
)
})

HeaderLink.displayName = 'Header.Link'

Expand Down
Loading
Loading