Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 677
feat(Header): Convert Header to CSS modules behind team feature flag#5192
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
396f65298b3d88299c16161a7fcc368f26474e6bf397650462796ef0aba6715dc2773df2f1fccFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| 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 |
| 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); | ||
| } |
| 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> | ||
| ) |
| 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); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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} | ||
| > | ||
| {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
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this logic will need to be ported over into the | ||
| })<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' | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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
fullwill be an attribute when the flag is enabled and will give us an unexpected attribute error, e.g.I think we should either update the styled component to use the data attribute pattern or only pass along
fullwhen the flag is enabled, if that makes sense.