Conversation
…nels The Pivot and Scatter settings panels hard-coded light backgrounds on their cards (#fff) and card headers (#fafafa), while the card titles inherit the theme's heading color. In dark mode this put near-white titles on a white background. The global .drag-tag field labels likewise stayed light. Use the existing theme variables instead: @component-background and @background-color-light for the cards, and @text-color, @background-color-light and @border-color-base for the field labels, with a dark override under html.dark. The label styles stay global because the CDK drag preview is appended to <body>. The light theme values are unchanged. Add a Playwright spec that checks both panels in both themes, and share the table paragraph fixture from the visualization page object.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR for?
In dark mode the Pivot and Scatter visualization settings panels were unreadable. Both components hard-coded light backgrounds on their cards (
#fff) and card headers (#fafafa), but the card titles take the theme's heading color. So in dark mode the titles were near-white text on a white background. The field labels (.drag-tag, defined inglobal.less) also stayed light.This PR replaces those values with the existing theme variables. It adds no new colors, and the light theme resolves to the same values as before.
#fff@component-background#fafafa@background-color-lightrgba(0,0,0,.65)/#fafafa/#d9d9d9@text-color/@background-color-light/@border-color-baseThe cards already sit inside
.themeMixin(...), so their fix is a one-line swap in each file. The field label styles are only used by these two panels, but they stay inglobal.less. The CDK drag preview is appended to<body>, outside the component host, so a component-scoped rule would not reach the label being dragged. Dark values are applied underhtml.dark .drag-tag { @import 'theme-dark'; ... }, the same patternantd-dark.lessuses.Computed styles measured in the browser, before and after (
/#/notebook/...with a%shtable paragraph, Pivot through the Bar Chart and Scatter through the Scatter Chart):rgb(255, 255, 255)rgb(31, 31, 31)rgb(250, 250, 250)rgb(38, 38, 38)rgba(255, 255, 255, 0.95)(on white)rgba(255, 255, 255, 0.95)rgb(250, 250, 250)/rgba(0, 0, 0, 0.65)rgb(38, 38, 38)/rgba(255, 255, 255, 0.85)<body>)rgb(38, 38, 38)/rgba(255, 255, 255, 0.85)The "before" row matches the values in the Jira report. In the light theme, every value above is identical before and after, including the drag preview. The compiled
.drag-tagrule is character-for-character the same as before.What type of PR is it?
Bug Fix
Todos
What is the Jira issue?
ZEPPELIN-6588
How should this be tested?
Done:
e2e/tests/theme/visualization-settings-theme.spec.ts: Pivot and Scatter, each in light and dark (4 tests). Each test checks every card, card header and card title, plus a field label, against the theme's resolved colors. The table paragraph fixture moved tonotebook-visualization-page.ts, so this spec andvisualization-rendering.spec.tsshare it.rgb(31, 31, 31), receivedrgb(255, 255, 255)), and the two light tests pass. With only theglobal.lesschange reverted, the dark tests get past the cards and fail on the field label (expectedrgb(38, 38, 38), receivedrgb(250, 250, 250)). So both assertions catch the defect.visualization-rendering.spec.tspass on chromium, firefox and webkit: 19 passed, no retries.npm run test:shell: 131 passed.npm run lint: exit 0, with the same warning count as master.Notes:
@NB-PARITY-060(theme). That scenario's action is changing the theme while a notebook is mounted, but this spec sets the theme in localStorage and reloads. Tagging it would also mean updating the registry coverage that ZEPPELIN-6640 tracks.Settingshows it as open. Each test here uses a fresh note, so it does not hit this.Screenshots (if appropriate)
N/A. The computed-style tables above record the before/after values.
Questions:
🤖 Generated with Claude Code