Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 30, 2025

Replaces all :has() CSS selectors in Button component with data attribute lookups to eliminate descendant scans and improve INP performance. On pages with many buttons, :has() can trigger style recalculation across all buttons during interactions, not just the affected element.

Changes

ButtonBase.tsx

  • Added useLayoutEffect to detect [data-kbd-chord] descendants and set data-has-kbd attribute
  • Added prop-based data attributes: data-has-visuals, data-has-leading-visual, data-has-text

ButtonBase.module.css

  • Replaced &:has([data-kbd-chord])&[data-has-kbd] (medium/large button sizes)
  • Replaced &:where([data-has-count]):has([data-component='leadingVisual']):not(:has([data-component='text']))&:where([data-has-count][data-has-leading-visual]:not([data-has-text]))
  • Removed "acceptable :has()" documentation comments

Visual verification

Button with KeybindingHint correctly applies data-has-kbd and adjusts padding:

Button with keyboard hint showing data attributes

Changelog

Changed

  • Button: Replaced :has() selectors with data attributes for performance optimization

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

Verify keyboard shortcut hint buttons maintain correct padding (6px inline-end) in Storybook under Components/Button/Features/Keybinding Hint stories.

Merge checklist

Original prompt

Summary

Replace all :has() selectors in the Button component with data attribute lookups to eliminate descendant scans entirely and improve INP performance.

Background

We've observed :has() doing full document scans in related scenarios. On pages with many buttons, every style recalc can trigger :has() evaluation on all buttons, not just the one being interacted with. Data attribute lookups are O(1) and only affect the interacted element.

Changes needed

1. Replace :has(.Visual) with [data-has-visuals] (already in progress)

/* Before */
&:has(.Visual) { ... }
&:not(:has(.Visual)) { ... }

/* After */
&[data-has-visuals] { ... }
&:not([data-has-visuals]) { ... }

Set data-has-visuals on the button when leadingVisual or trailingVisual props are provided.

2. Replace :has([data-kbd-chord]) with [data-has-kbd]

/* Before */
&:has([data-kbd-chord]) {
  padding-inline-end: var(--base-size-6);
}

/* After */
&[data-has-kbd] {
  padding-inline-end: var(--base-size-6);
}

Set data-has-kbd on the button when a keyboard shortcut hint is rendered.

3. Replace the icon-only + counter selector

/* Before */
&:where([data-has-count]):has([data-component='leadingVisual']):not(:has([data-component='text'])) { ... }

/* After - use combination of data attributes */
&:where([data-has-count][data-has-leading-visual]:not([data-has-text])) { ... }

Set these data attributes on the button based on props:

  • data-has-leading-visual - when leadingVisual prop is provided
  • data-has-text - when children (text content) is provided

4. Update ButtonBase.tsx

Add the necessary data attributes to the button element based on props:

data-has-visuals={leadingVisual || trailingVisual ? true : undefined}
data-has-kbd={/* when kbd chord is present */ ? true : undefined}
data-has-leading-visual={leadingVisual ? true : undefined}
data-has-text={children ? true : undefined}

5. Remove the "acceptable :has()" comments

Since we're eliminating all :has() selectors, remove the documentation comments that explained why certain :has() usage was acceptable.

Files to modify

  • packages/react/src/Button/ButtonBase.module.css - Replace all :has() selectors
  • packages/react/src/Button/ButtonBase.tsx - Add data attributes based on props

Context

This is part of PR #7327 and the broader INP performance optimization effort tracked in #7312.

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@changeset-bot
Copy link

changeset-bot bot commented Dec 30, 2025

⚠️ No Changeset found

Latest commit: 6829c57

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copilot AI changed the title [WIP] Replace :has() selectors in Button component Replace :has() selectors with data attributes in Button component for INP optimization Dec 30, 2025
Copilot AI requested a review from mattcosta7 December 30, 2025 19:50
@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 Dec 30, 2025
@github-actions
Copy link
Contributor

👋 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. Or, apply the integration-tests: skipped manually label to skip these checks.

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.

2 participants