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
16 changes: 16 additions & 0 deletions .changeset/perf-actionlist-has-selector.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@primer/react': patch
---

perf(ActionList): Optimize CSS `:has()` selectors for better INP

**Performance improvements:**
- Replace `:has([aria-disabled], [disabled])` with `[data-disabled]` attribute on `<li>` - avoids descendant scan
- Replace `:has([data-loading])` with `[data-loading]` on `<li>` - avoids descendant scan
- Scope TrailingAction loading selector to direct child: `:has(> .TrailingAction[data-loading])`
- Add documentation comments explaining acceptable `:has()` patterns (mixed descriptions, SubGroup active state)

**Why this matters:**
CSS `:has()` selectors that scan descendants can cause expensive style recalculations during user interactions, impacting INP (Interaction to Next Paint). By using data attributes set via JS on the parent `<li>`, we eliminate descendant traversal.

Part of #7312
39 changes: 29 additions & 10 deletions packages/react/src/ActionList/ActionList.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@
display: none;
}

/* if a list has a mix of items with and without descriptions, reset the label font-weight to normal */
/*
* If a list has a mix of items with and without descriptions, reset the label font-weight.
* NOTE: Uses double descendant :has() - traverses list twice per recalc.
* This is acceptable because:
* 1. ActionLists are typically small (10-50 items)
* 2. Alternative would require JS to detect mixed state and set data attribute
* 3. The visual impact is subtle (font-weight change), not critical styling
*/
&:has([data-has-description='true']):has([data-has-description='false']) {
& .ItemLabel {
font-weight: var(--base-text-weight-normal);
Expand Down Expand Up @@ -121,7 +128,8 @@
}
}

&:not(:has([aria-disabled], [disabled]), [data-has-subitem='true']) {
/* PERFORMANCE: Use data-disabled on <li> instead of :has([aria-disabled], [disabled]) which scans descendants */
&:not([aria-disabled], [data-disabled='true'], [data-has-subitem='true']) {
@media (hover: hover) {
&:hover,
&:active {
Expand Down Expand Up @@ -276,8 +284,8 @@
}
}

&:where([data-loading='true']),
&:has([data-loading='true']) {
/* PERFORMANCE: data-loading is set on the <li> by JS, avoiding :has() descendant scan */
&:where([data-loading='true']) {
& * {
color: var(--fgColor-muted);
}
Expand Down Expand Up @@ -322,10 +330,9 @@
}
}

/* disabled */

/* PERFORMANCE: data-disabled is set on the <li> by JS, avoiding :has() descendant scan */
&[aria-disabled='true'],
&:has([aria-disabled='true'], [disabled]) {
&[data-disabled='true'] {
& .ActionListContent * {
color: var(--control-fgColor-disabled);
}
Expand Down Expand Up @@ -366,7 +373,8 @@
}

/* When TrailingAction is in loading state, keep labels and descriptions accessible */
&:has(.TrailingAction [data-loading='true']):not([aria-disabled='true']) {
/* PERFORMANCE: scoped to direct child TrailingAction */
&:has(> .TrailingAction[data-loading='true']):not([aria-disabled='true']) {
/* Ensure labels and descriptions maintain accessibility contrast */
& .ItemLabel {
color: var(--fgColor-default);
Expand Down Expand Up @@ -540,7 +548,11 @@
display: none;
}

/* show active indicator on parent collapse if child is active */
/*
* Show active indicator on parent collapse if child is active.
* NOTE: Uses adjacent sibling + descendant :has() - SubGroup is typically small (few nav items).
* This is acceptable because scope is limited to the collapsed SubGroup's children.
*/
&:has(+ .SubGroup [data-active='true']) {
background: var(--control-transparent-bgColor-selected);

Expand Down Expand Up @@ -637,6 +649,7 @@ default block */
word-break: normal;
}

/* NOTE: Uses descendant :has() - scope is just this item's children (label + description). Acceptable. */
&:has([data-truncate='true']) {
& .ItemLabel {
flex: 1 0 auto;
Expand Down Expand Up @@ -713,7 +726,8 @@ span wrapping svg or text */
height: 100%;

/* Preserve width consistency when loading state is active for text buttons only */
&[data-loading='true']:has([data-component='buttonContent']) {
/* PERFORMANCE: scoped to direct child for O(1) lookup */
&[data-loading='true']:has(> [data-component='buttonContent']) {
/* Double the left padding to compensate for missing right padding */
padding: 0 0 0 calc(var(--base-size-12) * 2);

Expand All @@ -731,6 +745,11 @@ span wrapping svg or text */
}
}

/*
* NOTE: Uses descendant :has() - VisualComponent is nested inside Tooltip > button,
* which is 3 levels deep (> * > * > .TrailingVisual). This is acceptable since
* the search is scoped to this small wrapper element's subtree.
*/
.InactiveButtonWrap {
&:has(.TrailingVisual) {
grid-area: trailingVisual;
Expand Down
6 changes: 5 additions & 1 deletion packages/react/src/ActionList/Group.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@
&:not(:first-child) {
margin-block-start: var(--base-size-8);

/* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading */
/*
* If somebody tries to pass the `title` prop AND a `NavList.GroupHeading` as a child, hide the `ActionList.GroupHeading`.
* NOTE: Uses descendant :has() - this is an edge case handler for developer errors.
* Scope is limited to group's children, not entire list. Acceptable performance cost.
*/
/* stylelint-disable-next-line selector-max-specificity */
&:has(.GroupHeadingWrap + ul > .GroupHeadingWrap) {
/* stylelint-disable-next-line selector-max-specificity */
Expand Down
2 changes: 2 additions & 0 deletions packages/react/src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,8 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
data-variant={variant === 'danger' ? variant : undefined}
data-active={active ? true : undefined}
data-inactive={inactiveText ? true : undefined}
data-loading={loading && !inactive ? true : undefined}
data-disabled={disabled ? true : undefined}
data-has-subitem={slots.subItem ? true : undefined}
data-has-description={slots.description ? true : false}
className={clsx(classes.ActionListItem, className)}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/ActionList/TrailingAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export type ActionListTrailingActionProps = ElementProps & {
export const TrailingAction = forwardRef(
({as = 'button', icon, label, href = null, className, style, loading, ...props}, forwardedRef) => {
return (
<span className={clsx(className, classes.TrailingAction)} style={style}>
<span className={clsx(className, classes.TrailingAction)} style={style} data-loading={loading ? true : undefined}>
{icon ? (
<IconButton
as={as}
Expand Down
Loading