Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .changeset/action-list-group-heading-leading-visual.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
'@primer/react': minor
---

ActionList: Add `ActionList.GroupHeading.LeadingVisual` for a leading icon (or similar) on grouped list headings.

When the `primer_react_action_list_group_heading_leading_visual` feature flag is enabled, you can place an `ActionList.GroupHeading.LeadingVisual` inside `ActionList.GroupHeading` to render a decorative visual before the group's heading text. It can be combined with `ActionList.GroupHeading.TrailingAction`.

```tsx
<ActionList>
<ActionList.Group>
<ActionList.GroupHeading as="h3">
<ActionList.GroupHeading.LeadingVisual>
<FileDirectoryIcon />
</ActionList.GroupHeading.LeadingVisual>
Custom fields
</ActionList.GroupHeading>
<ActionList.Item>...</ActionList.Item>
</ActionList.Group>
</ActionList>
```
29 changes: 29 additions & 0 deletions packages/react/src/ActionList/ActionList.features.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,35 @@ export const WithTrailingActionOnGroupHeading = () => (

WithTrailingActionOnGroupHeading.storyName = 'With TrailingAction on GroupHeading (behind feature flag)'

export const WithLeadingVisualOnGroupHeading = () => (
<FeatureFlags flags={{primer_react_action_list_group_heading_leading_visual: true}}>
<ActionList>
<ActionList.Group>
<ActionList.GroupHeading as="h3">
<ActionList.GroupHeading.LeadingVisual>
<FileDirectoryIcon />
</ActionList.GroupHeading.LeadingVisual>
Custom fields
</ActionList.GroupHeading>
<ActionList.Item>Field 1</ActionList.Item>
<ActionList.Item>Field 2</ActionList.Item>
</ActionList.Group>
<ActionList.Group>
<ActionList.GroupHeading as="h3" variant="filled">
<ActionList.GroupHeading.LeadingVisual>
<ProjectIcon />
</ActionList.GroupHeading.LeadingVisual>
Repositories
</ActionList.GroupHeading>
<ActionList.Item>primer/react</ActionList.Item>
<ActionList.Item>primer/primitives</ActionList.Item>
</ActionList.Group>
</ActionList>
</FeatureFlags>
)

WithLeadingVisualOnGroupHeading.storyName = 'With LeadingVisual on GroupHeading (behind feature flag)'

export const FullVariant = () => (
<ActionList variant="full">
<ActionList.Item>Copy link</ActionList.Item>
Expand Down
10 changes: 10 additions & 0 deletions packages/react/src/ActionList/Group.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,13 @@
align-items: center;
margin-inline-start: auto;
}

.GroupHeadingWrap[data-has-leading-visual] {
flex-direction: row;
align-items: center;
gap: var(--base-size-8);

& > .GroupHeading {
align-self: center;
}
}
143 changes: 143 additions & 0 deletions packages/react/src/ActionList/Group.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,147 @@
).toThrow(/can not be used inside an ActionList with an ARIA role of "listbox"/)
})
})

