Skip to content

Commit

Permalink
Revert "chore: deprecate syncColors option, remove related fn tests"
Browse files Browse the repository at this point in the history
This reverts commit 4c8cb6e.
  • Loading branch information
nickofthyme committed Nov 14, 2024
1 parent 4c8cb6e commit b413da5
Show file tree
Hide file tree
Showing 14 changed files with 223 additions and 29 deletions.
3 changes: 0 additions & 3 deletions src/plugins/dashboard/common/content_management/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/dashboard/common/dashboard_container/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import {
EuiFlyoutHeader,
EuiForm,
EuiFormRow,
EuiIconTip,
EuiSwitch,
EuiText,
EuiTextArea,
EuiTitle,
} from '@elastic/eui';
Expand Down Expand Up @@ -265,6 +267,54 @@ export const DashboardSettings = ({ onClose }: DashboardSettingsProps) => {
)}
>
<>
<EuiFormRow>
<EuiSwitch
label={
<EuiText size="s">
{i18n.translate(
'dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchLabel',
{
defaultMessage: 'Sync color palettes across panels',
}
)}{' '}
<EuiIconTip
color="subdued"
content={
<FormattedMessage
id="dashboard.embeddableApi.showSettings.flyout.form.syncColorsBetweenPanelsSwitchHelp"
defaultMessage="Only valid for {default} and {compatibility} palettes"
values={{
default: (
<strong>
{i18n.translate('dashboard.palettes.defaultPaletteLabel', {
defaultMessage: 'Default',
})}
</strong>
),
compatibility: (
<strong>
{i18n.translate('dashboard.palettes.kibanaPaletteLabel', {
defaultMessage: 'Compatibility',
})}
</strong>
),
}}
/>
}
iconProps={{
className: 'eui-alignTop',
}}
position="top"
size="s"
type="questionInCircle"
/>
</EuiText>
}
checked={localSettings.syncColors}
onChange={(event) => updateDashboardSetting({ syncColors: event.target.checked })}
data-test-subj="dashboardSyncColorsCheckbox"
/>
</EuiFormRow>
<EuiFormRow>
<EuiSwitch
label={i18n.translate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ describe('getInheritedInput', () => {
timeslice: [number, number];
};
expect(embeddableInput.syncTooltips).toBe(false);
expect(embeddableInput.syncColors).toBe(false);
expect(embeddableInput.syncCursor).toBe(true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 1 addition & 3 deletions src/plugins/embeddable/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const getGenericEmbeddableState = (state?: Partial<EmbeddableInput>): Embeddable
disabledActions: [],
disableTriggers: false,
enhancements: undefined,
syncColors: false,
syncTooltips: false,
syncCursor: true,
viewMode: ViewMode.VIEW,
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/embeddable/public/store/input_slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ export const input = createSlice({
setSearchSessionId(state, action: PayloadAction<EmbeddableInput['searchSessionId']>) {
state.searchSessionId = action.payload;
},
/**
* @deprecated https://github.com/elastic/kibana/pull/197802
**/
setSyncColors(state, action: PayloadAction<EmbeddableInput['syncColors']>) {
state.syncColors = action.payload;
},
Expand Down
3 changes: 0 additions & 3 deletions src/plugins/expressions/public/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions test/functional/services/dashboard/dashboard_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down
3 changes: 0 additions & 3 deletions x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/functional/apps/dashboard/group2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
162 changes: 162 additions & 0 deletions x-pack/test/functional/apps/dashboard/group2/sync_colors.ts
Original file line number Diff line number Diff line change
@@ -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<string, string> = {};
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<string, Set<string>>();

[...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);
});
});
}

0 comments on commit b413da5

Please sign in to comment.