Uh oh!
There was an error while loading. Please reload this page.
perf(ActionList): replace :has() selectors with data attributes - #7894
Conversation
Hoists state that was being detected via :has() to JS-derived data attributes on the rendering components. Same DOM output otherwise; reduces style-recalculation cost on lists that render many items. Changes: Item now emits data-has-trailing-action and data-trailing-action-loading from its slot props; InactiveButtonWrap emits data-position from its existing position prop; the text-Button variant of TrailingActionButton emits data-has-label. Four :has() selectors removed from ActionList.module.css.
🦋 Changeset detectedLatest commit: 1e8f2f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
There was a problem hiding this comment.
Pull request overview
Replaces four :has(...) selectors in ActionList's CSS with JS-derived data attributes computed from props/slots at render time, to reduce per-item style-recalculation cost (especially impactful on Safari and large lists). This follows the same pattern recently applied to Button in #7893 and to ActionList.List's data-mixed-descriptions.
Changes:
- Add
data-has-trailing-actionanddata-trailing-action-loadingonActionList.Item, and replace the corresponding:has()selectors inActionList.module.css. - Add
data-positiononInactiveButtonWrap(from the existingpositionprop onVisualOrIndicator) anddata-has-label="true"on the text-Button branch ofTrailingAction, replacing the related:has()selectors. - Add a patch changeset documenting the perf-only internal change.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/ActionList/Item.tsx | Adds data-has-trailing-action and data-trailing-action-loading to the <li> root, derived from slots.trailingAction. |
| packages/react/src/ActionList/Visuals.tsx | Adds data-position to InactiveButtonWrap so CSS can target leading vs trailing without :has(). |
| packages/react/src/ActionList/TrailingAction.tsx | Adds data-has-label="true" on the text-Button branch so CSS can target it without :has([data-component='buttonContent']). |
| packages/react/src/ActionList/ActionList.module.css | Replaces four :has(...) selectors (and their stylelint-disable comments) with attribute selectors on the new data attributes. |
| .changeset/perf-actionlist-has-selectors.md | Patch changeset describing the internal perf change. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 1
Uh oh!
There was an error while loading. Please reload this page.
Asserts the data attributes the CSS now depends on: data-has-trailing-action and data-trailing-action-loading on Item (from the TrailingAction slot), data-position on the inactive-state visual wrap (leading vs trailing), and data-has-label on the text Button variant of TrailingActionButton (vs the IconButton variant).
…tion Addresses #7894 review feedback. The previous :has(> .TrailingAction) and :has(.TrailingAction [data-loading='true']) selectors only matched when the TrailingAction was actually rendered in the DOM, which is gated by !inactive && !loading && !menuContext. The slot-presence checks I introduced were broader, so an inactive/loading item or one inside an ActionMenu/SelectPanel/FilteredActionList that was passed a TrailingAction would now get display: flex and the loading label override even though no action element was rendered. Mirror the render gate on the data attributes so they're set only when the action actually renders. Adds tests for the inactive and loading branches; the menuContext branch can't be exercised in dev tests due to the existing invariant but is documented.
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/21833 |
Closes #
Replaces four
:has(...)selectors inActionListwith JS-derived data attributes computed from props/slots at render time. Same DOM output otherwise; reduces the per-Item style-recalculation cost.ActionList items render many times per page in real GitHub views (navigation, dropdowns, issue lists), so each
:has()selector evaluates many times on every DOM mutation inside the list. Hoisting the state to data attributes turns those into constant-time attribute matches.Replacements:
ActionListItem&:has(> .TrailingAction)&[data-has-trailing-action]ActionListItem&:has(.TrailingAction [data-loading='true']):not([data-is-disabled])&[data-trailing-action-loading]:not([data-is-disabled])InactiveButtonWrap&:has(.TrailingVisual)/&:has(.LeadingVisual)&[data-position='trailing']/&[data-position='leading']TrailingActionButton&[data-loading='true']:has([data-component='buttonContent'])&[data-loading='true'][data-has-label='true']The new attributes are derived from state the components already track:
data-has-trailing-action←slots.trailingActionpresencedata-trailing-action-loading←slots.trailingAction?.props.loadingdata-position← the existingpositionprop onVisualOrIndicatordata-has-label← the text-Button branch ofTrailingAction(vs theIconButtonbranch)Changelog
New
data-has-trailing-action,data-trailing-action-loadingonActionList.Itemdata-positionon the inactive-state visual wrapperdata-has-labelon the text-Button variant ofActionList.TrailingActionChanged
ActionListstyling no longer relies on:has()for the cases aboveRemoved
:has(...)selectors fromActionList.module.cssRollout strategy
Testing & Reviewing
No visual changes expected. Each new data attribute is set exactly when the equivalent
:has()selector previously matched.Existing
ActionListunit tests pass locally (138 tests across the Button + ActionList suites).Companion to #7893 (same pattern applied to Button). Independent — either can land first.
Merge checklist