Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Optimize color-mode-theme mixin by removing redundant pseudo-element selectors #2957

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Copilot wants to merge 3 commits into main
base: main
Choose a base branch
Loading
from copilot/optimize-color-mode-mixin

Conversation

Copy link

Copilot AI commented Jan 13, 2026
edited by mattcosta7
Loading

What are you trying to accomplish?

fixes https://github.com/github/pull-requests/issues/21966

I profiled these as 2 of the most expensive rules on a PR page - leading to signficant time spent in recalculate styles

Time (ms) Match Count Affected Nodes Invalidations Hit % Selector
54.950 8,879 0 0 100 [data-color-mode="light"][data-light-theme*="dark"] ::selection
34.405 8,879 8,680 0 100 [data-color-mode="dark"][data-dark-theme*="dark"] ::selection
21.747 8,879 0 0 100 [data-color-mode="auto"][data-dark-theme*="dark"] ::selection
29.999 8,879 4,340 0 100 [data-color-mode="dark"][data-dark-theme="dark_dimmed"] ::backdrop
18.593 8,879 0 0 100 [data-color-mode][data-color-mode="auto"][data-dark-theme="dark_dimmed"] ::backdrop
159.694 44,395 13,020 0 (Totals)

The color-mode-theme mixin was generating 6+ expensive ::selection and ::backdrop selectors for each theme variant, with substring attribute matching (*="dark") and all CSS custom properties redundantly output. This resulted in ~110ms+ of selector evaluation cost.

Changes:

  • Removed ::selection and ::backdrop from the color-mode-theme mixin
  • Added single global ::selection and ::backdrop rules to src/base/base.scss that inherit theme variables from parent elements

Impact:

  • 6+ complex ::selection selectors → 1 simple global selector
  • 6+ complex ::backdrop selectors → 2 simple global selectors
  • ~110ms+ → <1ms selector evaluation cost
  • No attribute matching overhead on pseudo-elements

What approach did you choose and why?

CSS custom properties inherit through the cascade. Since theme variables are already defined on [data-color-mode] ancestors, pseudo-elements automatically inherit those values. Re-declaring all variables on pseudo-elements for every theme variant is redundant and expensive.

The global rules only include properties that actually affect pseudo-elements (color, background-color for ::selection; background-color for ::backdrop), not the entire variable set.

What should reviewers focus on?

Verify that text selection and backdrop styling works correctly across all themes (light, dark, dark_dimmed, dark_high_contrast) in the compiled CSS output.

Can these changes ship as is?

  • Yes, this PR does not depend on additional changes. 🚢
Original prompt

Problem

The current color-mode-theme mixin in src/support/mixins/color-modes.scss outputs ::selection and ::backdrop pseudo-element rules for every theme variant, creating multiple expensive selectors:

[data-color-mode="light"][data-light-theme*="dark"] ::selection { /* all theme variables */ }
[data-color-mode="dark"][data-dark-theme*="dark"] ::selection { /* all theme variables */ }
[data-color-mode="auto"][data-dark-theme*="dark"] ::selection { /* all theme variables */ }
// ... repeated for ::backdrop

Performance profiling shows this is extremely costly:

  • [data-color-mode="light"][data-light-theme*="dark"] ::selection54.95ms
  • [data-color-mode="dark"][data-dark-theme*="dark"] ::selection34.41ms
  • [data-color-mode="auto"][data-dark-theme*="dark"] ::selection21.75ms
  • [data-color-mode="dark"][data-dark-theme="dark_dimmed"] ::backdrop30.00ms
  • Plus several more ::backdrop selectors at 12-18ms each

Total: ~110ms+ of selector evaluation cost

The issues are:

  1. Substring attribute matching (*="dark") prevents browser fast-path optimizations
  2. Multiple complex compound attribute selectors for each theme
  3. All CSS custom properties are output to ::selection/::backdrop, but these pseudo-elements can only use a few properties (color, background-color, etc.)
  4. These document-scoped pseudo-elements are re-evaluated on every selection/compositing event

