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(SubNav): Convert SubNav to CSS modules behind team feature flag#5202
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
954b4bdfa7ef831277fa3667791d5021911b900a89ba2344cad185bd1ec2c06File 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 `SubNav` component to use CSS modules behind the feature flag primer_react_css_modules_team |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| .SubNavDev { | ||
| padding: var(--base-size-4); | ||
| border: var(--borderWidth-thick) solid var(--borderColor-default); | ||
| } | ||
| .SubNavLinksDev { | ||
| margin: var(--base-size-8); | ||
| } | ||
| .SubNavLinkDev { | ||
| font-weight: var(--text-title-weight-large); | ||
| color: var(--fgColor-accent); | ||
| &:is([data-selected]) { | ||
| background-color: var(--bgColor-open-emphasis); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| import React from 'react' | ||
| import type {Meta} from '@storybook/react' | ||
| import SubNav from './SubNav' | ||
| import type {ComponentProps} from '../utils/types' | ||
| import {FeatureFlags} from '../FeatureFlags' | ||
| import styles from './SubNav.dev.module.css' | ||
| export default { | ||
| title: 'Components/SubNav/Dev', | ||
| component: SubNav, | ||
| subcomponents: { | ||
| 'SubNav.Link': SubNav.Link, | ||
| }, | ||
| } as Meta<ComponentProps<typeof SubNav>> | ||
| export const WithCss = () => ( | ||
| <FeatureFlags | ||
| flags={{ | ||
| primer_react_css_modules_team: true, | ||
| primer_react_css_modules_staff: true, | ||
| primer_react_css_modules_ga: true, | ||
| }} | ||
| > | ||
| <SubNav aria-label="Main" className={styles.SubNavDev}> | ||
| <SubNav.Links className={styles.SubNavLinksDev}> | ||
| <SubNav.Link href="#home" selected className={styles.SubNavLinkDev}> | ||
| Home | ||
| </SubNav.Link> | ||
| <SubNav.Link href="#documentation" className={styles.SubNavLinkDev}> | ||
| Documentation | ||
| </SubNav.Link> | ||
| <SubNav.Link href="#support" className={styles.SubNavLinkDev}> | ||
| Support | ||
| </SubNav.Link> | ||
| </SubNav.Links> | ||
| </SubNav> | ||
| </FeatureFlags> | ||
| ) | ||
| export const WithSx = () => ( | ||
| <SubNav aria-label="Main" sx={{p: 1, display: 'flex', border: '2px solid', borderColor: 'border.default'}}> | ||
| <SubNav.Links sx={{m: 2}}> | ||
| <SubNav.Link | ||
| href="#home" | ||
| selected | ||
| sx={{color: 'accent.fg', fontWeight: 'bold', '&:is([data-selected])': {backgroundColor: 'danger.fg'}}} | ||
| > | ||
| Home | ||
| </SubNav.Link> | ||
| <SubNav.Link href="#documentation" sx={{color: 'accent.fg', fontWeight: 'bold'}}> | ||
| Documentation | ||
| </SubNav.Link> | ||
| <SubNav.Link href="#support" sx={{color: 'accent.fg', fontWeight: 'bold'}}> | ||
| Support | ||
| </SubNav.Link> | ||
| </SubNav.Links> | ||
| </SubNav> | ||
| ) | ||
| export const WithSxAndCSS = () => ( | ||
| <FeatureFlags | ||
| flags={{ | ||
| primer_react_css_modules_team: true, | ||
| primer_react_css_modules_staff: true, | ||
| primer_react_css_modules_ga: true, | ||
| }} | ||
| > | ||
| <SubNav | ||
| aria-label="Main" | ||
| sx={{p: 1, display: 'flex', border: '2px solid', borderColor: 'border.default'}} | ||
| className={styles.SubNavDev} | ||
| > | ||
| <SubNav.Links sx={{m: 2}} className={styles.SubNavLinksDev}> | ||
| <SubNav.Link | ||
| href="#home" | ||
| selected | ||
| className={styles.SubNavLinkDev} | ||
| sx={{'&:is([data-selected])': {backgroundColor: 'danger.fg'}}} | ||
| > | ||
| Home | ||
| </SubNav.Link> | ||
| <SubNav.Link href="#documentation" className={styles.SubNavLinkDev}> | ||
| Documentation | ||
| </SubNav.Link> | ||
| <SubNav.Link href="#support" sx={{color: 'accent.fg', fontWeight: 'bold'}} className={styles.SubNavLinkDev}> | ||
| Support | ||
| </SubNav.Link> | ||
| </SubNav.Links> | ||
| </SubNav> | ||
| </FeatureFlags> | ||
| ) |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,69 @@ | ||||||||
| .SubNav { | ||||||||
| display: flex; | ||||||||
| justify-content: space-between; | ||||||||
| } | ||||||||
| .Body { | ||||||||
| display: flex; | ||||||||
| /* stylelint-disable-next-line primer/spacing */ | ||||||||
| margin-bottom: -1px; | ||||||||
| & > * { | ||||||||
| margin-left: var(--base-size-8); | ||||||||
| } | ||||||||
| & > *:first-child { | ||||||||
| margin-left: 0; | ||||||||
| } | ||||||||
| } | ||||||||
| .Actions { | ||||||||
| align-self: center; | ||||||||
| } | ||||||||
| .Links { | ||||||||
| display: flex; | ||||||||
| } | ||||||||
| .Link { | ||||||||
| display: flex; | ||||||||
| min-height: 34px; /* custom values for SubNav */ | ||||||||
| padding-right: var(--base-size-16); | ||||||||
| padding-left: var(--base-size-16); | ||||||||
| font-size: var(--text-body-size-medium); | ||||||||
| font-weight: var(--base-text-weight-medium); | ||||||||
| /* stylelint-disable-next-line primer/typography */ | ||||||||
| line-height: 20px; /* custom values for SubNav */ | ||||||||
| color: var(--fgColor-default); | ||||||||
| text-align: center; | ||||||||
| text-decoration: none; | ||||||||
| border-top: var(--borderWidth-thin) solid var(--borderColor-default); | ||||||||
| border-right: var(--borderWidth-thin) solid var(--borderColor-default); | ||||||||
| border-bottom: var(--borderWidth-thin) solid var(--borderColor-default); | ||||||||
| align-items: center; | ||||||||
| &:first-of-type { | ||||||||
| border-left: var(--borderWidth-thin) solid var(--borderColor-default); | ||||||||
| border-top-left-radius: var(--borderRadius-medium); | ||||||||
| border-bottom-left-radius: var(--borderRadius-medium); | ||||||||
| } | ||||||||
| &:last-of-type { | ||||||||
| border-top-right-radius: var(--borderRadius-medium); | ||||||||
| border-bottom-right-radius: var(--borderRadius-medium); | ||||||||
| } | ||||||||
| &:hover, | ||||||||
| &:focus { | ||||||||
| text-decoration: none; | ||||||||
| background-color: var(--bgColor-muted); | ||||||||
| transition: background-color 0.2s ease; | ||||||||
| } | ||||||||
| &:is([data-selected]) { | ||||||||
| color: var(--fgColor-onEmphasis); | ||||||||
| background-color: var(--bgColor-accent-emphasis); | ||||||||
| /* stylelint-disable-next-line primer/colors */ | ||||||||
| border-color: var(--bgColor-accent-emphasis); | ||||||||
Comment on lines
+66
to
+67
| ||||||||
| /* stylelint-disable-next-line primer/colors */ | |
| border-color:var(--bgColor-accent-emphasis); | |
| border-color:var(--borderColor-accent-emphasis); |
Should actually be the same end result
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.
@langermank Reverted this change since VRT was failing due to the border having a different color with Dark Hight Contrast

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.
Should this one be
:where?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.
When I tried using
:where, the hover/focus styling was overriding the selected styling, which is why I switched to:is. Open to alternative solutions.