diff --git a/.changeset/famous-trainers-allow.md b/.changeset/famous-trainers-allow.md new file mode 100644 index 00000000000..652225b4fb2 --- /dev/null +++ b/.changeset/famous-trainers-allow.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +fix(Timeline): render as unordered list diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index b10d4e35447..ac28e67abae 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -7,4 +7,5 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({ primer_react_action_list_item_as_button: false, primer_react_select_panel_with_modern_action_list: false, primer_react_overlay_overflow: false, + primer_react_timeline_as_list: false, }) diff --git a/packages/react/src/Timeline/Timeline.docs.json b/packages/react/src/Timeline/Timeline.docs.json index f05e31eece3..e8bb818c315 100644 --- a/packages/react/src/Timeline/Timeline.docs.json +++ b/packages/react/src/Timeline/Timeline.docs.json @@ -57,6 +57,10 @@ "type": "SystemStyleObject" } ] + }, + { + "name": "Timeline.Group", + "props": [] } ] } diff --git a/packages/react/src/Timeline/Timeline.features.stories.tsx b/packages/react/src/Timeline/Timeline.features.stories.tsx index 0a6731096ff..8b4bc8002b9 100644 --- a/packages/react/src/Timeline/Timeline.features.stories.tsx +++ b/packages/react/src/Timeline/Timeline.features.stories.tsx @@ -5,6 +5,7 @@ import Timeline from './Timeline' import Octicon from '../Octicon' import {GitBranchIcon, GitCommitIcon, GitMergeIcon} from '@primer/octicons-react' import Link from '../Link' +import {FeatureFlags} from '../FeatureFlags' export default { title: 'Components/Timeline/Features', @@ -18,73 +19,91 @@ export default { } as Meta> export const ClipSidebar = () => ( - - - - - - This is a message - - - - - - This is a message - - + + + + + + + + This is a message + + + + + + This is a message + + + + ) export const CondensedItems = () => ( - - - - - - This is a message - - - - - - This is a message - - + + + + + + + + This is a message + + + + + + This is a message + + + + ) export const TimelineBreak = () => ( - - - - - - This is a message - - - - - - - This is a message - - + + + + + + + + This is a message + + + + + + + + + This is a message + + + + ) export const WithInlineLinks = () => ( - - - - - - - - Monalisa - - enabled auto-merge (squash) - - - + + + + + + + + + + Monalisa + + enabled auto-merge (squash) + + + + + ) diff --git a/packages/react/src/Timeline/Timeline.stories.tsx b/packages/react/src/Timeline/Timeline.stories.tsx index f8356d44589..ee5eeff25a2 100644 --- a/packages/react/src/Timeline/Timeline.stories.tsx +++ b/packages/react/src/Timeline/Timeline.stories.tsx @@ -4,6 +4,7 @@ import type {ComponentProps} from '../utils/types' import Timeline from './Timeline' import Octicon from '../Octicon' import {GitCommitIcon} from '@primer/octicons-react' +import {FeatureFlags} from '../FeatureFlags' export default { title: 'Components/Timeline', @@ -17,49 +18,57 @@ export default { } as Meta> export const Default = () => ( - - - - - - This is a message - - - - - - This is a message - - - - - - This is a message - - + + + + + + + + This is a message + + + + + + This is a message + + + + + + This is a message + + + + ) export const Playground: StoryFn> = args => ( - - - - - - This is a message - - - - - - This is a message - - - - - - This is a message - - + + + + + + + + This is a message + + + + + + This is a message + + + + + + This is a message + + + + ) Playground.args = { diff --git a/packages/react/src/Timeline/Timeline.tsx b/packages/react/src/Timeline/Timeline.tsx index 32c7c33aa7a..7089e9c2756 100644 --- a/packages/react/src/Timeline/Timeline.tsx +++ b/packages/react/src/Timeline/Timeline.tsx @@ -6,10 +6,20 @@ import {get} from '../constants' import type {SxProp} from '../sx' import sx from '../sx' import type {ComponentProps} from '../utils/types' +import {useFeatureFlag} from '../FeatureFlags' +import {warning} from '../utils/warning' const Timeline = styled.div<{clipSidebar?: boolean} & SxProp>` display: flex; flex-direction: column; + list-style: none; + + .Timeline-Group { + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; + } + ${props => props.clipSidebar && css` @@ -27,7 +37,7 @@ const Timeline = styled.div<{clipSidebar?: boolean} & SxProp>` type StyledTimelineItemProps = {condensed?: boolean} & SxProp -const TimelineItem = styled.div.attrs(props => ({ +const StyledTimelineItem = styled.div.attrs(props => ({ className: clsx('Timeline-Item', props.className), }))` display: flex; @@ -68,6 +78,16 @@ const TimelineItem = styled.div.attrs(props => ({ ${sx}; ` +export type TimelineItemsProps = { + as?: As +} & SxProp & + React.ComponentProps + +function TimelineItem(props: TimelineItemsProps) { + const asList = useFeatureFlag('primer_react_timeline_as_list') + return +} + export type TimelineBadgeProps = {children?: React.ReactNode} & SxProp const TimelineBadge = (props: TimelineBadgeProps) => { @@ -108,7 +128,7 @@ const TimelineBody = styled.div` ${sx}; ` -const TimelineBreak = styled.div` +const StyledTimelineBreak = styled.div` position: relative; z-index: 1; height: 24px; @@ -121,6 +141,26 @@ const TimelineBreak = styled.div` ${sx}; ` +function TimelineBreak(props: ComponentProps) { + const asList = useFeatureFlag('primer_react_timeline_as_list') + return +} + +function TimelineGroup({children, ...props}: React.ComponentPropsWithoutRef<'ul'>) { + const asList = useFeatureFlag('primer_react_timeline_as_list') + warning( + !asList, + `Timeline.Group is only meant to be used with the timeline as list feature, you may want to turn on the 'primer_react_timeline_as_list' feature flag. Using Timeline.Group without this feature may have unintended consequences`, + ) + return ( + + {children} + + ) +} + +TimelineGroup.displayName = 'Timeline.Group' + TimelineItem.displayName = 'Timeline.Item' TimelineBadge.displayName = 'Timeline.Badge' @@ -130,12 +170,13 @@ TimelineBody.displayName = 'Timeline.Body' TimelineBreak.displayName = 'Timeline.Break' export type TimelineProps = ComponentProps -export type TimelineItemsProps = ComponentProps export type TimelineBodyProps = ComponentProps export type TimelineBreakProps = ComponentProps +export type TimelineGroupProps = ComponentProps export default Object.assign(Timeline, { Item: TimelineItem, Badge: TimelineBadge, Body: TimelineBody, Break: TimelineBreak, + Group: TimelineGroup, }) diff --git a/packages/react/src/Timeline/__tests__/Timeline.test.tsx b/packages/react/src/Timeline/__tests__/Timeline.test.tsx index c02faab4135..97a04e1aaa9 100644 --- a/packages/react/src/Timeline/__tests__/Timeline.test.tsx +++ b/packages/react/src/Timeline/__tests__/Timeline.test.tsx @@ -4,6 +4,7 @@ import {render, rendersClass, behavesAsComponent, checkExports} from '../../util import React from 'react' import Timeline from '..' +import {FeatureFlags} from '../../FeatureFlags' describe('Timeline', () => { behavesAsComponent({Component: Timeline}) @@ -27,7 +28,11 @@ describe('Timeline.Item', () => { behavesAsComponent({Component: Timeline.Item}) it('should have no axe violations', async () => { - const {container} = HTMLRender() + const {container} = HTMLRender( + + + , + ) const results = await axe.run(container) expect(results).toHaveNoViolations() }) @@ -39,6 +44,22 @@ describe('Timeline.Item', () => { it('adds the Timeline-Item class', () => { expect(rendersClass(, 'Timeline-Item')).toEqual(true) }) + + it("should render as 'li' if FF is turned on", () => { + const {getByRole} = HTMLRender( + + + , + ) + expect(getByRole('listitem')).toBeInTheDocument() + expect(getByRole('listitem').tagName).toBe('LI') + }) + + it("should render as 'div' by default", () => { + const {queryByRole, getByText} = HTMLRender(test) + expect(queryByRole('listitem')).toBeNull() + expect(getByText('test').tagName).toBe('DIV') + }) }) describe('Timeline.Badge', () => { @@ -50,3 +71,48 @@ describe('Timeline.Badge', () => { expect(results).toHaveNoViolations() }) }) + +describe('Timeline.Break', () => { + behavesAsComponent({Component: Timeline.Break}) + + it('should have no axe violations', async () => { + const {container} = HTMLRender() + const results = await axe.run(container) + expect(results).toHaveNoViolations() + }) +}) + +describe('Timeline.Group', () => { + behavesAsComponent({Component: Timeline.Group, options: {skipAs: true, skipSx: true}}) + + it('should have no axe violations', async () => { + const {container} = HTMLRender( + + + + test + + + , + ) + const results = await axe.run(container) + expect(results).toHaveNoViolations() + }) + + it('should throw warning if FF not enabled', async () => { + const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + HTMLRender( + + + test + + , + ) + expect(spy).toHaveBeenCalledWith( + 'Warning:', + "Timeline.Group is only meant to be used with the timeline as list feature, you may want to turn on the 'primer_react_timeline_as_list' feature flag. Using Timeline.Group without this feature may have unintended consequences", + ) + + spy.mockRestore() + }) +}) diff --git a/packages/react/src/Timeline/__tests__/Timeline.types.test.tsx b/packages/react/src/Timeline/__tests__/Timeline.types.test.tsx index c86ca5e2b9a..74f21fa9f8e 100644 --- a/packages/react/src/Timeline/__tests__/Timeline.types.test.tsx +++ b/packages/react/src/Timeline/__tests__/Timeline.types.test.tsx @@ -9,6 +9,7 @@ export function shouldAcceptCallWithNoProps() { + ) } @@ -26,6 +27,8 @@ export function shouldNotAcceptSystemProps() { {/* @ts-expect-error system props should not be accepted */} + {/* @ts-expect-error system props should not be accepted */} + ) } 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..65236af28aa 100644 --- a/packages/react/src/Timeline/__tests__/__snapshots__/Timeline.test.tsx.snap +++ b/packages/react/src/Timeline/__tests__/__snapshots__/Timeline.test.tsx.snap @@ -9,6 +9,13 @@ exports[`Timeline renders with clipSidebar prop 1`] = ` -webkit-flex-direction: column; -ms-flex-direction: column; flex-direction: column; + list-style: none; +} + +.c0 .Timeline-Group { + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; } .c0 .Timeline-Item:first-child {