Skip to content

[ZEPPELIN-6640] Theme the React remote from the host instead of the shell's CSS - #5420

Open
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6640
Open

[ZEPPELIN-6640] Theme the React remote from the host instead of the shell's CSS#5420
kimyenac wants to merge 1 commit into
apache:masterfrom
kimyenac:ZEPPELIN-6640

Conversation

@kimyenac

@kimyenac kimyenac commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

The React remote never learns which theme the shell is in. PublishedParagraph wraps its tree in antd's ConfigProvider but only sets token.fontFamily, so antd always builds its styles from the default (light) algorithm.

Dark mode still looks correct today, but not for a reason either side declares. ThemeService writes data-theme and a dark class onto the document root, and the shell's global ng-zorro-antd stylesheet targets the same .ant-* class names the remote's markup happens to use, so the shell's dark rules land on top of the remote's own light CSS-in-JS. In the published paragraph with ?react=true in dark mode, .ant-table computes to rgb(31, 31, 31). Disable every stylesheet except the remote's own injected style[data-css-hash] tags and the same element becomes rgb(255, 255, 255) on the dark page.

Anything the shell's CSS cannot reach stays light. The charts in TableVisualization are drawn on a canvas, and no chart config sets axis, grid or legend colors, so chart.js v4 defaults apply (#666 text, rgba(0, 0, 0, 0.1) grid). Against the dark page background (#141414) the tick and category labels sit at about 3.2:1, below the 4.5:1 WCAG AA threshold for text, and the grid lines are effectively invisible.

This gives the remote the theme as an input instead of letting it inherit one by accident. src/theme/hostTheme.ts reads the theme the shell already publishes on the document root and follows it while mounted, ZeppelinThemeProvider selects antd's dark or default algorithm from it and exposes the resolved value through context for code that draws outside antd, and chartTheme.ts sets the two chart.js globals that ticks, legend labels and grid lines resolve from.

The theme is read from the DOM rather than passed in through reactProps. The published paragraph still mounts through its own window.reactApp path, so a props based version would have to be wired into that path as well as the directive, and the two host files it would touch are the ones ZEPPELIN-6564 and ZEPPELIN-6565 are currently changing. Reading the attribute the shell already publishes for its own CSS covers both mount paths and needs no host change, and a theme toggle then re-renders the remote without going through change detection. If you would rather keep host state flowing in through props, having the provider take an explicit theme prop is a small follow-up.

Matching the shell pixel for pixel is not the goal. antd's dark container token is #141414 where ng-zorro's is #1f1f1f, and where the shell's global rules still win, they win.

What type of PR is it?

Bug Fix

Todos

None

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-6640

How should this be tested?

  • New vitest specs under projects/zeppelin-react/src/theme/: theme resolution from the document root and its fallbacks, live updates when the shell toggles the theme, antd algorithm selection asserted through theme.useToken(), and the chart.js defaults. The project's suite is green at 29 specs. Note that unit tests do not gate CI yet (ZEPPELIN-6566).
  • Manual check in both modes on the published paragraph with ?react=true, on a paragraph with a TABLE result. Switch to Bar Chart in dark mode: the axis labels, legend and grid lines are readable where they were not. The table view and light mode are unchanged.
  • To see the cause rather than the symptom, disable the host's stylesheets in devtools. On master the remote's table turns white on the dark page. Here it stays dark, and the rule the remote injects reads color: rgba(255, 255, 255, 0.85).
  • A production build of the remote (npm run build).

Screenshots (if appropriate)

before
1-before-chart-dark

after
2-after-chart-dark

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? Yes, projects/zeppelin-react/README.md gains the provider step in "Adding a new React module"

…hell's CSS

The remote wraps its tree in antd's ConfigProvider but never sets an
algorithm, so it always builds light styles. Dark mode looks right only
because the shell's global ng-zorro-antd rules target the same .ant-*
class names, which leaves anything the shell cannot reach in light. The
chart.js canvas keeps its 'apache#666' default text, about 3.2:1 against the
dark page background, and its grid lines are invisible.

Read the theme the shell already publishes on the document root, pick
antd's dark or default algorithm from it, and set the two chart.js
globals that ticks, legend labels and grid lines resolve from. No
Angular change is needed.

@voidmatcha voidmatcha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM 👍

Nothing imports PublishedParagraph or TableVisualization in a spec, so reverting both files to their pre-PR state still leaves all 29 tests passing.

Adding resolve.alias to vitest.config.ts would make PublishedParagraph importable from a spec, and you could then assert that it renders with dark tokens under data-theme="dark". #5391 adds the same alias, but the content is identical so the conflict is trivial, and since there is no telling when that one gets reviewed and merged, applying it here first seems reasonable.

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.

2 participants