From dedcd3ca5e3aab23eaf908014a13f9cc859ede7b Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Mon, 17 Mar 2025 22:58:35 +0000 Subject: [PATCH 1/2] Remove the CSS modules feature flag from the Timeline component --- .changeset/lazy-hotels-brake.md | 5 + packages/react/src/Timeline/Timeline.tsx | 250 ++++++----------------- 2 files changed, 67 insertions(+), 188 deletions(-) create mode 100644 .changeset/lazy-hotels-brake.md diff --git a/.changeset/lazy-hotels-brake.md b/.changeset/lazy-hotels-brake.md new file mode 100644 index 00000000000..61c69ac2614 --- /dev/null +++ b/.changeset/lazy-hotels-brake.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +Remove the CSS modules feature flag from the Timeline component diff --git a/packages/react/src/Timeline/Timeline.tsx b/packages/react/src/Timeline/Timeline.tsx index 1e1553488b7..b859d55f21e 100644 --- a/packages/react/src/Timeline/Timeline.tsx +++ b/packages/react/src/Timeline/Timeline.tsx @@ -1,255 +1,129 @@ import {clsx} from 'clsx' -import React, {type HTMLProps} from 'react' -import styled, {css} from 'styled-components' +import React from 'react' import Box from '../Box' -import {get} from '../constants' import type {SxProp} from '../sx' -import sx from '../sx' -import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent' -import {useFeatureFlag} from '../FeatureFlags' import classes from './Timeline.module.css' import {defaultSxProp} from '../utils/defaultSxProp' -const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_ga' - type StyledTimelineProps = {clipSidebar?: boolean; className?: string} & SxProp -const ToggleTimeline = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'div', - styled.div` - display: flex; - flex-direction: column; - ${props => - props.clipSidebar && - css` - .Timeline-Item:first-child { - padding-top: 0; - } - - .Timeline-Item:last-child { - padding-bottom: 0; - } - `} - - ${sx}; - `, -) +export type TimelineProps = StyledTimelineProps & React.ComponentPropsWithoutRef<'div'> -export type TimelineProps = StyledTimelineProps & HTMLProps +const Timeline = React.forwardRef( + ({clipSidebar, className, sx: sxProp = defaultSxProp, ...props}, forwardRef) => { + if (sxProp !== defaultSxProp) { + return ( + + ) + } -const Timeline = React.forwardRef(function Timeline( - {clipSidebar, className, ...props}, - forwardRef, -) { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - if (enabled) { return ( - ) - } - - return -}) + }, +) Timeline.displayName = 'Timeline' type StyledTimelineItemProps = {condensed?: boolean; className?: string} & SxProp -const ToggleTimelineItem = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'div', - styled.div.attrs(props => ({ - className: clsx('Timeline-Item', props.className), - }))` - display: flex; - position: relative; - padding: ${get('space.3')} 0; - margin-left: ${get('space.3')}; - - &::before { - position: absolute; - top: 0; - bottom: 0; - left: 0; - display: block; - width: 2px; - content: ''; - background-color: ${get('colors.border.muted')}; - } - - ${props => - props.condensed && - css` - padding-top: ${get('space.1')}; - padding-bottom: 0; - &:last-child { - padding-bottom: ${get('space.3')}; - } - - .TimelineItem-Badge { - height: 16px; - margin-top: ${get('space.2')}; - margin-bottom: ${get('space.2')}; - color: ${get('colors.fg.muted')}; - background-color: ${get('colors.canvas.default')}; - border: 0; - } - `} - - ${sx}; - `, -) - /** * @deprecated Use the `TimelineItemProps` type instead */ -export type TimelineItemsProps = StyledTimelineItemProps & HTMLProps +export type TimelineItemsProps = StyledTimelineItemProps & SxProp & React.ComponentPropsWithoutRef<'div'> -export type TimelineItemProps = StyledTimelineItemProps & HTMLProps +export type TimelineItemProps = StyledTimelineItemProps & SxProp & React.ComponentPropsWithoutRef<'div'> -const TimelineItem = React.forwardRef(function TimelineItem( - {condensed, className, ...props}, - forwardRef, -) { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - if (enabled) { +const TimelineItem = React.forwardRef( + ({condensed, className, sx: sxProp = defaultSxProp, ...props}, forwardRef) => { + if (sxProp !== defaultSxProp) { + return ( + + ) + } return ( - ) - } - - return -}) + }, +) TimelineItem.displayName = 'TimelineItem' export type TimelineBadgeProps = {children?: React.ReactNode; className?: string} & SxProp & React.ComponentPropsWithoutRef<'div'> -const TimelineBadge = ({sx, className, ...props}: TimelineBadgeProps) => { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - if (enabled) { - if (sx !== defaultSxProp) { - return ( -
- -
- ) - } +const TimelineBadge = ({sx: sxProp = defaultSxProp, className, ...props}: TimelineBadgeProps) => { + if (sxProp !== defaultSxProp) { return (
-
+
) } return ( - - - {props.children} - - +
+
+
) } TimelineBadge.displayName = 'Timeline.Badge' -const ToggleTimelineBody = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'div', - styled.div` - min-width: 0; - max-width: 100%; - margin-top: ${get('space.1')}; - color: ${get('colors.fg.muted')}; - flex: auto; - font-size: ${get('fontSizes.1')}; - ${sx}; - `, -) - export type TimelineBodyProps = { /** Class name for custom styling */ className?: string } & SxProp & - HTMLProps - -const TimelineBody = React.forwardRef(function TimelineBody( - {className, ...props}, - forwardRef, -) { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - if (enabled) { - return - } + React.ComponentPropsWithoutRef<'div'> - return -}) +const TimelineBody = React.forwardRef( + ({className, sx: sxProp = defaultSxProp, ...props}, forwardRef) => { + if (sxProp !== defaultSxProp) { + return + } + return
+ }, +) TimelineBody.displayName = 'TimelineBody' -const ToggleTimelineBreak = toggleStyledComponent( - CSS_MODULES_FEATURE_FLAG, - 'div', - styled.div` - position: relative; - z-index: 1; - height: 24px; - margin: 0; - margin-bottom: -${get('space.3')}; - margin-left: 0; - background-color: ${get('colors.canvas.default')}; - border: 0; - border-top: ${get('space.1')} solid ${get('colors.border.default')}; - ${sx}; - `, -) - export type TimelineBreakProps = { /** Class name for custom styling */ className?: string } & SxProp & - HTMLProps - -const TimelineBreak = React.forwardRef(function TimelineBreak( - {className, ...props}, - forwardRef, -) { - const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG) - if (enabled) { - return - } + React.ComponentPropsWithoutRef<'div'> - return -}) +const TimelineBreak = React.forwardRef( + ({className, sx: sxProp = defaultSxProp, ...props}, forwardRef) => { + if (sxProp !== defaultSxProp) { + return + } + return
+ }, +) TimelineBreak.displayName = 'TimelineBreak' From 5926c3a6f7380eaf96cee5f898890461de8015a8 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Mon, 17 Mar 2025 23:08:42 +0000 Subject: [PATCH 2/2] Fix tests --- packages/react/src/Timeline/Timeline.tsx | 4 +- .../src/Timeline/__tests__/Timeline.test.tsx | 92 +++---------------- .../__snapshots__/Timeline.test.tsx.snap | 60 +----------- 3 files changed, 21 insertions(+), 135 deletions(-) diff --git a/packages/react/src/Timeline/Timeline.tsx b/packages/react/src/Timeline/Timeline.tsx index b859d55f21e..1ba71c9e5fd 100644 --- a/packages/react/src/Timeline/Timeline.tsx +++ b/packages/react/src/Timeline/Timeline.tsx @@ -53,7 +53,7 @@ const TimelineItem = React.forwardRef( ( return (
diff --git a/packages/react/src/Timeline/__tests__/Timeline.test.tsx b/packages/react/src/Timeline/__tests__/Timeline.test.tsx index f94e176c607..ccd50ab48ed 100644 --- a/packages/react/src/Timeline/__tests__/Timeline.test.tsx +++ b/packages/react/src/Timeline/__tests__/Timeline.test.tsx @@ -4,10 +4,9 @@ import {render, rendersClass, behavesAsComponent, checkExports} from '../../util import React from 'react' import Timeline from '..' -import {FeatureFlags} from '../../FeatureFlags' describe('Timeline', () => { - behavesAsComponent({Component: Timeline}) + behavesAsComponent({Component: Timeline, options: {skipAs: true}}) checkExports('Timeline', { default: Timeline, @@ -24,26 +23,12 @@ describe('Timeline', () => { }) it('should support `className` on the outermost element', () => { - const Element = () => - const FeatureFlagElement = () => { - return ( - - - - ) - } - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') + expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') }) }) describe('Timeline.Item', () => { - behavesAsComponent({Component: Timeline.Item}) + behavesAsComponent({Component: Timeline.Item, options: {skipAs: true}}) it('should have no axe violations', async () => { const {container} = HTMLRender() @@ -60,21 +45,9 @@ describe('Timeline.Item', () => { }) it('should support `className` on the outermost element', () => { - const Element = () => - const FeatureFlagElement = () => { - return ( - - - - ) - } - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') + expect(HTMLRender().container.firstChild).toHaveClass( + 'test-class-name', + ) }) }) @@ -88,20 +61,9 @@ describe('Timeline.Badge', () => { }) it('should support `className` on the outermost element', () => { - const Element = () => - const FeatureFlagElement = () => { - return ( - - - - ) - } - expect(HTMLRender().container.firstChild?.firstChild).toHaveClass('test-class-name') + expect(HTMLRender().container.firstChild?.firstChild).toHaveClass( + 'test-class-name', + ) }) }) @@ -115,21 +77,9 @@ describe('Timeline.Body', () => { }) it('should support `className` on the outermost element', () => { - const Element = () => - const FeatureFlagElement = () => { - return ( - - - - ) - } - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') + expect(HTMLRender().container.firstChild).toHaveClass( + 'test-class-name', + ) }) }) @@ -143,20 +93,8 @@ describe('Timeline.Break', () => { }) it('should support `className` on the outermost element', () => { - const Element = () => - const FeatureFlagElement = () => { - return ( - - - - ) - } - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') - expect(HTMLRender().container.firstChild).toHaveClass('test-class-name') + expect(HTMLRender().container.firstChild).toHaveClass( + 'test-class-name', + ) }) }) diff --git a/packages/react/src/Timeline/__tests__/__snapshots__/Timeline.test.tsx.snap b/packages/react/src/Timeline/__tests__/__snapshots__/Timeline.test.tsx.snap index b96e1e117af..ee54da222cb 100644 --- a/packages/react/src/Timeline/__tests__/__snapshots__/Timeline.test.tsx.snap +++ b/packages/react/src/Timeline/__tests__/__snapshots__/Timeline.test.tsx.snap @@ -1,67 +1,15 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`Timeline renders with clipSidebar prop 1`] = ` -.c0 { - display: -webkit-box; - display: -webkit-flex; - display: -ms-flexbox; - display: flex; - -webkit-flex-direction: column; - -ms-flex-direction: column; - flex-direction: column; -} - -.c0 .Timeline-Item:first-child { - padding-top: 0; -} - -.c0 .Timeline-Item:last-child { - padding-bottom: 0; -} -
`; exports[`Timeline.Item renders with condensed prop 1`] = ` -.c0 { - display: -webkit-box; - display: -webkit-flex; - display: -ms-flexbox; - display: flex; - position: relative; - padding: 16px 0; - margin-left: 16px; - padding-top: 4px; - padding-bottom: 0; -} - -.c0::before { - position: absolute; - top: 0; - bottom: 0; - left: 0; - display: block; - width: 2px; - content: ''; - background-color: var(--borderColor-muted,var(--color-border-muted,hsla(210,18%,87%,1))); -} - -.c0:last-child { - padding-bottom: 16px; -} - -.c0 .TimelineItem-Badge { - height: 16px; - margin-top: 8px; - margin-bottom: 8px; - color: var(--fgColor-muted,var(--color-fg-muted,#656d76)); - background-color: var(--bgColor-default,var(--color-canvas-default,#ffffff)); - border: 0; -} -
`;