describe('GroupHeading.LeadingVisual (behind feature flag)', () => {
it('renders GroupHeading.LeadingVisual as a sibling of the heading element when the feature flag is enabled', () => {
const {getByRole, getByTestId} = HTMLRender(
<FeatureFlags flags={{primer_react_action_list_group_heading_leading_visual: true}}>
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading as="h2">
<ActionList.GroupHeading.LeadingVisual>
<PlusIcon aria-label="leading-icon" data-testid="leading-visual" />

Check failure on line 244 in packages/react/src/ActionList/Group.test.tsx

View workflow job for this annotation

GitHub Actions / lint

[aria-label] text should be formatted the same as you would visual text. Use sentence case
</ActionList.GroupHeading.LeadingVisual>
Group Heading
</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>
</FeatureFlags>,
)

const heading = getByRole('heading', {level: 2})
const visual = getByTestId('leading-visual')

// The visual must NOT be inside the heading element
expect(heading).not.toContainElement(visual)
// The heading text should only contain the heading text
expect(heading).toHaveTextContent('Group Heading')
// The visual should be inside the same wrapper as the heading
expect(heading.parentElement).toContainElement(visual)
})

it('passes GroupHeading.LeadingVisual through as a child of the heading when the feature flag is disabled', () => {
const {getByRole, getByTestId} = HTMLRender(
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading as="h2">
<ActionList.GroupHeading.LeadingVisual>
<PlusIcon aria-label="leading-icon" data-testid="leading-visual" />

Check failure on line 272 in packages/react/src/ActionList/Group.test.tsx

View workflow job for this annotation

GitHub Actions / lint

[aria-label] text should be formatted the same as you would visual text. Use sentence case
</ActionList.GroupHeading.LeadingVisual>
Group Heading
</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>,
)

const heading = getByRole('heading', {level: 2})
const visual = getByTestId('leading-visual')

// Without the flag, the slot is not consumed: the visual passes through
// and still renders inside the heading element.
expect(heading).toContainElement(visual)
})

it('renders GroupHeading.LeadingVisual inside a listbox role without throwing', () => {
const {getByText, getByTestId} = HTMLRender(
<FeatureFlags flags={{primer_react_action_list_group_heading_leading_visual: true}}>
<ActionList role="listbox">
<ActionList.Group>
<ActionList.GroupHeading>
<ActionList.GroupHeading.LeadingVisual>
<PlusIcon aria-label="leading-icon" data-testid="leading-visual" />

Check failure on line 296 in packages/react/src/ActionList/Group.test.tsx

View workflow job for this annotation

GitHub Actions / lint

[aria-label] text should be formatted the same as you would visual text. Use sentence case
</ActionList.GroupHeading.LeadingVisual>
Group Heading
</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>
</FeatureFlags>,
)

const label = getByText('Group Heading')
const visual = getByTestId('leading-visual')
// The visual renders before the presentational heading span, not inside it.
expect(label).not.toContainElement(visual)
expect(label.parentElement).toContainElement(visual)
})

it('renders both LeadingVisual and TrailingAction when both feature flags are enabled', () => {
const {getByRole, getByTestId} = HTMLRender(
<FeatureFlags
flags={{
primer_react_action_list_group_heading_leading_visual: true,
primer_react_action_list_group_heading_trailing_action: true,
}}
>
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading as="h2">
<ActionList.GroupHeading.LeadingVisual>
<PlusIcon aria-label="leading-icon" data-testid="leading-visual" />

Check failure on line 326 in packages/react/src/ActionList/Group.test.tsx

View workflow job for this annotation

GitHub Actions / lint

[aria-label] text should be formatted the same as you would visual text. Use sentence case
</ActionList.GroupHeading.LeadingVisual>
Group Heading
<ActionList.GroupHeading.TrailingAction label="New field" icon={PlusIcon} />
</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>
</FeatureFlags>,
)

const heading = getByRole('heading', {level: 2})
const button = getByRole('button', {name: 'New field'})
const visual = getByTestId('leading-visual')

expect(heading).toHaveTextContent('Group Heading')
expect(heading).not.toContainElement(button)
expect(heading).not.toContainElement(visual)
expect(heading.parentElement).toContainElement(button)
expect(heading.parentElement).toContainElement(visual)
})

it('does not strip LeadingVisual when only the TrailingAction feature flag is enabled', () => {
const {getByRole, getByTestId} = HTMLRender(
<FeatureFlags flags={{primer_react_action_list_group_heading_trailing_action: true}}>
<ActionList>
<ActionList.Heading as="h1">Heading</ActionList.Heading>
<ActionList.Group>
<ActionList.GroupHeading as="h2">
<ActionList.GroupHeading.LeadingVisual>
<PlusIcon aria-label="leading-icon" data-testid="leading-visual" />

Check failure on line 356 in packages/react/src/ActionList/Group.test.tsx

View workflow job for this annotation

GitHub Actions / lint

[aria-label] text should be formatted the same as you would visual text. Use sentence case
</ActionList.GroupHeading.LeadingVisual>
Group Heading
<ActionList.GroupHeading.TrailingAction label="New field" icon={PlusIcon} />
</ActionList.GroupHeading>
<ActionList.Item>Item</ActionList.Item>
</ActionList.Group>
</ActionList>
</FeatureFlags>,
)

const heading = getByRole('heading', {level: 2})
const button = getByRole('button', {name: 'New field'})
const visual = getByTestId('leading-visual')

// TrailingAction is extracted (its flag is on), but the LeadingVisual flag
// is off so it must remain inside the heading rather than being stripped.
expect(heading).not.toContainElement(button)
expect(heading).toContainElement(visual)
})
})
})
39 changes: 27 additions & 12 deletions packages/react/src/ActionList/Group.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@
import groupClasses from './Group.module.css'
import type {FCWithSlotMarker} from '../utils/types/Slots'
import {GroupHeadingTrailingAction} from './GroupHeadingTrailingAction'
import {LeadingVisual} from './Visuals'
import {useFeatureFlag} from '../FeatureFlags'
import type {SlotConfig} from '../hooks/useSlots'

const GROUP_HEADING_TRAILING_ACTION_FEATURE_FLAG = 'primer_react_action_list_group_heading_trailing_action'
const GROUP_HEADING_LEADING_VISUAL_FEATURE_FLAG = 'primer_react_action_list_group_heading_leading_visual'

