Skip to content

Fix modebar scheme to follow nearest .light/.dark ancestor#218

Open
masenf wants to merge 1 commit into
mainfrom
claude/modebar-nested-container-styling-zsovt5
Open

Fix modebar scheme to follow nearest .light/.dark ancestor#218
masenf wants to merge 1 commit into
mainfrom
claude/modebar-nested-container-styling-zsovt5

Conversation

@masenf

@masenf masenf commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

The modebar and badge theming now correctly resolves from the nearest .light or .dark ancestor class, rather than darkening on any .dark ancestor regardless of proximity. This fixes a CSS specificity bug where a .light island nested inside a .dark page would incorrectly render with a dark toolbar.

Changes

  • CSS theming refactor (js/src/20_theme.ts):

    • Changed from descendant selector (.dark .xy) to direct selectors on scheme containers (:where(:root,:host,.light) for light; :where(.dark) for dark)
    • Moved --xy-* palette declarations onto the scheme-defining containers themselves rather than the chart, enabling proper CSS custom-property inheritance where the nearest declaration wins
    • Updated extensive comments explaining the inheritance model and the fix for the old descendant-selector bug (§36)
  • Test coverage (tests/test_ui_issue_regressions.py):

    • Added test_modebar_scheme_follows_nearest_container_class() to verify three scenarios:
      • Dark card on light page → dark toolbar
      • Light island in dark page → light toolbar
      • No scheme class → light toolbar (fallback)
  • Documentation updates:

    • spec/api/styling.md: Clarified that scheme resolution follows the nearest ancestor, with examples of nesting behavior
    • docs/styling/themes-and-tokens.md: Updated modebar description to emphasize nearest-ancestor resolution and composition semantics
    • tests/test_static_client_security.py: Added assertions to keep the spec contract explicit about nearest-ancestor behavior

Implementation Details

The fix leverages CSS custom-property inheritance semantics: by declaring --xy-* variables on the scheme containers (.dark, .light, :root, :host) rather than on the chart itself, the chart inherits from whichever container is closest in the DOM tree. This is more robust than selector specificity and composes naturally with nested containers.

https://claude.ai/code/session_01Sq7FQ5HLCppfwerutFcTd3

The internal --xy-* modebar and badge palette flipped to dark via the
descendant selector ".dark .xy", which matches whenever *any* ancestor
carries .dark and has no notion of .light as a stopping signal. A chart in
a nested container therefore could not follow the container it actually sits
in: a .light island inside a .dark page still rendered a dark toolbar,
contradicting the documented promise that an explicit .light keeps the light
palette.

Declare the scheme palette on the scheme-defining containers themselves
(.dark -> dark; .light, :root, :host -> light) and let it ride ordinary
custom-property inheritance, so a chart resolves the value from the nearest
ancestor carrying a scheme class and falls back to the document root (or
shadow host) otherwise. This is the cascade behavior the dossier (36) already
promises for per-container theming. Both nesting directions now resolve to the
closest class; public --chart-modebar-* / --chart-badge-* overrides are
unaffected.

Add a headless-Chromium regression asserting a dark card on a light page, a
light island in a dark page, and the no-class fallback, and update the styling
spec and themes docs.
@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 102 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing claude/modebar-nested-container-styling-zsovt5 (8702dbe) with main (ed7b809)

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR changes modebar and badge colors to inherit from the nearest light or dark container. The main changes are:

  • Moves internal palette variables onto scheme containers.
  • Adds browser tests for nested light and dark elements.
  • Documents nearest-ancestor scheme resolution.

Confidence Score: 4/5

The shadow-root theming path needs a fix before merging.

Normal document trees resolve nested light and dark containers as intended. A stylesheet inside a shadow root cannot select scheme classes on the host or outside ancestors. The unconditional :host rule therefore gives dark-hosted charts the light palette.

js/src/20_theme.ts; tests/test_ui_issue_regressions.py

T-Rex T-Rex Logs

What T-Rex did

  • The verification attempt explored a shadow-host that forces the light palette, but the shadow-root render did not complete because the harness failed to replace the real render target and timed out after the maximum execution steps.
  • The behavior reproduction showed the before state with a light island inside a dark page and dark rgba values, and the after state with a properly rendered chart, light island, and a visible modebar, as demonstrated by the standalone bundle posters and videos.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
js/src/20_theme.ts Moves palette variables onto scheme containers, but shadow-root charts cannot inherit dark state from the host or its ancestors.
tests/test_ui_issue_regressions.py Tests nested schemes in the document tree but does not cover the supported shadow-root path.
spec/api/styling.md Documents nearest-ancestor behavior, including behavior not currently preserved across a shadow boundary.
docs/styling/themes-and-tokens.md Updates user guidance for nearest-container theme resolution.
tests/test_static_client_security.py Updates static assertions to keep the revised styling contract explicit.

Reviews (1): Last reviewed commit: "Resolve modebar/badge scheme from the ne..." | Re-trigger Greptile

Comment thread js/src/20_theme.ts
Comment on lines +103 to +104
:where(:root,:host,.light){--xy-badge-text:#0f172a;--xy-badge-bg:rgba(255,255,255,.82);--xy-badge-shadow:0 1px 4px rgba(15,23,42,.14);--xy-modebar-bg:rgba(255,255,255,.78);--xy-modebar-menu-bg:rgba(255,255,255,.94);--xy-modebar-border:rgba(128,128,128,.18);--xy-modebar-menu-border:rgba(128,128,128,.22);--xy-modebar-active:rgba(128,128,128,.2);--xy-modebar-shadow:0 1px 4px rgba(0,0,0,.08);--xy-modebar-menu-shadow:0 5px 18px rgba(15,23,42,.18)}
:where(.dark){--xy-badge-text:#f8fafc;--xy-badge-bg:rgba(30,35,44,.88);--xy-badge-shadow:0 1px 4px rgba(0,0,0,.5);--xy-modebar-bg:rgba(37,42,52,.9);--xy-modebar-menu-bg:rgba(30,35,44,.97);--xy-modebar-border:rgba(255,255,255,.14);--xy-modebar-menu-border:rgba(255,255,255,.16);--xy-modebar-active:rgba(255,255,255,.16);--xy-modebar-shadow:0 1px 4px rgba(0,0,0,.5);--xy-modebar-menu-shadow:0 8px 24px rgba(0,0,0,.6)}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Shadow Host Forces Light Palette

When a chart is mounted in a shadow root, ensureChromeStylesheet() inserts these rules inside that root. There, :host always sets the light palette, while .dark cannot match a dark host or an ancestor outside the shadow tree, so charts under a dark page render a light modebar and badges instead of following the nearest scheme.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant