Skip to content

Commit 5e514f3

Browse files
chore(ActionList): Remove the CSS modules feature flag from the ActionList.Group component (#6023)
Co-authored-by: primer[bot] <119360173+primer[bot]@users.noreply.github.com>
1 parent 6c2c999 commit 5e514f3

File tree

4 files changed

+114
-186
lines changed

4 files changed

+114
-186
lines changed

.changeset/fair-hands-glow.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': minor
3+
---
4+
5+
Remove the CSS modules feature flag from the ActionList.Group component

packages/react/src/ActionList/Group.module.css

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,24 @@
1-
.Group:not(:first-child) {
2-
margin-block-start: var(--base-size-8);
1+
.Group {
2+
list-style: none;
33

4-
/* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading */
5-
/* stylelint-disable-next-line selector-max-specificity */
6-
&:has(.GroupHeadingWrap + ul > .GroupHeadingWrap) {
4+
&:not(:first-child) {
5+
margin-block-start: var(--base-size-8);
6+
7+
/* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading */
78
/* stylelint-disable-next-line selector-max-specificity */
8-
& > .GroupHeadingWrap {
9-
display: none;
9+
&:has(.GroupHeadingWrap + ul > .GroupHeadingWrap) {
10+
/* stylelint-disable-next-line selector-max-specificity */
11+
& > .GroupHeadingWrap {
12+
display: none;
13+
}
1014
}
1115
}
1216
}
1317

18+
.GroupList {
19+
padding-inline-start: 0;
20+
}
21+
1422
.GroupHeadingWrap {
1523
display: flex;
1624
font-size: var(--text-body-size-small);

packages/react/src/ActionList/Group.tsx

Lines changed: 19 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React from 'react'
22
import {useId} from '../hooks/useId'
3-
import Box from '../Box'
43
import type {SxProp} from '../sx'
54
import {ListContext, type ActionListProps} from './shared'
65
import type {AriaRole} from '../utils/types'
@@ -9,10 +8,9 @@ import {useSlots} from '../hooks/useSlots'
98
import {defaultSxProp} from '../utils/defaultSxProp'
109
import {invariant} from '../utils/invariant'
1110
import {clsx} from 'clsx'
12-
import {useFeatureFlag} from '../FeatureFlags'
1311
import classes from './ActionList.module.css'
1412
import groupClasses from './Group.module.css'
15-
import {actionListCssModulesFlag} from './featureflag'
13+
import {BoxWithFallback} from '../internal/components/BoxWithFallback'
1614

1715
type HeadingProps = {
1816
as?: 'h1' | 'h2' | 'h3' | 'h4' | 'h5' | 'h6'
@@ -31,9 +29,9 @@ const Heading: React.FC<HeadingProps & React.HTMLAttributes<HTMLHeadingElement>>
3129
}) => {
3230
return (
3331
// Box is temporary to support lingering sx usage
34-
<Box as={Component} className={className} sx={sx} id={id} {...rest}>
32+
<BoxWithFallback as={Component} className={className} sx={sx} id={id} {...rest}>
3533
{children}
36-
</Box>
34+
</BoxWithFallback>
3735
)
3836
}
3937

@@ -94,7 +92,6 @@ export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
9492
sx = defaultSxProp,
9593
...props
9694
}) => {
97-
const enabled = useFeatureFlag(actionListCssModulesFlag)
9895
const id = useId()
9996
const {role: listRole} = React.useContext(ListContext)
10097

@@ -114,70 +111,12 @@ export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
114111
groupHeadingId = id
115112
}
116113

117-
if (enabled) {
118-
if (sx !== defaultSxProp) {
119-
return (
120-
<Box
121-
as="li"
122-
className={clsx(className, groupClasses.Group)}
123-
role={listRole ? 'none' : undefined}
124-
sx={sx}
125-
{...props}
126-
>
127-
<GroupContext.Provider value={{selectionVariant, groupHeadingId}}>
128-
{title && !slots.groupHeading ? (
129-
// Escape hatch: supports old API <ActionList.Group title="group title"> in a non breaking way
130-
<GroupHeading variant={variant} auxiliaryText={auxiliaryText} _internalBackwardCompatibleTitle={title} />
131-
) : null}
132-
{/* Supports new API ActionList.GroupHeading */}
133-
{!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null}
134-
<ul
135-
// if listRole is set (listbox or menu), we don't label the list with the groupHeadingId
136-
// because the heading is hidden from the accessibility tree and only used for presentation role.
137-
// We will instead use aria-label to label the list. See a line below.
138-
aria-labelledby={listRole ? undefined : groupHeadingId}
139-
aria-label={listRole ? (title ?? (slots.groupHeading?.props.children as string)) : undefined}
140-
role={role || (listRole && 'group')}
141-
>
142-
{slots.groupHeading ? childrenWithoutSlots : props.children}
143-
</ul>
144-
</GroupContext.Provider>
145-
</Box>
146-
)
147-
}
148-
return (
149-
<li className={clsx(className, groupClasses.Group)} role={listRole ? 'none' : undefined} {...props}>
150-
<GroupContext.Provider value={{selectionVariant, groupHeadingId}}>
151-
{title && !slots.groupHeading ? (
152-
// Escape hatch: supports old API <ActionList.Group title="group title"> in a non breaking way
153-
<GroupHeading variant={variant} auxiliaryText={auxiliaryText} _internalBackwardCompatibleTitle={title} />
154-
) : null}
155-
{/* Supports new API ActionList.GroupHeading */}
156-
{!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null}
157-
<ul
158-
// if listRole is set (listbox or menu), we don't label the list with the groupHeadingId
159-
// because the heading is hidden from the accessibility tree and only used for presentation role.
160-
// We will instead use aria-label to label the list. See a line below.
161-
aria-labelledby={listRole ? undefined : groupHeadingId}
162-
aria-label={listRole ? (title ?? (slots.groupHeading?.props.children as string)) : undefined}
163-
role={role || (listRole && 'group')}
164-
>
165-
{slots.groupHeading ? childrenWithoutSlots : props.children}
166-
</ul>
167-
</GroupContext.Provider>
168-
</li>
169-
)
170-
}
171114
return (
172-
<Box
115+
<BoxWithFallback
173116
as="li"
117+
className={clsx(className, groupClasses.Group)}
174118
role={listRole ? 'none' : undefined}
175-
sx={{
176-
'&:not(:first-child)': {marginTop: 2},
177-
listStyle: 'none', // hide the ::marker inserted by browser's stylesheet
178-
...sx,
179-
}}
180-
className={className}
119+
sx={sx}
181120
{...props}
182121
>
183122
<GroupContext.Provider value={{selectionVariant, groupHeadingId}}>
@@ -187,20 +126,19 @@ export const Group: React.FC<React.PropsWithChildren<ActionListGroupProps>> = ({
187126
) : null}
188127
{/* Supports new API ActionList.GroupHeading */}
189128
{!title && slots.groupHeading ? React.cloneElement(slots.groupHeading) : null}
190-
<Box
191-
as="ul"
192-
sx={{paddingInlineStart: 0}}
129+
<ul
193130
// if listRole is set (listbox or menu), we don't label the list with the groupHeadingId
194131
// because the heading is hidden from the accessibility tree and only used for presentation role.
195132
// We will instead use aria-label to label the list. See a line below.
196133
aria-labelledby={listRole ? undefined : groupHeadingId}
197134
aria-label={listRole ? (title ?? (slots.groupHeading?.props.children as string)) : undefined}
198135
role={role || (listRole && 'group')}
136+
className={groupClasses.GroupList}
199137
>
200138
{slots.groupHeading ? childrenWithoutSlots : props.children}
201-
</Box>
139+
</ul>
202140
</GroupContext.Provider>
203-
</Box>
141+
</BoxWithFallback>
204142
)
205143
}
206144

@@ -278,26 +216,15 @@ export const GroupHeading: React.FC<React.PropsWithChildren<ActionListGroupHeadi
278216
as={headingWrapElement}
279217
data-component="GroupHeadingWrap"
280218
>
281-
{sx !== defaultSxProp ? (
282-
<Heading
283-
className={clsx(className, groupClasses.GroupHeading)}
284-
as={as || 'h3'}
285-
id={groupHeadingId}
286-
sx={sx}
287-
{...props}
288-
>
289-
{_internalBackwardCompatibleTitle ?? children}
290-
</Heading>
291-
) : (
292-
<Heading
293-
className={clsx(className, groupClasses.GroupHeading)}
294-
as={as || 'h3'}
295-
id={groupHeadingId}
296-
{...props}
297-
>
298-
{_internalBackwardCompatibleTitle ?? children}
299-
</Heading>
300-
)}
219+
<Heading
220+
className={clsx(className, groupClasses.GroupHeading)}
221+
as={as || 'h3'}
222+
id={groupHeadingId}
223+
sx={sx}
224+
{...props}
225+
>
226+
{_internalBackwardCompatibleTitle ?? children}
227+
</Heading>
301228
{auxiliaryText && <div className={classes.Description}>{auxiliaryText}</div>}
302229
</HeadingWrap>
303230
)}

0 commit comments

Comments
 (0)