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?
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"] ::selection — 54.95ms
[data-color-mode="dark"][data-dark-theme*="dark"] ::selection — 34.41ms
[data-color-mode="auto"][data-dark-theme*="dark"] ::selection — 21.75ms
[data-color-mode="dark"][data-dark-theme="dark_dimmed"] ::backdrop — 30.00ms
- Plus several more
::backdrop selectors at 12-18ms each
Total: ~110ms+ of selector evaluation cost
The issues are:
- Substring attribute matching (
*="dark") prevents browser fast-path optimizations
- Multiple complex compound attribute selectors for each theme
- All CSS custom properties are output to
::selection/::backdrop, but these pseudo-elements can only use a few properties (color, background-color, etc.)
- 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
-
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
-
Remove ::backdrop duplication from the mixin - same approach
-
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
src/support/mixins/color-modes.scss - Remove ::selection and ::backdrop from the mixin
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.
Uh oh!
There was an error while loading. Please reload this page.
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
[data-color-mode="light"][data-light-theme*="dark"] ::selection[data-color-mode="dark"][data-dark-theme*="dark"] ::selection[data-color-mode="auto"][data-dark-theme*="dark"] ::selection[data-color-mode="dark"][data-dark-theme="dark_dimmed"] ::backdrop[data-color-mode][data-color-mode="auto"][data-dark-theme="dark_dimmed"] ::backdropThe
color-mode-thememixin was generating 6+ expensive::selectionand::backdropselectors 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:
::selectionand::backdropfrom thecolor-mode-thememixin::selectionand::backdroprules tosrc/base/base.scssthat inherit theme variables from parent elementsImpact:
::selectionselectors → 1 simple global selector::backdropselectors → 2 simple global selectorsWhat 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-colorfor::selection;background-colorfor::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?
Original prompt
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.