This is an automated email from the ASF dual-hosted git repository.
tbonelee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new 4e49e3c7f2 [ZEPPELIN-6588] Use theme colors in the pivot and scatter
settings panels
4e49e3c7f2 is described below
commit 4e49e3c7f262ddb5e82081160d49355d6e93f88b
Author: κΉμλ <[email protected]>
AuthorDate: Sun Sep 27 17:27:21 2026 +0900
[ZEPPELIN-6588] Use theme colors in the pivot and scatter settings panels
### 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 in `global.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.
| Element | Before | After |
|---|---|---|
| Card background | `#fff` | `<at>component-background` |
| Card header background | `#fafafa` | `<at>background-color-light` |
| Field label text / background / border | `rgba(0,0,0,.65)` / `#fafafa` /
`#d9d9d9` | `<at>text-color` / `<at>background-color-light` /
`<at>border-color-base` |
The 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 in `global.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 under `html.dark .drag-tag {
<at>import 'theme-dark'; ... }`, the same pattern `antd-dark.less` uses.
Computed styles measured in the browser, before and after
(`/#/notebook/...` with a `%sh` table paragraph, Pivot through the Bar Chart
and Scatter through the Scatter Chart):
| Dark theme | Before | After |
|---|---|---|
| Card | `rgb(255, 255, 255)` | `rgb(31, 31, 31)` |
| Card header | `rgb(250, 250, 250)` | `rgb(38, 38, 38)` |
| Card title | `rgba(255, 255, 255, 0.95)` (on white) | `rgba(255, 255,
255, 0.95)` |
| Field label bg / text | `rgb(250, 250, 250)` / `rgba(0, 0, 0, 0.65)` |
`rgb(38, 38, 38)` / `rgba(255, 255, 255, 0.85)` |
| Drag preview (child of `<body>`) | same as field label, light | `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-tag` rule is character-for-character the same as before.
### What type of PR is it?
Bug Fix
### Todos
* [x] Use theme variables for the Pivot and Scatter settings card and
header backgrounds
* [x] Theme the field labels, including the drag preview
* [x] Add Playwright coverage for both panels in both themes
### What is the Jira issue?
[ZEPPELIN-6588](https://issues.apache.org/jira/browse/ZEPPELIN-6588)
### How should this be tested?
Done:
* New spec `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 to `notebook-visualization-page.ts`,
so this spec and `visualization-rendering.spec.ts` share it.
* Ran the new spec against the unfixed styles. The two dark tests fail on
the card background (expected `rgb(31, 31, 31)`, received `rgb(255, 255,
255)`), and the two light tests pass. With only the `global.less` change
reverted, the dark tests get past the cards and fail on the field label
(expected `rgb(38, 38, 38)`, received `rgb(250, 250, 250)`). So both assertions
catch the defect.
* With the fix, the new spec plus `visualization-rendering.spec.ts` pass 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.
* Manual check on a local server in both themes: card titles and field
labels are readable, dragging a field between Keys/Groups/Values still works,
and the light theme looks unchanged.
Notes:
* Not tagged with `<at>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.
* Drag-and-drop was checked by hand but has no new automated coverage.
Behavior coverage for these panels is ZEPPELIN-6514.
* Unrelated existing behavior found while testing, present on master too:
reload a note whose saved mode is Scatter, switch to Bar and back to Scatter,
and the Scatter settings panel stays hidden while `Setting` shows 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:
* Does the license files need to update? No. The new spec has the ASF
header.
* Is there breaking changes for older versions? No. Light theme output is
unchanged.
* Does this needs documentation? No.
π€ Generated with [Claude Code](https://claude.com/claude-code)
Closes #5504 from kimyenac/ZEPPELIN-6588.
Signed-off-by: ChanHo Lee <[email protected]>
---
.../e2e/models/notebook-visualization-page.ts | 29 +++++
.../paragraph/visualization-rendering.spec.ts | 4 +-
.../theme/visualization-settings-theme.spec.ts | 137 +++++++++++++++++++++
.../pivot-setting/pivot-setting.component.less | 4 +-
.../scatter-setting/scatter-setting.component.less | 4 +-
zeppelin-web-angular/src/styles/global.less | 15 ++-
6 files changed, 183 insertions(+), 10 deletions(-)
diff --git a/zeppelin-web-angular/e2e/models/notebook-visualization-page.ts
b/zeppelin-web-angular/e2e/models/notebook-visualization-page.ts
index a223398462..6d7c6ec585 100644
--- a/zeppelin-web-angular/e2e/models/notebook-visualization-page.ts
+++ b/zeppelin-web-angular/e2e/models/notebook-visualization-page.ts
@@ -15,6 +15,9 @@
import { Locator, Page } from '@playwright/test';
import { BasePage } from './base-page';
+export const TABLE_PARAGRAPH = `%sh
+printf '%%table
city\\tsales\\tcost\\nSeoul\\t30\\t12\\nBusan\\t20\\t8\\nIncheon\\t10\\t5\\n'`;
+
export class NotebookVisualizationPage extends BasePage {
readonly tableMode: Locator;
readonly barChartMode: Locator;
@@ -30,6 +33,9 @@ export class NotebookVisualizationPage extends BasePage {
readonly lineChartCanvas: Locator;
readonly areaChartCanvas: Locator;
readonly scatterChartCanvas: Locator;
+ readonly settingTrigger: Locator;
+ readonly pivotSetting: Locator;
+ readonly scatterSetting: Locator;
private readonly resultDisplay: Locator;
constructor(page: Page) {
@@ -49,6 +55,29 @@ export class NotebookVisualizationPage extends BasePage {
this.lineChartCanvas =
this.resultDisplay.locator('zeppelin-line-chart-visualization canvas');
this.areaChartCanvas =
this.resultDisplay.locator('zeppelin-area-chart-visualization canvas');
this.scatterChartCanvas =
this.resultDisplay.locator('zeppelin-scatter-chart-visualization canvas');
+ this.settingTrigger = this.resultDisplay.getByText('Setting', { exact:
true });
+ this.pivotSetting =
this.resultDisplay.locator('zeppelin-visualization-pivot-setting');
+ this.scatterSetting =
this.resultDisplay.locator('zeppelin-visualization-scatter-setting');
+ }
+
+ modeRadio(mode: Locator): Locator {
+ return mode.locator('input[type="radio"]');
+ }
+
+ settingCards(setting: Locator): Locator {
+ return setting.locator('.ant-card');
+ }
+
+ settingCardHeads(setting: Locator): Locator {
+ return setting.locator('.ant-card-head');
+ }
+
+ settingCardTitles(setting: Locator): Locator {
+ return setting.locator('.ant-card-head-title');
+ }
+
+ settingFieldTags(setting: Locator): Locator {
+ return setting.locator('.drag-tag');
}
async renderedPixelCount(canvas: Locator): Promise<number> {
diff --git
a/zeppelin-web-angular/e2e/tests/notebook/paragraph/visualization-rendering.spec.ts
b/zeppelin-web-angular/e2e/tests/notebook/paragraph/visualization-rendering.spec.ts
index dd96106086..c76433d7b6 100644
---
a/zeppelin-web-angular/e2e/tests/notebook/paragraph/visualization-rendering.spec.ts
+++
b/zeppelin-web-angular/e2e/tests/notebook/paragraph/visualization-rendering.spec.ts
@@ -14,7 +14,7 @@
import { expect, Locator, test } from '@playwright/test';
import { NotebookParagraphPage } from 'e2e/models/notebook-paragraph-page';
-import { NotebookVisualizationPage } from
'e2e/models/notebook-visualization-page';
+import { NotebookVisualizationPage, TABLE_PARAGRAPH } from
'e2e/models/notebook-visualization-page';
import {
addPageAnnotation,
addPageAnnotationBeforeEach,
@@ -25,8 +25,6 @@ import {
waitForZeppelinReady
} from '../../../utils';
-const TABLE_PARAGRAPH = `%sh
-printf '%%table
city\\tsales\\tcost\\nSeoul\\t30\\t12\\nBusan\\t20\\t8\\nIncheon\\t10\\t5\\n'`;
const TABLE_HEADERS = ['city', 'sales', 'cost'];
const TABLE_CELLS = ['Seoul', '30', '12', 'Busan', '20', '8', 'Incheon', '10',
'5'];
diff --git
a/zeppelin-web-angular/e2e/tests/theme/visualization-settings-theme.spec.ts
b/zeppelin-web-angular/e2e/tests/theme/visualization-settings-theme.spec.ts
new file mode 100644
index 0000000000..bb0865f445
--- /dev/null
+++ b/zeppelin-web-angular/e2e/tests/theme/visualization-settings-theme.spec.ts
@@ -0,0 +1,137 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { expect, test } from '@playwright/test';
+import { DarkModePage } from 'e2e/models/dark-mode-page';
+import { NotebookParagraphPage } from 'e2e/models/notebook-paragraph-page';
+import { NotebookVisualizationPage, TABLE_PARAGRAPH } from
'e2e/models/notebook-visualization-page';
+import {
+ addPageAnnotationBeforeEach,
+ createTestNotebook,
+ PAGES,
+ performLoginIfRequired,
+ setParagraphText,
+ waitForZeppelinReady
+} from '../../utils';
+
+// Resolved values of the theme variables the panels use:
@component-background,
+// @background-color-light, @heading-color, @text-color and @border-color-base.
+const THEMES = [
+ {
+ theme: 'light',
+ card: 'rgb(255, 255, 255)',
+ head: 'rgb(250, 250, 250)',
+ title: 'rgba(0, 0, 0, 0.85)',
+ tagBackground: 'rgb(250, 250, 250)',
+ tagText: 'rgba(0, 0, 0, 0.65)',
+ tagBorder: 'rgb(217, 217, 217)'
+ },
+ {
+ theme: 'dark',
+ card: 'rgb(31, 31, 31)',
+ head: 'rgb(38, 38, 38)',
+ title: 'rgba(255, 255, 255, 0.95)',
+ tagBackground: 'rgb(38, 38, 38)',
+ tagText: 'rgba(255, 255, 255, 0.85)',
+ tagBorder: 'rgb(67, 67, 67)'
+ }
+] as const;
+
+const PANELS = [
+ {
+ name: 'Pivot',
+ page: PAGES.VISUALIZATIONS.COMMON.PIVOT_SETTING,
+ titles: ['Available Fields', 'Keys', 'Groups', 'Values'],
+ chartMode: (visualizationPage: NotebookVisualizationPage) =>
visualizationPage.barChartMode,
+ setting: (visualizationPage: NotebookVisualizationPage) =>
visualizationPage.pivotSetting
+ },
+ {
+ name: 'Scatter',
+ page: PAGES.VISUALIZATIONS.COMMON.SCATTER_SETTING,
+ titles: ['Available Fields', 'XAxis', 'YAxis', 'Group', 'Size'],
+ chartMode: (visualizationPage: NotebookVisualizationPage) =>
visualizationPage.scatterChartMode,
+ setting: (visualizationPage: NotebookVisualizationPage) =>
visualizationPage.scatterSetting
+ }
+];
+
+for (const panel of PANELS) {
+ test.describe(`${panel.name} Settings Theme`, () => {
+ addPageAnnotationBeforeEach(panel.page);
+
+ let darkModePage: DarkModePage;
+ let paragraphPage: NotebookParagraphPage;
+ let visualizationPage: NotebookVisualizationPage;
+
+ test.beforeEach(async ({ page }) => {
+ await test.step('Given a notebook paragraph with deterministic table
output', async () => {
+ await page.goto('/#/');
+ await waitForZeppelinReady(page);
+ await performLoginIfRequired(page);
+
+ const { noteId, paragraphId } = await createTestNotebook(page);
+ await setParagraphText(page, noteId, paragraphId, TABLE_PARAGRAPH);
+
+ darkModePage = new DarkModePage(page);
+ paragraphPage = new NotebookParagraphPage(page);
+ visualizationPage = new NotebookVisualizationPage(page);
+ await page.goto(`/#/notebook/${noteId}`);
+ await expect(paragraphPage.paragraphContainer).toBeVisible({ timeout:
30000 });
+
+ await paragraphPage.runParagraph();
+ await expect(visualizationPage.dataTable).toBeVisible({ timeout: 30000
});
+ });
+ });
+
+ for (const colors of THEMES) {
+ test(`uses the ${colors.theme} theme colors for card headers and field
labels`, async ({ page }) => {
+ const setting = panel.setting(visualizationPage);
+
+ await test.step(`Given the ${colors.theme} theme`, async () => {
+ await darkModePage.setThemeInLocalStorage(colors.theme);
+ await page.reload();
+ await waitForZeppelinReady(page);
+ await expect(darkModePage.rootElement).toHaveAttribute('data-theme',
colors.theme);
+ await expect(visualizationPage.dataTable).toBeVisible({ timeout:
30000 });
+ });
+
+ await test.step(`When opening the ${panel.name} settings`, async () =>
{
+ const chartMode = panel.chartMode(visualizationPage);
+ await expect(async () => {
+ await chartMode.click();
+ await expect(visualizationPage.modeRadio(chartMode)).toBeChecked({
timeout: 1000 });
+ }).toPass({ timeout: 10000 });
+ await visualizationPage.settingTrigger.click();
+ await expect(setting).toBeVisible();
+ });
+
+ await test.step('Then every card and its header use the theme colors',
async () => {
+ await
expect(visualizationPage.settingCardTitles(setting)).toHaveText(panel.titles);
+ for (let index = 0; index < panel.titles.length; index++) {
+ await
expect(visualizationPage.settingCards(setting).nth(index)).toHaveCSS('background-color',
colors.card);
+ await
expect(visualizationPage.settingCardHeads(setting).nth(index)).toHaveCSS(
+ 'background-color',
+ colors.head
+ );
+ await
expect(visualizationPage.settingCardTitles(setting).nth(index)).toHaveCSS('color',
colors.title);
+ }
+ });
+
+ await test.step('And the field labels use the theme colors', async ()
=> {
+ const fieldTag =
visualizationPage.settingFieldTags(setting).filter({ hasText: 'city' }).first();
+ await expect(fieldTag).toHaveCSS('background-color',
colors.tagBackground);
+ await expect(fieldTag).toHaveCSS('color', colors.tagText);
+ await expect(fieldTag).toHaveCSS('border-top-color',
colors.tagBorder);
+ });
+ });
+ }
+ });
+}
diff --git
a/zeppelin-web-angular/src/app/visualizations/common/pivot-setting/pivot-setting.component.less
b/zeppelin-web-angular/src/app/visualizations/common/pivot-setting/pivot-setting.component.less
index 9091c36775..bddd2b24a2 100644
---
a/zeppelin-web-angular/src/app/visualizations/common/pivot-setting/pivot-setting.component.less
+++
b/zeppelin-web-angular/src/app/visualizations/common/pivot-setting/pivot-setting.component.less
@@ -22,11 +22,11 @@
min-height: 23px;
}
nz-card {
- background: #fff;
+ background: @component-background;
::ng-deep {
.ant-card-head {
padding: 0 12px;
- background: #fafafa;
+ background: @background-color-light;
}
}
}
diff --git
a/zeppelin-web-angular/src/app/visualizations/common/scatter-setting/scatter-setting.component.less
b/zeppelin-web-angular/src/app/visualizations/common/scatter-setting/scatter-setting.component.less
index 35d9c4e389..368c813146 100644
---
a/zeppelin-web-angular/src/app/visualizations/common/scatter-setting/scatter-setting.component.less
+++
b/zeppelin-web-angular/src/app/visualizations/common/scatter-setting/scatter-setting.component.less
@@ -22,11 +22,11 @@
min-height: 23px;
}
nz-card {
- background: #fff;
+ background: @component-background;
::ng-deep {
.ant-card-head {
padding: 0 12px;
- background: #fafafa;
+ background: @background-color-light;
}
}
}
diff --git a/zeppelin-web-angular/src/styles/global.less
b/zeppelin-web-angular/src/styles/global.less
index b72478b783..1ad743597a 100644
--- a/zeppelin-web-angular/src/styles/global.less
+++ b/zeppelin-web-angular/src/styles/global.less
@@ -53,9 +53,11 @@
opacity: 0.5;
}
+// Global rather than per component: the CDK drag preview is appended to
<body>.
.drag-tag {
+ @import 'theme-light';
box-sizing: border-box;
- color: rgba(0, 0, 0, 0.65);
+ color: @text-color;
font-variant: tabular-nums;
list-style: none;
font-feature-settings: 'tnum';
@@ -66,8 +68,8 @@
font-size: 12px;
line-height: 20px;
white-space: nowrap;
- background: #fafafa;
- border: 1px solid #d9d9d9;
+ background: @background-color-light;
+ border: 1px solid @border-color-base;
border-radius: 0px;
cursor: pointer;
opacity: 1;
@@ -81,6 +83,13 @@
}
}
+html.dark .drag-tag {
+ @import 'theme-dark';
+ color: @text-color;
+ background: @background-color-light;
+ border-color: @border-color-base;
+}
+
.interpreter-box {
margin-bottom: 12px;
line-height: 32px;
