Skip to content

fix performance regression caused by modern UI styles - #325985

Merged
Benjamin Christopher Simmonds (benibenj) merged 1 commit into
microsoft:mainfrom
Kaidesuyoo:fix/modern-ui-css-performance-regression
Jul 24, 2026
Merged

Benjamin Christopher Simmonds (benibenj) merged 1 commit into
microsoft:mainfrom
Kaidesuyoo:fix/modern-ui-css-performance-regression

Conversation

@Kaidesuyoo

@Kaidesuyoo Kaidesuyo (Kaidesuyoo) commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #325984

This issue is caused by the performance overhead of the :has() selector and is similar to the issue discussed in #324986 .

The regression was introduced by #325013 , which added these selectors for the modern UI styles.

Additional note: The :has() selector on .pane-header is intentionally left unchanged. Compared with the removed selector, it has a much narrower scope and does not cause the same level of style invalidation overhead, so keeping it should be reasonable.

For more details about the :has() pseudo-class, refer to the MDN documentation.

Copilot AI review requested due to automatic review settings July 15, 2026 15:44
@vs-code-engineering

Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

Benjamin Christopher Simmonds (@benibenj)

Matched files:

  • src/vs/workbench/browser/parts/paneCompositePart.ts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Replaces an expensive Explorer-wide :has() selector while preserving title casing.

Changes:

  • Exposes the active pane composite ID as a DOM data attribute.
  • Uses that attribute to target Explorer titles efficiently.

Reviewed changes

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

File Description
paneCompositePart.ts Maintains the active composite data attribute.
fontRamp.css Replaces the costly selector with an attribute selector.

@mrleemurray Lee Murray (mrleemurray) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the approach and verified both the specificity math and the DOM wiring — this is a sound, minimal, behavior-preserving fix. Approving in spirit; a couple of optional notes below.

Correctness

  • this.element in AbstractPaneCompositePart is the part container that receives the .part class (part.classList.add('part', ...) in workbench.ts), so dataset.activeComposite → data-active-composite lands exactly on the .part:not(.editor)[...] element the selector targets. ✔
  • The set/delete pair is symmetric under the existing activePaneContextKey.get() === id guard, and each part instance manages its own element, so there's no cross-part interference. ✔

Specificity is preserved — the subtle bit the original :has() comment flagged. The rule must out-specify the shared capitalize ramp rule (… .part:not(.editor) > .title > .title-label h2, specificity (0,6,1)):

  • Old :has() selector: (0,9,1)
  • New attribute selector: (0,7,1) — the [data-active-composite="…"] still contributes a class-level weight

(0,7,1) beats (0,6,1), and text-transform: none also comes later in source order, so the Explorer title-casing behavior is fully maintained. ✔

Optional note: the attribute is written for every composite (search, scm, etc.) while the CSS only keys on the explorer value — that's fine and keeps the attribute reusable.

*/
.style-override.monaco-workbench .part:not(.editor):has(> .content > .composite.explorer-viewlet) > .title > .title-label h2,
.style-override.monaco-workbench .part:not(.editor)[data-active-composite="workbench.view.explorer"] > .title > .title-label h2,
.style-override .monaco-pane-view .pane > .pane-header:has(> .icon.codicon-explorer-view-icon) > .title {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This sibling :has() selector is left unchanged. It's far cheaper than the removed one (scoped to .pane-header elements rather than re-evaluating on every workbench subtree mutation), so leaving it is reasonable. Since the linked issue title is generic ("Modern UI can impact UI performance"), could you add a one-line note in the PR description that this pane-view variant was intentionally kept? Optional, non-blocking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@benibenj
Benjamin Christopher Simmonds (benibenj) merged commit 45fb8c7 into microsoft:main Jul 24, 2026
29 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.131.0 milestone Jul 24, 2026
@vs-code-engineering vs-code-engineering Bot locked and limited conversation to collaborators Sep 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Modern UI can impact UI performance

6 participants