Skip to content

perf(ActionList): replace :has(~ .SubGroup [data-active]) collapsed-parent indicator with attribute selector#7902

Draft
mattcosta7 wants to merge 1 commit into
mainfrom
perf/actionlist-subgroup-active-indicator
Draft

perf(ActionList): replace :has(~ .SubGroup [data-active]) collapsed-parent indicator with attribute selector#7902
mattcosta7 wants to merge 1 commit into
mainfrom
perf/actionlist-subgroup-active-indicator

Conversation

@mattcosta7
Copy link
Copy Markdown
Contributor

Closes #

Replaces the most expensive :has() selector remaining in packages/react/src/ActionList/ActionList.module.css&:has(~ .SubGroup [data-active='true']) — with a plain attribute + child-combinator selector. This was the worst-shape :has() left in the package: it combined a general sibling combinator with a descendant search, and the engine had to re-run that walk against every collapsed parent any time a descendant data-active attribute changed.

Before:

.ActionListContent[aria-expanded='false']:has(~ .SubGroup [data-active='true']) {
  /* active-parent indicator styles */
}

After:

.ActionListItem[data-has-subitem='true'][data-active='true']
  > .ActionListContent[aria-expanded='false'] {
  /* active-parent indicator styles */
}

The selector goes from a sibling walk + descendant subtree search (re-evaluated on every data-active mutation inside the subtree) to a constant-time attribute lookup on the same <li> that the consumer already controls.

Why this is safe:

The new selector depends on data-active being set on the parent <li>. The only in-repo consumer that renders an ActionList.Item with ActionList.SubItem is NavList.ItemWithSubNav, which already does its own hasCurrentNavItem recursion and passes active={!isOpen && containsCurrentItem} to the parent — exactly matching the condition the old :has() was detecting. Net visual output is identical for NavList.SubNav usage.

Behavior change for direct ActionList.Item + ActionList.SubItem consumers

If a consumer renders ActionList.SubItem directly (without NavList.SubNav) and relied on the :has() selector to auto-light-up a collapsed parent when a descendant had data-active='true', they will now need to pass active={true} to the parent ActionList.Item themselves. This is the same pattern NavList.ItemWithSubNav already implements; no other in-repo usage exists.

Changelog

Changed

  • Internal: ActionList.module.css no longer uses :has() to detect an active descendant in a collapsed parent's sub-group; the same styles fire when the parent ActionList.Item itself has active={true} (which NavList.ItemWithSubNav already sets via its existing containsCurrentItem walk).

Removed

  • The &:has(~ .SubGroup [data-active='true']) selector from ActionList.module.css.

Rollout strategy

  • Patch release
  • Minor release
  • Major release; if selected, include a written rollout or migration plan
  • None; if selected, include a brief description as to why

Testing & Reviewing

  • All 121 packages/react/src/ActionList/** + packages/react/src/NavList/** unit tests pass.
  • tsc --noEmit on packages/react is clean.
  • Stylelint + Prettier + ESLint clean on touched files (only pre-existing -webkit-tap-highlight-color browser-compat warning at line 509, unrelated to this change).

VRT expectations: No change to NavList.WithSubItems / NavList.WithNestedSubItems / NavList.WithTrailingActionInSubItem renderings — the parent ActionList.Item already carries data-active='true' whenever the previous :has() would have matched.

Companion to #7901 (PageHeader) and #7894 (other ActionList :has() selectors).

Merge checklist

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 29, 2026

🦋 Changeset detected

Latest commit: 76fdb17

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions Bot added the integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm label May 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant