Uh oh!
There was an error while loading. Please reload this page.
perf(css): remove expensive :has() selectors for Safari style-recalc - #8115
Conversation
Replace Safari-costly :has() selectors with cheaper equivalents: - ActionList/NavList: :has(~ .SubGroup [data-active]) -> parent li's data-active (NavList already computes !isOpen && containsCurrentItem); preserves (0,4,0) specificity. Adds a VRT story for the collapsed-active parent state (needs a play fn since NavList auto-expands active subnavs). - Breadcrumbs: :has(.MenuOverlay) -> data-overflow-menu on the menu <li>. - SelectPanel: :has(input:placeholder-shown) -> input:placeholder-shown ~ .TextInput-action (siblings in the wrapper). - ActionList: remove dead :has([data-truncate]) (attribute never rendered). - SegmentedControl: remove no-op :focus-within:has(:focus-visible) (.Item has no background of its own). Button :has([data-kbd-chord]) intentionally kept (no clean React signal; static anchor = low cost).
🦋 Changeset detectedLatest commit: a223203 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
This PR reduces Safari/WebKit style-recalculation cost by removing or replacing several :has() selectors in @primer/react component styles, shifting to cheaper selectors (e.g., data attributes, sibling selectors) and deleting dead/no-op rules. It also adds a dev Storybook interaction to capture the only behavior-affecting case in visual regression.
Changes:
- Replace costly
:has()-based styling in ActionList/NavList, Breadcrumbs, and SelectPanel2 with data-attribute and sibling-selector approaches. - Remove dead/no-op
:has()rules from ActionList and SegmentedControl. - Add a NavList dev Storybook story + play function to exercise collapsed-parent “current item” styling for VRT.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/SegmentedControl/SegmentedControl.module.css | Removes a no-op :has() focus rule to avoid unnecessary selector work. |
| packages/react/src/NavList/NavList.dev.stories.tsx | Adds an interaction story to capture collapsed-parent active indicator behavior in VRT. |
| packages/react/src/experimental/SelectPanel2/SelectPanel.module.css | Replaces :has() with an input sibling selector to hide the clear action when empty. |
| packages/react/src/Breadcrumbs/Breadcrumbs.tsx | Marks the overflow menu item with data-overflow-menu for cheaper CSS targeting. |
| packages/react/src/Breadcrumbs/Breadcrumbs.module.css | Switches overflow-item wrapping rule from :has(.MenuOverlay) to [data-overflow-menu]. |
| packages/react/src/ActionList/ActionList.module.css | Keys collapsed-parent active styling off parent data-active and removes a dead :has([data-truncate]) rule. |
| .changeset/perf-remove-expensive-has-selectors.md | Adds a patch changeset for the Safari style-recalc performance improvements. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Low
Uh oh!
There was an error while loading. Please reload this page.
Integration test results from github/github-ui PR:
All checks passed! |
Closes #
Removes several Safari-costly
:has()selectors from the CSS, replacing them with cheaper equivalents (data attributes, a sibling selector) or removing dead/no-op rules.:has()can trigger quadratic style-invalidation in WebKit/Safari; this trims the most expensive remaining uses while keeping rendering behavior intact.Changelog
Changed
<li>'sdata-activeinstead of:has(~ .SubGroup [data-active='true']). NavList already computes this exact state (!isOpen && containsCurrentItem), and the original selector specificity(0,4,0)is preserved.data-overflow-menuin React, replacing:has(.MenuOverlay).input:placeholder-shown ~ .TextInput-action(a forward sibling selector) instead of:has(input:placeholder-shown).Removed
:has([data-truncate='true'])rule (thedata-truncateattribute is never rendered;Truncateemitsdata-inline), so it matched nothing.:focus-within:has(:focus-visible)rule (.Itemhas no background of its own, so it did nothing).New
CollapsedSubNavWithCurrentItem) with a play function that collapses an active sub-nav, so the collapsed-parent active state is captured for visual regression (it can't render statically because NavList auto-expands sub-navs containing the current item).Kept (intentionally)
:has([data-kbd-chord])is retained. The keybinding hint arrives as an arbitrarytrailingVisualrender function, so there's no clean React signal to replace it; a per-button layout-effectquerySelectorwould be worse (and conflicts with reducing querySelector-in-effects). Its anchor is fully static, so it doesn't trigger the dynamic quadratic re-invalidation:has()is known for.Are the styles identical?
inputand.TextInput-actionare the only such siblings in the wrapper; both target the same single element[data-overflow-menu]<li>that:has(.MenuOverlay)matched:has([data-truncate])data-truncatenever rendered):has(:focus-visible).Itemhas no background; the rule was a no-oparia-currentinternally — a small correctness improvement the old selector missed. Please confirm via VRT.Performance benchmarks
Chromium/Blink, 6× CPU throttle, style-recalc isolated via
getComputedStyle(no forced layout), min-of-trials. Blink has:has()invalidation sets that Safari lacks, so real-world Safari gains are larger than these.:has():has(~ .SubGroup [data-active]):has(.MenuOverlay):has(input:placeholder-shown):has([data-truncate]):has(:focus-visible)SelectPanel is roughly neutral per-keystroke in Blink (single input →
:has()already cheap); its clear win is under full restyle and, more importantly, in Safari where the keystroke-dynamic:has()is the pathological case.Rollout strategy
Testing & Reviewing
Components/NavList/Dev → Collapsed SubNav With Current Itemstory. The other four changes are provably byte-identical.Merge checklist