diff --git a/.changeset/timeline-list-semantics.md b/.changeset/timeline-list-semantics.md new file mode 100644 index 00000000000..c1f63f19e7a --- /dev/null +++ b/.changeset/timeline-list-semantics.md @@ -0,0 +1,17 @@ +--- +'@primer/react': patch +--- + +Timeline: Add `primer_react_timeline_list_semantics` feature flag to opt into list semantics + +When the `primer_react_timeline_list_semantics` feature flag is enabled, `Timeline` renders as `
    ` and `Timeline.Item` / `Timeline.Break` render as `
  1. ` so screen reader users get list navigation (total item count, position in sequence). The default behavior is unchanged — `Timeline` and its subcomponents still render as `
    ` until the flag is opted into. + +Enable the flag with the `FeatureFlags` provider: + +```tsx +import {FeatureFlags} from '@primer/react/experimental' + + + … + +``` diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index efe60fb47ad..eebbd691c10 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -6,4 +6,5 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({ primer_react_select_panel_order_selected_at_top: false, primer_react_styled_react_use_primer_theme_providers: false, primer_react_action_list_group_heading_trailing_action: false, + primer_react_timeline_list_semantics: false, }) diff --git a/packages/react/src/Timeline/Timeline.features.stories.tsx b/packages/react/src/Timeline/Timeline.features.stories.tsx index 58a7d81e641..806f7b42e5c 100644 --- a/packages/react/src/Timeline/Timeline.features.stories.tsx +++ b/packages/react/src/Timeline/Timeline.features.stories.tsx @@ -1,5 +1,6 @@ import type {Meta} from '@storybook/react-vite' import type {ComponentProps} from '../utils/types' +import {FeatureFlags} from '../FeatureFlags' import Timeline from './Timeline' import { CheckIcon, @@ -375,3 +376,29 @@ export const WithAvatar = () => (
    ) + +export const WithListSemantics = () => ( + + + + + + + Opted in: Timeline renders as an ordered list with list items. + + + + + + Each Timeline.Item renders as a li. + + + + + + + Timeline.Break renders as a presentational li. + + + +) diff --git a/packages/react/src/Timeline/Timeline.module.css b/packages/react/src/Timeline/Timeline.module.css index 2636131a40a..bbedaee66a7 100644 --- a/packages/react/src/Timeline/Timeline.module.css +++ b/packages/react/src/Timeline/Timeline.module.css @@ -1,6 +1,9 @@ .Timeline { display: flex; flex-direction: column; + list-style: none; + padding: 0; + margin: 0; &:where([data-clip-sidebar='start']), &:where([data-clip-sidebar='both']) { diff --git a/packages/react/src/Timeline/Timeline.tsx b/packages/react/src/Timeline/Timeline.tsx index dc173c0ba35..c2a89a85db0 100644 --- a/packages/react/src/Timeline/Timeline.tsx +++ b/packages/react/src/Timeline/Timeline.tsx @@ -1,10 +1,11 @@ import {clsx} from 'clsx' import React from 'react' +import {useFeatureFlag} from '../FeatureFlags' import classes from './Timeline.module.css' type StyledTimelineProps = {clipSidebar?: boolean | 'start' | 'end' | 'both'; className?: string} -export type TimelineProps = StyledTimelineProps & React.ComponentPropsWithoutRef<'div'> +export type TimelineProps = StyledTimelineProps & Omit, 'role'> function resolveClipSidebar(clipSidebar: TimelineProps['clipSidebar']): string | undefined { if (clipSidebar === true || clipSidebar === 'both') return 'both' @@ -12,17 +13,36 @@ function resolveClipSidebar(clipSidebar: TimelineProps['clipSidebar']): string | return undefined } -const Timeline = React.forwardRef(({clipSidebar, className, ...props}, forwardRef) => { - const resolvedClipSidebar = resolveClipSidebar(clipSidebar) - return ( -
    - ) -}) +const Timeline = React.forwardRef( + ({clipSidebar, className, ...props}, forwardRef) => { + const useListSemantics = useFeatureFlag('primer_react_timeline_list_semantics') + const resolvedClipSidebar = resolveClipSidebar(clipSidebar) + + if (useListSemantics) { + return ( + // Explicit role restores list semantics in Safari/VoiceOver, which strips + // them when list-style: none is applied (WebKit intentional behaviour). + // eslint-disable-next-line jsx-a11y/no-redundant-roles +
      } + data-clip-sidebar={resolvedClipSidebar} + /> + ) + } + + return ( +
      )} + className={clsx(className, classes.Timeline)} + ref={forwardRef as React.ForwardedRef} + data-clip-sidebar={resolvedClipSidebar} + /> + ) + }, +) Timeline.displayName = 'Timeline' @@ -31,17 +51,30 @@ type StyledTimelineItemProps = {condensed?: boolean; className?: string} /** * @deprecated Use the `TimelineItemProps` type instead */ -export type TimelineItemsProps = StyledTimelineItemProps & React.ComponentPropsWithoutRef<'div'> +export type TimelineItemsProps = StyledTimelineItemProps & React.ComponentPropsWithoutRef<'li'> -export type TimelineItemProps = StyledTimelineItemProps & React.ComponentPropsWithoutRef<'div'> +export type TimelineItemProps = StyledTimelineItemProps & React.ComponentPropsWithoutRef<'li'> -const TimelineItem = React.forwardRef( +const TimelineItem = React.forwardRef( ({condensed, className, ...props}, forwardRef) => { + const useListSemantics = useFeatureFlag('primer_react_timeline_list_semantics') + + if (useListSemantics) { + return ( +
    1. } + data-condensed={condensed ? '' : undefined} + /> + ) + } + return (
      )} className={clsx(className, 'Timeline-Item', classes.TimelineItem)} - ref={forwardRef} + ref={forwardRef as React.ForwardedRef} data-condensed={condensed ? '' : undefined} /> ) @@ -95,11 +128,32 @@ TimelineBody.displayName = 'TimelineBody' export type TimelineBreakProps = { /** Class name for custom styling */ className?: string -} & React.ComponentPropsWithoutRef<'div'> +} & Omit, 'role'> + +const TimelineBreak = React.forwardRef( + ({className, ...props}, forwardRef) => { + const useListSemantics = useFeatureFlag('primer_react_timeline_list_semantics') + + if (useListSemantics) { + return ( +
    2. } + role="presentation" + /> + ) + } -const TimelineBreak = React.forwardRef(({className, ...props}, forwardRef) => { - return
      -}) + return ( +
      )} + className={clsx(className, classes.TimelineBreak)} + ref={forwardRef as React.ForwardedRef} + /> + ) + }, +) TimelineBreak.displayName = 'TimelineBreak' diff --git a/packages/react/src/Timeline/__tests__/Timeline.test.tsx b/packages/react/src/Timeline/__tests__/Timeline.test.tsx index c7f37362159..66a1e9b7061 100644 --- a/packages/react/src/Timeline/__tests__/Timeline.test.tsx +++ b/packages/react/src/Timeline/__tests__/Timeline.test.tsx @@ -1,12 +1,28 @@ import {render} from '@testing-library/react' +import type {ReactElement} from 'react' import {describe, expect, it} from 'vitest' import Timeline from '..' +import {FeatureFlags} from '../../FeatureFlags' import {implementsClassName} from '../../utils/testing' import classes from '../Timeline.module.css' +function renderWithListSemantics(ui: ReactElement) { + return render({ui}) +} + describe('Timeline', () => { implementsClassName(Timeline, classes.Timeline) + it('renders as a div by default (flag off)', () => { + const {container} = render() + expect(container.firstChild?.nodeName).toBe('DIV') + }) + + it('does not set role="list" by default (flag off)', () => { + const {container} = render() + expect(container.firstChild).not.toHaveAttribute('role') + }) + it('renders with clipSidebar prop (boolean)', () => { const {container} = render() expect(container.firstChild).toHaveAttribute('data-clip-sidebar', 'both') @@ -38,8 +54,41 @@ describe('Timeline', () => { }) }) +describe('Timeline with primer_react_timeline_list_semantics flag', () => { + it('renders as an ordered list', () => { + const {container} = renderWithListSemantics() + expect(container.firstChild?.nodeName).toBe('OL') + }) + + it('has role="list" to restore semantics in Safari/VoiceOver', () => { + const {container} = renderWithListSemantics() + expect(container.firstChild).toHaveAttribute('role', 'list') + }) + + it('renders items as list items', () => { + const {container} = renderWithListSemantics( + + + , + ) + expect(container.querySelector('ol > li')).not.toBeNull() + }) + + it('renders break as a presentational list item', () => { + const {container} = renderWithListSemantics() + expect(container.firstChild?.nodeName).toBe('LI') + expect(container.firstChild).toHaveAttribute('role', 'presentation') + }) +}) + describe('Timeline.Item', () => { implementsClassName(Timeline.Item, classes.TimelineItem) + + it('renders as a div by default (flag off)', () => { + const {container} = render() + expect(container.firstChild?.nodeName).toBe('DIV') + }) + it('renders with condensed prop', () => { const {container} = render() expect(container).toMatchSnapshot() @@ -71,6 +120,12 @@ describe('Timeline.Body', () => { describe('Timeline.Break', () => { implementsClassName(Timeline.Break, classes.TimelineBreak) + + it('renders as a div by default (flag off)', () => { + const {container} = render() + expect(container.firstChild?.nodeName).toBe('DIV') + expect(container.firstChild).not.toHaveAttribute('role') + }) }) describe('Timeline.Actions', () => {