type HeadingProps = {
as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
Expand Down Expand Up @@ -162,20 +165,24 @@
const {role: listRole} = React.useContext(ListContext)
const {groupHeadingId} = React.useContext(GroupContext)
const trailingActionEnabled = useFeatureFlag(GROUP_HEADING_TRAILING_ACTION_FEATURE_FLAG)
const leadingVisualEnabled = useFeatureFlag(GROUP_HEADING_LEADING_VISUAL_FEATURE_FLAG)

// When the feature flag is on, extract a single
// ActionList.GroupHeading.TrailingAction child so it renders as a sibling
// of the heading element (inside the HeadingWrap) instead of inside the
// heading itself. When the flag is off, we fall through to the previous
// behavior so existing usage is unaffected.
const [slots, childrenWithoutSlots] = useSlots(children, {
trailingAction: GroupHeadingTrailingAction,
})
// When a feature flag is on, extract the matching
// ActionList.GroupHeading.LeadingVisual / .TrailingAction child so it renders
// as a sibling of the heading element (inside the HeadingWrap) instead of
// inside the heading itself. The config only includes enabled slots, so a
// disabled feature passes its child through unchanged (legacy behavior) and is
// never stripped from the heading content by the other feature.
const slotConfig: SlotConfig = {}
if (leadingVisualEnabled) slotConfig.leadingVisual = LeadingVisual
if (trailingActionEnabled) slotConfig.trailingAction = GroupHeadingTrailingAction
const [slots, childrenWithoutSlots] = useSlots(children, slotConfig)

const trailingAction = trailingActionEnabled ? slots.trailingAction : null
const headingChildren = trailingActionEnabled ? childrenWithoutSlots : children
const leadingVisual = slots.leadingVisual ?? null

Check failure on line 181 in packages/react/src/ActionList/Group.tsx

View workflow job for this annotation

GitHub Actions / lint

Unnecessary conditional, left-hand side of `??` operator is always `null` or `undefined`
const trailingAction = slots.trailingAction ?? null

Check failure on line 182 in packages/react/src/ActionList/Group.tsx

View workflow job for this annotation

GitHub Actions / lint

Unnecessary conditional, left-hand side of `??` operator is always `null` or `undefined`
const headingChildren = childrenWithoutSlots

if (trailingAction) {

Check failure on line 185 in packages/react/src/ActionList/Group.tsx

View workflow job for this annotation

GitHub Actions / lint

Unnecessary conditional, value is always falsy
invariant(
listRole === undefined || listRole === 'list',
`ActionList.GroupHeading.TrailingAction can not be used inside an ActionList with an ARIA role of "${listRole}". Trailing actions on group headings are only supported in lists with the default "list" role.`,
Expand Down Expand Up @@ -207,11 +214,13 @@
className={groupClasses.GroupHeadingWrap}
aria-hidden="true"
data-variant={variant}
data-has-leading-visual={leadingVisual ? '' : undefined}

Check failure on line 217 in packages/react/src/ActionList/Group.tsx

View workflow job for this annotation

GitHub Actions / lint

Unnecessary conditional, value is always falsy
// TODO: next-major: switch for data-component="ActionList.GroupHeading" next major
data-component="GroupHeadingWrap"
as={headingWrapElement}
{...props}
>
{leadingVisual}
<span className={clsx(className, groupClasses.GroupHeading)} id={groupHeadingId}>
{_internalBackwardCompatibleTitle ?? headingChildren}
</span>
Expand All @@ -222,11 +231,13 @@
<HeadingWrap
className={groupClasses.GroupHeadingWrap}
data-variant={variant}
data-has-leading-visual={leadingVisual ? '' : undefined}

Check failure on line 234 in packages/react/src/ActionList/Group.tsx

View workflow job for this annotation

GitHub Actions / lint

Unnecessary conditional, value is always falsy
data-has-trailing-action={trailingAction ? '' : undefined}
as={headingWrapElement}
// TODO: next-major: switch for data-component="ActionList.GroupHeading" next major
data-component="GroupHeadingWrap"
>
{leadingVisual}
<Heading
className={clsx(className, groupClasses.GroupHeading)}
as={as || 'h3'}
Expand All @@ -249,8 +260,12 @@
Group.__SLOT__ = Symbol('ActionList.Group')
GroupHeadingImpl.__SLOT__ = Symbol('ActionList.GroupHeading')

// Expose GroupHeadingTrailingAction as ActionList.GroupHeading.TrailingAction
// so the API mirrors the visual nesting (the action lives inside the heading).
// Expose the shared ActionList.LeadingVisual and GroupHeadingTrailingAction as
// ActionList.GroupHeading.LeadingVisual / .TrailingAction so the API mirrors the
// visual nesting (the visual and action live inside the heading). Reusing
// ActionList.LeadingVisual keeps group-heading visuals consistent with item
// visuals, the same way NavList.LeadingVisual reuses it.
export const GroupHeading = Object.assign(GroupHeadingImpl, {
LeadingVisual,
TrailingAction: GroupHeadingTrailingAction,
})
1 change: 1 addition & 0 deletions packages/react/src/FeatureFlags/DefaultFeatureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ 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_action_list_group_heading_leading_visual: false,
primer_react_timeline_list_semantics: false,
})
Loading