Solution

CSS custom properties inherit from the originating element. Since the theme variables are already defined on [data-color-mode] ancestors, ::selection and ::backdrop can inherit them automatically.

Changes Required

  1. Remove &::selection from the color-mode-theme mixin in src/support/mixins/color-modes.scss:

    • Remove the &, &::selection { @content; } pattern
    • Just output @content directly to the theme selectors
  2. Remove ::backdrop duplication from the mixin - same approach

  3. Add single global ::selection and ::backdrop rules (likely in src/base/base.scss or a new file):

    ::selection {
     color: var(--fgColor-onEmphasis, var(--color-fg-on-emphasis));
     background-color: var(--bgColor-accent-emphasis, var(--color-accent-emphasis));
    }
    ::backdrop {
     background-color: var(--overlay-backdrop-bgColor, var(--color-primer-canvas-backdrop));
    }

Expected Impact

Before After
6+ complex ::selection selectors 1 simple ::selection selector
6+ complex ::backdrop selectors 1 simple ::backdrop selector
~110ms+ total selector cost <1ms
Substring attribute matching No attribute matching
All variables output to pseudo-elements Only 2-3 relevant properties

Why This Works

When you select text inside an element that has --bgColor-accent-emphasis defined (via the [data-color-mode] ancestor), the ::selection pseudo-element inherits that value automatically. The current approach redundantly re-declares all variables on ::selection when they're already available via inheritance.

Files to Modify

  1. src/support/mixins/color-modes.scss - Remove ::selection and ::backdrop from the mixin
  2. src/base/base.scss (or new file) - Add single global ::selection and ::backdrop rules

Testing

  • Verify text selection styling works correctly across all themes (light, dark, dark_dimmed, dark_high_contrast)
  • Verify ::backdrop styling works for dialogs/modals across all themes
  • Verify no visual regressions in selection/backdrop appearance

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.

Copy link

changeset-bot bot commented Jan 13, 2026
edited
Loading

🦋 Changeset detected

Latest commit: 4e1b5a7

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

This PR includes changesets to release 1 package
Name Type
@primer/css 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

Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copilot AI changed the title (削除) [WIP] Optimize color mode theme mixin for better performance (削除ここまで) (追記) Optimize color-mode-theme mixin by removing redundant pseudo-element selectors (追記ここまで) Jan 13, 2026
Copilot AI requested a review from mattcosta7 January 13, 2026 14:27
Copy link

@copilot add a changeset

Co-authored-by: mattcosta7 <8616962+mattcosta7@users.noreply.github.com>
Copy link
Author

Copilot AI commented Jan 13, 2026

@copilot add a changeset

Added changeset in commit 4e1b5a7.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR optimizes the color-mode-theme mixin by removing redundant ::selection and ::backdrop pseudo-element selectors that were generating expensive CSS with substring attribute matching. The optimization moves these styles to simple global rules that inherit theme variables from parent elements, reducing selector evaluation cost from ~110ms+ to <1ms.

Changes:

  • Removed ::selection and ::backdrop selector generation from the color-mode-theme mixin
  • Added global ::selection and ::backdrop rules to src/base/base.scss that use inherited CSS custom properties
  • Added changeset documenting the performance optimization

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/support/mixins/color-modes.scss Simplified the color-mode-theme mixin by removing redundant pseudo-element selector generation, reducing complexity and improving performance
src/base/base.scss Added global ::selection and ::backdrop rules that inherit theme variables from parent elements
.changeset/optimize-color-mode-pseudo-elements.md Documented the optimization as a patch change with performance metrics

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Reviewers

Copilot code review Copilot Copilot left review comments

@mattcosta7 mattcosta7 Awaiting requested review from mattcosta7

@liuliu-dev liuliu-dev Awaiting requested review from liuliu-dev liuliu-dev is a code owner automatically assigned from primer/engineer-reviewers

@jonrohan jonrohan Awaiting requested review from jonrohan

At least 1 approving review is required to merge this pull request.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /