From b413da5c8197bb96289edf0503fd0303c39a8426 Mon Sep 17 00:00:00 2001 From: nickofthyme Date: Thu, 14 Nov 2024 16:58:52 -0600 Subject: [PATCH] Revert "chore: deprecate `syncColors` option, remove related fn tests" This reverts commit 4c8cb6eab716f1dbcbf06f0c954ac03f26a55a36. --- .../common/content_management/constants.ts | 3 - .../common/dashboard_container/types.ts | 3 - .../component/settings/settings_flyout.tsx | 50 ++++++ .../embeddable/dashboard_container.test.tsx | 1 + .../content_management/v3/cm_services.ts | 9 +- src/plugins/embeddable/common/types.ts | 4 +- .../embeddables/diff_embeddable_input.test.ts | 1 + .../embeddable/public/store/input_slice.ts | 3 - src/plugins/expressions/public/types/index.ts | 3 - .../embeddable/visualize_embeddable.tsx | 3 - .../services/dashboard/dashboard_settings.ts | 6 + .../public/embeddable/expression_wrapper.tsx | 3 - .../functional/apps/dashboard/group2/index.ts | 1 + .../apps/dashboard/group2/sync_colors.ts | 162 ++++++++++++++++++ 14 files changed, 223 insertions(+), 29 deletions(-) create mode 100644 x-pack/test/functional/apps/dashboard/group2/sync_colors.ts diff --git a/src/plugins/dashboard/common/content_management/constants.ts b/src/plugins/dashboard/common/content_management/constants.ts index e235cb43d6357..978271680af12 100644 --- a/src/plugins/dashboard/common/content_management/constants.ts +++ b/src/plugins/dashboard/common/content_management/constants.ts @@ -18,9 +18,6 @@ export const DEFAULT_PANEL_HEIGHT = 15; export const DEFAULT_DASHBOARD_OPTIONS = { hidePanelTitles: false, useMargins: true, - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ syncColors: true, syncCursor: true, syncTooltips: true, diff --git a/src/plugins/dashboard/common/dashboard_container/types.ts b/src/plugins/dashboard/common/dashboard_container/types.ts index cf8c6b02e550e..dd3f7302038c0 100644 --- a/src/plugins/dashboard/common/dashboard_container/types.ts +++ b/src/plugins/dashboard/common/dashboard_container/types.ts @@ -64,9 +64,6 @@ export interface DashboardContainerInput extends EmbeddableInput { hidePanelTitles: DashboardOptions['hidePanelTitles']; syncTooltips: DashboardOptions['syncTooltips']; useMargins: DashboardOptions['useMargins']; - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ syncColors: DashboardOptions['syncColors']; syncCursor: DashboardOptions['syncCursor']; diff --git a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx index 76617866ca3f1..20fd2b93119de 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/settings/settings_flyout.tsx @@ -22,7 +22,9 @@ import { EuiFlyoutHeader, EuiForm, EuiFormRow, + EuiIconTip, EuiSwitch, + EuiText, EuiTextArea, EuiTitle, } from '@elastic/eui'; @@ -265,6 +267,54 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => { )} > <> + + + {i18n.translate( + 'dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel', + { + defaultMessage: 'Sync color palettes across panels', + } + )}{' '} + + {i18n.translate('dashboard.palettes.defaultPaletteLabel', { + defaultMessage: 'Default', + })} + + ), + compatibility: ( + + {i18n.translate('dashboard.palettes.kibanaPaletteLabel', { + defaultMessage: 'Compatibility', + })} + + ), + }} + /> + } + iconProps={{ + className: 'eui-alignTop', + }} + position="top" + size="s" + type="questionInCircle" + /> + + } + checked={localSettings.syncColors} + onChange={(event) => updateDashboardSetting({ syncColors: event.target.checked })} + data-test-subj="dashboardSyncColorsCheckbox" + /> + { timeslice: [number, number]; }; expect(embeddableInput.syncTooltips).toBe(false); + expect(embeddableInput.syncColors).toBe(false); expect(embeddableInput.syncCursor).toBe(true); }); }); diff --git a/src/plugins/dashboard/server/content_management/v3/cm_services.ts b/src/plugins/dashboard/server/content_management/v3/cm_services.ts index 88f3599e3224b..e086d1cc1460a 100644 --- a/src/plugins/dashboard/server/content_management/v3/cm_services.ts +++ b/src/plugins/dashboard/server/content_management/v3/cm_services.ts @@ -309,16 +309,9 @@ export const optionsSchema = schema.object({ defaultValue: DEFAULT_DASHBOARD_OPTIONS.useMargins, meta: { description: 'Show margins between panels in the dashboard layout.' }, }), - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ syncColors: schema.boolean({ defaultValue: DEFAULT_DASHBOARD_OPTIONS.syncColors, - meta: { - deprecated: true, - description: - 'Previously used to synchronize legacy colors between related panels in the dashboard.', - }, + meta: { description: 'Synchronize colors between related panels in the dashboard.' }, }), syncTooltips: schema.boolean({ defaultValue: DEFAULT_DASHBOARD_OPTIONS.syncTooltips, diff --git a/src/plugins/embeddable/common/types.ts b/src/plugins/embeddable/common/types.ts index 40f38df545a8a..951ecd9026ded 100644 --- a/src/plugins/embeddable/common/types.ts +++ b/src/plugins/embeddable/common/types.ts @@ -57,9 +57,7 @@ export type EmbeddableInput = { searchSessionId?: string; /** - * Flag whether legacy colors should be synced with other panels - * - * @deprecated https://github.com/elastic/kibana/pull/197802 + * Flag whether colors should be synced with other panels */ syncColors?: boolean; diff --git a/src/plugins/embeddable/public/lib/embeddables/diff_embeddable_input.test.ts b/src/plugins/embeddable/public/lib/embeddables/diff_embeddable_input.test.ts index f59c808af0348..49f097f500a46 100644 --- a/src/plugins/embeddable/public/lib/embeddables/diff_embeddable_input.test.ts +++ b/src/plugins/embeddable/public/lib/embeddables/diff_embeddable_input.test.ts @@ -20,6 +20,7 @@ const getGenericEmbeddableState = (state?: Partial): Embeddable disabledActions: [], disableTriggers: false, enhancements: undefined, + syncColors: false, syncTooltips: false, syncCursor: true, viewMode: ViewMode.VIEW, diff --git a/src/plugins/embeddable/public/store/input_slice.ts b/src/plugins/embeddable/public/store/input_slice.ts index 6925cc85ae1c5..3b3cf8b8ee588 100644 --- a/src/plugins/embeddable/public/store/input_slice.ts +++ b/src/plugins/embeddable/public/store/input_slice.ts @@ -38,9 +38,6 @@ export const input = createSlice({ setSearchSessionId(state, action: PayloadAction) { state.searchSessionId = action.payload; }, - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ setSyncColors(state, action: PayloadAction) { state.syncColors = action.payload; }, diff --git a/src/plugins/expressions/public/types/index.ts b/src/plugins/expressions/public/types/index.ts index bac67b5309699..2eb881e0a2bdd 100644 --- a/src/plugins/expressions/public/types/index.ts +++ b/src/plugins/expressions/public/types/index.ts @@ -49,9 +49,6 @@ export interface IExpressionLoaderParams { onRenderError?: RenderErrorHandlerFnType; searchSessionId?: string; renderMode?: RenderMode; - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ syncColors?: boolean; syncCursor?: boolean; syncTooltips?: boolean; diff --git a/src/plugins/visualizations/public/legacy/embeddable/visualize_embeddable.tsx b/src/plugins/visualizations/public/legacy/embeddable/visualize_embeddable.tsx index 2155a2457b766..196753d73b28c 100644 --- a/src/plugins/visualizations/public/legacy/embeddable/visualize_embeddable.tsx +++ b/src/plugins/visualizations/public/legacy/embeddable/visualize_embeddable.tsx @@ -108,9 +108,6 @@ export class VisualizeEmbeddable private query?: Query; private filters?: Filter[]; private searchSessionId?: string; - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ private syncColors?: boolean; private syncTooltips?: boolean; private syncCursor?: boolean; diff --git a/test/functional/services/dashboard/dashboard_settings.ts b/test/functional/services/dashboard/dashboard_settings.ts index 60f23d9c02f41..b0c2f6d691b8e 100644 --- a/test/functional/services/dashboard/dashboard_settings.ts +++ b/test/functional/services/dashboard/dashboard_settings.ts @@ -77,6 +77,12 @@ export function DashboardSettingsProvider({ getService }: FtrProviderContext) { await testSubjects.setEuiSwitch('dashboardPanelTitlesCheckbox', status); } + public async toggleSyncColors(value: boolean) { + const status = value ? 'check' : 'uncheck'; + log.debug(`toggleSyncColors::${status}`); + await testSubjects.setEuiSwitch('dashboardSyncColorsCheckbox', status); + } + public async toggleSyncCursor(value: boolean) { const status = value ? 'check' : 'uncheck'; log.debug(`toggleSyncCursor::${status}`); diff --git a/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx b/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx index a53cb5a9475c9..d16df5bf9d1e8 100644 --- a/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx +++ b/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx @@ -33,9 +33,6 @@ export interface ExpressionWrapperProps { ) => void; onRender$: () => void; renderMode?: RenderMode; - /** - * @deprecated https://github.com/elastic/kibana/pull/197802 - **/ syncColors?: boolean; syncTooltips?: boolean; syncCursor?: boolean; diff --git a/x-pack/test/functional/apps/dashboard/group2/index.ts b/x-pack/test/functional/apps/dashboard/group2/index.ts index c2431a8e86c45..a233126f6e4a6 100644 --- a/x-pack/test/functional/apps/dashboard/group2/index.ts +++ b/x-pack/test/functional/apps/dashboard/group2/index.ts @@ -9,6 +9,7 @@ import { FtrProviderContext } from '../../../ftr_provider_context'; export default function ({ loadTestFile }: FtrProviderContext) { describe('dashboard', function () { + loadTestFile(require.resolve('./sync_colors')); loadTestFile(require.resolve('./_async_dashboard')); loadTestFile(require.resolve('./dashboard_lens_by_value')); loadTestFile(require.resolve('./dashboard_maps_by_value')); diff --git a/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts b/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts new file mode 100644 index 0000000000000..a89f35d917130 --- /dev/null +++ b/x-pack/test/functional/apps/dashboard/group2/sync_colors.ts @@ -0,0 +1,162 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { DebugState } from '@elastic/charts'; +import expect from '@kbn/expect'; +import chroma from 'chroma-js'; +import { FtrProviderContext } from '../../../ftr_provider_context'; + +export default function ({ getService, getPageObjects }: FtrProviderContext) { + const esArchiver = getService('esArchiver'); + const { dashboard, header, lens } = getPageObjects(['dashboard', 'header', 'lens']); + const dashboardAddPanel = getService('dashboardAddPanel'); + const dashboardSettings = getService('dashboardSettings'); + const filterBar = getService('filterBar'); + const elasticChart = getService('elasticChart'); + const kibanaServer = getService('kibanaServer'); + + function getColorMapping(debugState: DebugState | null) { + if (!debugState) return {}; + const colorMapping: Record = {}; + debugState.bars?.forEach(({ name, color }) => { + colorMapping[name] = color; + }); + + return colorMapping; + } + + describe('sync colors', function () { + before(async function () { + await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/logstash_functional'); + await kibanaServer.importExport.load( + 'x-pack/test/functional/fixtures/kbn_archiver/lens/lens_basic.json' + ); + }); + + after(async function () { + await esArchiver.unload('x-pack/test/functional/es_archives/logstash_functional'); + await kibanaServer.importExport.unload( + 'x-pack/test/functional/fixtures/kbn_archiver/lens/lens_basic.json' + ); + await kibanaServer.savedObjects.cleanStandardList(); + }); + + it('should sync colors on dashboard for legacy default palette', async function () { + await dashboard.navigateToApp(); + await elasticChart.setNewChartUiDebugFlag(true); + await dashboard.clickCreateDashboardPrompt(); + + // create non-filtered xy chart + await dashboardAddPanel.clickCreateNewLink(); + await lens.goToTimeRange(); + await lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'count', + field: 'Records', + }); + await lens.configureDimension({ + dimension: 'lnsXY_splitDimensionPanel > lns-empty-dimension', + operation: 'terms', + field: 'geo.src', + palette: { mode: 'legacy', id: 'default' }, + }); + await lens.saveAndReturn(); + await header.waitUntilLoadingHasFinished(); + + // create filtered xy chart + await dashboardAddPanel.clickCreateNewLink(); + await lens.configureDimension({ + dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension', + operation: 'count', + field: 'Records', + }); + await lens.configureDimension({ + dimension: 'lnsXY_splitDimensionPanel > lns-empty-dimension', + operation: 'terms', + field: 'geo.src', + palette: { mode: 'legacy', id: 'default' }, + }); + await filterBar.addFilter({ field: 'geo.src', operation: 'is not', value: 'CN' }); + await lens.saveAndReturn(); + await header.waitUntilLoadingHasFinished(); + + // create datatable vis + await dashboardAddPanel.clickCreateNewLink(); + await lens.switchToVisualization('lnsDatatable'); + await lens.configureDimension({ + dimension: 'lnsDatatable_rows > lns-empty-dimension', + operation: 'terms', + field: 'geo.src', + keepOpen: true, + }); + await lens.setTermsNumberOfValues(5); + await lens.setTableDynamicColoring('cell'); + await lens.setPalette('default', true); + await lens.closeDimensionEditor(); + await lens.configureDimension({ + dimension: 'lnsDatatable_metrics > lns-empty-dimension', + operation: 'count', + field: 'Records', + }); + await lens.saveAndReturn(); + + // Set dashboard to sync colors + await dashboard.openSettingsFlyout(); + await dashboardSettings.toggleSyncColors(true); + await dashboardSettings.clickApplyButton(); + await header.waitUntilLoadingHasFinished(); + await dashboard.waitForRenderComplete(); + + const colorMappings1 = Object.entries( + getColorMapping(await dashboard.getPanelChartDebugState(0)) + ); + const colorMappings2 = Object.entries( + getColorMapping(await dashboard.getPanelChartDebugState(1)) + ); + + const els = await lens.getDatatableCellsByColumn(0); + const colorMappings3 = await Promise.all( + els.map(async (el) => [ + await el.getVisibleText(), + chroma((await lens.getStylesFromCell(el))['background-color']).hex(), // eui converts hex to rgb + ]) + ); + + expect(colorMappings1).to.have.length(6); + expect(colorMappings2).to.have.length(6); + expect(colorMappings3).to.have.length(6); + + const mergedColorAssignments = new Map>(); + + [...colorMappings1, ...colorMappings2, ...colorMappings3].forEach(([key, color]) => { + if (!mergedColorAssignments.has(key)) mergedColorAssignments.set(key, new Set()); + mergedColorAssignments.get(key)?.add(color); + }); + + // Each key should have only been assigned one color across all 3 visualizations + mergedColorAssignments.forEach((colors, key) => { + expect(colors.size).eql( + 1, + `Key "${key}" was assigned multiple colors: ${JSON.stringify([...colors])}` + ); + }); + }); + + it('should be possible to disable color sync', async () => { + await dashboard.openSettingsFlyout(); + await dashboardSettings.toggleSyncColors(false); + await dashboardSettings.clickApplyButton(); + await header.waitUntilLoadingHasFinished(); + const colorMapping1 = getColorMapping(await dashboard.getPanelChartDebugState(0)); + const colorMapping2 = getColorMapping(await dashboard.getPanelChartDebugState(1)); + const colorsByOrder1 = Object.values(colorMapping1); + const colorsByOrder2 = Object.values(colorMapping2); + // colors by order of occurence have to be the same + expect(colorsByOrder1).to.eql(colorsByOrder2); + }); + }); +}