From 61604369a18dec6a504ef520a37e59ad29e4a9eb Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Tue, 8 Oct 2024 19:15:44 -0700 Subject: [PATCH] =?UTF-8?q?Revert=20"Convert=20`Breadcrumbs`=20and=20`Brea?= =?UTF-8?q?dcrumbs.Item`=20to=20CSS=20modules=20behind=20th=E2=80=A6"=20(#?= =?UTF-8?q?5092)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 6d8b7f4f616d46b022aec32d292177b529ea01d4. --- .changeset/mighty-buckets-push.md | 5 - e2e/components/Breadcrumbs.test.ts | 48 ------ packages/react/src/Banner/Banner.tsx | 1 - .../src/Breadcrumbs/Breadcrumbs.module.css | 57 ------- .../react/src/Breadcrumbs/Breadcrumbs.tsx | 147 ++++++------------ .../react/src/ButtonGroup/ButtonGroup.tsx | 1 - .../__tests__/toggleStyledComponent.test.tsx | 32 +--- .../internal/utils/toggleStyledComponent.tsx | 10 +- 8 files changed, 55 insertions(+), 246 deletions(-) delete mode 100644 .changeset/mighty-buckets-push.md delete mode 100644 packages/react/src/Breadcrumbs/Breadcrumbs.module.css diff --git a/.changeset/mighty-buckets-push.md b/.changeset/mighty-buckets-push.md deleted file mode 100644 index 4d4b0f6eea5..00000000000 --- a/.changeset/mighty-buckets-push.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"@primer/react": minor ---- - -Convert `Breadcrumbs` and `Breadcrumbs.Item` to CSS modules behind the primer_react_css_modules_team feature flag diff --git a/e2e/components/Breadcrumbs.test.ts b/e2e/components/Breadcrumbs.test.ts index badd8ea4c0f..5b4a763084e 100644 --- a/e2e/components/Breadcrumbs.test.ts +++ b/e2e/components/Breadcrumbs.test.ts @@ -11,32 +11,6 @@ test.describe('Breadcrumbs', () => { id: 'components-breadcrumbs--default', globals: { colorScheme: theme, - featureFlags: { - primer_react_css_modules_team: true, - }, - }, - }) - - // Default state - expect(await page.screenshot()).toMatchSnapshot(`Breadcrumbs.Default.${theme}.png`) - - // Hover state - await page.getByRole('link', {name: 'Home'}).hover() - expect(await page.screenshot()).toMatchSnapshot(`Breadcrumbs.Default.${theme}.hover.png`) - - // Focus state - await page.keyboard.press('Tab') - expect(await page.screenshot()).toMatchSnapshot(`Breadcrumbs.Default.${theme}.focus.png`) - }) - - test('default @vrt (styled components)', async ({page}) => { - await visit(page, { - id: 'components-breadcrumbs--default', - globals: { - colorScheme: theme, - featureFlags: { - primer_react_css_modules_team: false, - }, }, }) @@ -57,28 +31,6 @@ test.describe('Breadcrumbs', () => { id: 'components-breadcrumbs--default', globals: { colorScheme: theme, - featureFlags: { - primer_react_css_modules_team: true, - }, - }, - }) - await expect(page).toHaveNoViolations({ - rules: { - 'color-contrast': { - enabled: theme !== 'dark_dimmed', - }, - }, - }) - }) - - test('axe @aat (styled components)', async ({page}) => { - await visit(page, { - id: 'components-breadcrumbs--default', - globals: { - colorScheme: theme, - featureFlags: { - primer_react_css_modules_team: false, - }, }, }) await expect(page).toHaveNoViolations({ diff --git a/packages/react/src/Banner/Banner.tsx b/packages/react/src/Banner/Banner.tsx index f33021d67d7..6b85cf67c5e 100644 --- a/packages/react/src/Banner/Banner.tsx +++ b/packages/react/src/Banner/Banner.tsx @@ -209,7 +209,6 @@ const StyledBanner = toggleStyledComponent( * line-height of `20px` so that means that the height of icons should match * that value. */ - 'div', styled.div` display: grid; grid-template-columns: auto minmax(0, 1fr) auto; diff --git a/packages/react/src/Breadcrumbs/Breadcrumbs.module.css b/packages/react/src/Breadcrumbs/Breadcrumbs.module.css deleted file mode 100644 index 9a1d621fa39..00000000000 --- a/packages/react/src/Breadcrumbs/Breadcrumbs.module.css +++ /dev/null @@ -1,57 +0,0 @@ -.BreadcrumbsBase { - display: flex; - justify-content: space-between; -} - -.BreadcrumbsList { - padding-left: 0; - margin-top: 0; - margin-bottom: 0; -} - -.ItemWrapper { - display: inline-block; - font-size: var(--text-body-size-medium); - white-space: nowrap; - list-style: none; - - &::after { - display: inline-block; - height: 0.8em; - /* stylelint-disable-next-line primer/spacing */ - margin: 0 0.5em; - font-size: var(--text-body-size-medium); - content: ''; - /* stylelint-disable-next-line primer/borders */ - border-right: 0.1em solid var(--fgColor-muted); - transform: rotate(15deg) translateY(0.0625em); - } - - &:first-child { - margin-left: 0; - } - - &:last-child { - &::after { - content: none; - } - } -} - -.Item { - display: inline-block; - - &:hover, - &:focus { - text-decoration: underline; - } -} - -.ItemSelected { - color: var(--fgColor-default); - pointer-events: none; - - &:focus { - text-decoration: none; - } -} diff --git a/packages/react/src/Breadcrumbs/Breadcrumbs.tsx b/packages/react/src/Breadcrumbs/Breadcrumbs.tsx index 909df8ae58d..7b1753aa853 100644 --- a/packages/react/src/Breadcrumbs/Breadcrumbs.tsx +++ b/packages/react/src/Breadcrumbs/Breadcrumbs.tsx @@ -7,50 +7,38 @@ import {get} from '../constants' import type {SxProp} from '../sx' import sx from '../sx' import type {ComponentProps} from '../utils/types' -import classes from './Breadcrumbs.module.css' -import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' -import {FeatureFlags, useFeatureFlag} from '../FeatureFlags' -import Link from '../Link' const SELECTED_CLASS = 'selected' -const Wrapper = toggleStyledComponent( - 'primer_react_css_modules_team', - 'li', - styled.li` +const Wrapper = styled.li` + display: inline-block; + white-space: nowrap; + list-style: none; + &::after { + font-size: ${get('fontSizes.1')}; + content: ''; display: inline-block; - white-space: nowrap; - list-style: none; + height: 0.8em; + margin: 0 0.5em; + border-right: 0.1em solid; + border-color: ${get('colors.fg.muted')}; + transform: rotate(15deg) translateY(0.0625em); + } + &:first-child { + margin-left: 0; + } + &:last-child { &::after { - font-size: ${get('fontSizes.1')}; - content: ''; - display: inline-block; - height: 0.8em; - margin: 0 0.5em; - border-right: 0.1em solid; - border-color: ${get('colors.fg.muted')}; - transform: rotate(15deg) translateY(0.0625em); - } - &:first-child { - margin-left: 0; - } - &:last-child { - &::after { - content: none; - } + content: none; } - `, -) + } +` -const BreadcrumbsBase = toggleStyledComponent( - 'primer_react_css_modules_team', - 'nav', - styled.nav` - display: flex; - justify-content: space-between; - ${sx}; - `, -) +const BreadcrumbsBase = styled.nav` + display: flex; + justify-content: space-between; + ${sx}; +` export type BreadcrumbsProps = React.PropsWithChildren< { @@ -58,31 +46,13 @@ export type BreadcrumbsProps = React.PropsWithChildren< } & SxProp > -const BreadcrumbsList = ({children}: React.PropsWithChildren) => { - const enabled = useFeatureFlag('primer_react_css_modules_team') - if (enabled) { - return
    {children}
- } - - return ( - - {children} - - ) -} - function Breadcrumbs({className, children, sx: sxProp}: React.PropsWithChildren) { - const enabled = useFeatureFlag('primer_react_css_modules_team') - const wrappedChildren = React.Children.map(children, child => ( - {child} - )) + const wrappedChildren = React.Children.map(children, child => {child}) return ( - - {wrappedChildren} + + + {wrappedChildren} + ) } @@ -92,46 +62,27 @@ type StyledBreadcrumbsItemProps = { selected?: boolean } & SxProp -const StyledBreadcrumbsItem = toggleStyledComponent( - 'primer_react_css_modules_team', - 'a', - styled.a.attrs(props => ({ - className: clsx(props.selected && SELECTED_CLASS, props.className), - 'aria-current': props.selected ? 'page' : null, - }))` - color: ${get('colors.accent.fg')}; - display: inline-block; - font-size: ${get('fontSizes.1')}; +const BreadcrumbsItem = styled.a.attrs(props => ({ + className: clsx(props.selected && SELECTED_CLASS, props.className), + 'aria-current': props.selected ? 'page' : null, +}))` + color: ${get('colors.accent.fg')}; + display: inline-block; + font-size: ${get('fontSizes.1')}; + text-decoration: none; + &:hover, + &:focus { + text-decoration: underline; + } + &.selected { + color: ${get('colors.fg.default')}; + pointer-events: none; + } + &.selected:focus { text-decoration: none; - &:hover, - &:focus { - text-decoration: underline; - } - &.selected { - color: ${get('colors.fg.default')}; - pointer-events: none; - } - &.selected:focus { - text-decoration: none; - } - ${sx}; - `, -) -const BreadcrumbsItem = ({ - selected, - ...props -}: StyledBreadcrumbsItemProps & React.ComponentPropsWithoutRef) => { - const enabled = useFeatureFlag('primer_react_css_modules_team') - if (enabled) { - return ( - // Remove this when the feature flag is removed from Link - - - - ) } - return -} + ${sx}; +` Breadcrumbs.displayName = 'Breadcrumbs' diff --git a/packages/react/src/ButtonGroup/ButtonGroup.tsx b/packages/react/src/ButtonGroup/ButtonGroup.tsx index 74f9ff32e15..b700f4fcd43 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.tsx +++ b/packages/react/src/ButtonGroup/ButtonGroup.tsx @@ -10,7 +10,6 @@ import {useFeatureFlag} from '../FeatureFlags' const StyledButtonGroup = toggleStyledComponent( 'primer_react_css_modules_staff', - 'div', styled.div` display: inline-flex; vertical-align: middle; diff --git a/packages/react/src/internal/utils/__tests__/toggleStyledComponent.test.tsx b/packages/react/src/internal/utils/__tests__/toggleStyledComponent.test.tsx index bd243f447cb..6a4206003d0 100644 --- a/packages/react/src/internal/utils/__tests__/toggleStyledComponent.test.tsx +++ b/packages/react/src/internal/utils/__tests__/toggleStyledComponent.test.tsx @@ -5,32 +5,8 @@ import {toggleStyledComponent} from '../toggleStyledComponent' import styled from 'styled-components' describe('toggleStyledComponent', () => { - test('renders the component as `defaultAs` when flag is enabled and no `as` prop is provided', () => { - const TestComponent = toggleStyledComponent('testFeatureFlag', 'span', ({as: BaseComponent = 'div'}) => ( - - )) - const {container} = render( - - - , - ) - expect(container.firstChild).toBeInstanceOf(HTMLSpanElement) - }) - - test('renders the component as `defaultAs` when flag is disabled and no `as` prop is provided', () => { - const TestComponent = toggleStyledComponent('testFeatureFlag', 'span', ({as: BaseComponent = 'div'}) => ( - - )) - const {container} = render( - - - , - ) - expect(container.firstChild).toBeInstanceOf(HTMLSpanElement) - }) - test('renders the `as` prop when flag is enabled', () => { - const TestComponent = toggleStyledComponent('testFeatureFlag', 'div', () =>
) + const TestComponent = toggleStyledComponent('testFeatureFlag', () =>
) const {container} = render( @@ -40,7 +16,7 @@ describe('toggleStyledComponent', () => { }) test('renders a div as fallback when flag is enabled and no `as` prop is provided', () => { - const TestComponent = toggleStyledComponent('testFeatureFlag', 'div', () =>
) + const TestComponent = toggleStyledComponent('testFeatureFlag', () =>
) const {container} = render( @@ -50,7 +26,7 @@ describe('toggleStyledComponent', () => { }) test('renders Box with `as` if `sx` is provided and flag is enabled', () => { - const TestComponent = toggleStyledComponent('testFeatureFlag', 'div', () => styled.div``) + const TestComponent = toggleStyledComponent('testFeatureFlag', () => styled.div``) const {container} = render( @@ -62,7 +38,7 @@ describe('toggleStyledComponent', () => { }) test('renders styled component when flag is disabled', () => { - const StyledComponent = toggleStyledComponent('testFeatureFlag', 'div', styled.div.attrs({['data-styled']: true})``) + const StyledComponent = toggleStyledComponent('testFeatureFlag', styled.div.attrs({['data-styled']: true})``) const {container} = render( diff --git a/packages/react/src/internal/utils/toggleStyledComponent.tsx b/packages/react/src/internal/utils/toggleStyledComponent.tsx index 5c551af11f8..7b366cb0eb0 100644 --- a/packages/react/src/internal/utils/toggleStyledComponent.tsx +++ b/packages/react/src/internal/utils/toggleStyledComponent.tsx @@ -17,18 +17,12 @@ type CSSModulesProps = { * * @param flag - the feature flag that will control whether or not the provided * styled component is used - * @param defautlAs - the default component to use when `as` is not provided * @param Component - the styled component that will be used if the feature flag * is disabled */ -export function toggleStyledComponent( - flag: string, - // eslint-disable-next-line @typescript-eslint/no-explicit-any - defaultAs: string | React.ComponentType, - Component: React.ComponentType

, -) { +export function toggleStyledComponent(flag: string, Component: React.ComponentType

) { const Wrapper = React.forwardRef(function Wrapper( - {as: BaseComponent = defaultAs, sx: sxProp = defaultSxProp, ...rest}, + {as: BaseComponent = 'div', sx: sxProp = defaultSxProp, ...rest}, ref, ) { const enabled = useFeatureFlag(flag)