From 511c1707b9bd8f198dc16ac39d0e94f46706813a Mon Sep 17 00:00:00 2001 From: kimyenac Date: Sat, 26 Sep 2026 19:55:07 +0900 Subject: [PATCH] [ZEPPELIN-6588] Use theme colors in the pivot and scatter settings panels 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 . 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. --- .../e2e/models/notebook-visualization-page.ts | 29 ++++ .../paragraph/visualization-rendering.spec.ts | 4 +- .../visualization-settings-theme.spec.ts | 137 ++++++++++++++++++ .../pivot-setting.component.less | 4 +- .../scatter-setting.component.less | 4 +- zeppelin-web-angular/src/styles/global.less | 15 +- 6 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 zeppelin-web-angular/e2e/tests/theme/visualization-settings-theme.spec.ts diff --git a/zeppelin-web-angular/e2e/models/notebook-visualization-page.ts b/zeppelin-web-angular/e2e/models/notebook-visualization-page.ts index a223398462d9..6d7c6ec585e7 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 { 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 dd9610608612..c76433d7b6cc 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 000000000000..bb0865f445d4 --- /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 9091c367751a..bddd2b24a277 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 35d9c4e3892a..368c813146aa 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 b72478b7837f..1ad743597a5d 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 . .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;