Skip to content

Commit a907c9b

Browse files
Wylie Conlonelasticmachine
andauthored
[Lens] Use suggestion system in chart switcher for subtypes (#64613)
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent 1106482 commit a907c9b

File tree

9 files changed

+143
-36
lines changed

9 files changed

+143
-36
lines changed

x-pack/plugins/lens/public/datatable_visualization/visualization.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ export const datatableVisualization: Visualization<
4141
},
4242
],
4343

44+
getVisualizationTypeId() {
45+
return 'lnsDatatable';
46+
},
47+
4448
getLayerIds(state) {
4549
return state.layers.map(l => l.layerId);
4650
},

x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/chart_switch.test.tsx

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,25 @@ describe('chart_switch', () => {
6262
id: 'subvisC2',
6363
label: 'C2',
6464
},
65+
{
66+
icon: 'empty',
67+
id: 'subvisC3',
68+
label: 'C3',
69+
},
6570
],
71+
getSuggestions: jest.fn(options => {
72+
if (options.subVisualizationId === 'subvisC2') {
73+
return [];
74+
}
75+
return [
76+
{
77+
score: 1,
78+
title: '',
79+
state: `suggestion`,
80+
previewIcon: 'empty',
81+
},
82+
];
83+
}),
6684
},
6785
};
6886
}
@@ -313,10 +331,11 @@ describe('chart_switch', () => {
313331
expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toBeUndefined();
314332
});
315333

316-
it('should not indicate data loss if visualization is not changed', () => {
334+
it('should not show a warning when the subvisualization is the same', () => {
317335
const dispatch = jest.fn();
318336
const frame = mockFrame(['a', 'b', 'c']);
319337
const visualizations = mockVisualizations();
338+
visualizations.visC.getVisualizationTypeId.mockReturnValue('subvisC2');
320339
const switchVisualizationType = jest.fn(() => 'therebedragons');
321340

322341
visualizations.visC.switchVisualizationType = switchVisualizationType;
@@ -333,10 +352,10 @@ describe('chart_switch', () => {
333352
/>
334353
);
335354

336-
expect(getMenuItem('subvisC2', component).prop('betaBadgeIconType')).toBeUndefined();
355+
expect(getMenuItem('subvisC2', component).prop('betaBadgeIconType')).not.toBeDefined();
337356
});
338357

339-
it('should remove all layers if there is no suggestion', () => {
358+
it('should get suggestions when switching subvisualization', () => {
340359
const dispatch = jest.fn();
341360
const visualizations = mockVisualizations();
342361
visualizations.visB.getSuggestions.mockReturnValueOnce([]);
@@ -377,7 +396,7 @@ describe('chart_switch', () => {
377396
const dispatch = jest.fn();
378397
const frame = mockFrame(['a', 'b', 'c']);
379398
const visualizations = mockVisualizations();
380-
const switchVisualizationType = jest.fn(() => 'therebedragons');
399+
const switchVisualizationType = jest.fn(() => 'switched');
381400

382401
visualizations.visC.switchVisualizationType = switchVisualizationType;
383402

@@ -393,12 +412,12 @@ describe('chart_switch', () => {
393412
/>
394413
);
395414

396-
switchTo('subvisC2', component);
397-
expect(switchVisualizationType).toHaveBeenCalledWith('subvisC2', 'therebegriffins');
415+
switchTo('subvisC3', component);
416+
expect(switchVisualizationType).toHaveBeenCalledWith('subvisC3', 'suggestion');
398417
expect(dispatch).toHaveBeenCalledWith(
399418
expect.objectContaining({
400419
type: 'SWITCH_VISUALIZATION',
401-
initialState: 'therebedragons',
420+
initialState: 'switched',
402421
})
403422
);
404423
expect(frame.removeLayers).not.toHaveBeenCalled();

x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/chart_switch.tsx

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,16 @@ export function ChartSwitch(props: Props) {
105105
const switchVisType =
106106
props.visualizationMap[visualizationId].switchVisualizationType ||
107107
((_type: string, initialState: unknown) => initialState);
108-
if (props.visualizationId === visualizationId) {
108+
const layers = Object.entries(props.framePublicAPI.datasourceLayers);
109+
const containsData = layers.some(
110+
([_layerId, datasource]) => datasource.getTableSpec().length > 0
111+
);
112+
// Always show the active visualization as a valid selection
113+
if (
114+
props.visualizationId === visualizationId &&
115+
props.visualizationState &&
116+
newVisualization.getVisualizationTypeId(props.visualizationState) === subVisualizationId
117+
) {
109118
return {
110119
visualizationId,
111120
subVisualizationId,
@@ -116,13 +125,13 @@ export function ChartSwitch(props: Props) {
116125
};
117126
}
118127

119-
const layers = Object.entries(props.framePublicAPI.datasourceLayers);
120-
const containsData = layers.some(
121-
([_layerId, datasource]) => datasource.getTableSpec().length > 0
128+
const topSuggestion = getTopSuggestion(
129+
props,
130+
visualizationId,
131+
newVisualization,
132+
subVisualizationId
122133
);
123134

124-
const topSuggestion = getTopSuggestion(props, visualizationId, newVisualization);
125-
126135
let dataLoss: VisualizationSelection['dataLoss'];
127136

128137
if (!containsData) {
@@ -250,14 +259,16 @@ export function ChartSwitch(props: Props) {
250259
function getTopSuggestion(
251260
props: Props,
252261
visualizationId: string,
253-
newVisualization: Visualization<unknown, unknown>
262+
newVisualization: Visualization<unknown, unknown>,
263+
subVisualizationId?: string
254264
): Suggestion | undefined {
255265
const suggestions = getSuggestions({
256266
datasourceMap: props.datasourceMap,
257267
datasourceStates: props.datasourceStates,
258268
visualizationMap: { [visualizationId]: newVisualization },
259269
activeVisualizationId: props.visualizationId,
260270
visualizationState: props.visualizationState,
271+
subVisualizationId,
261272
}).filter(suggestion => {
262273
// don't use extended versions of current data table on switching between visualizations
263274
// to avoid confusing the user.

x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_helpers.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export function getSuggestions({
4444
datasourceStates,
4545
visualizationMap,
4646
activeVisualizationId,
47+
subVisualizationId,
4748
visualizationState,
4849
field,
4950
}: {
@@ -57,6 +58,7 @@ export function getSuggestions({
5758
>;
5859
visualizationMap: Record<string, Visualization>;
5960
activeVisualizationId: string | null;
61+
subVisualizationId?: string;
6062
visualizationState: unknown;
6163
field?: unknown;
6264
}): Suggestion[] {
@@ -89,7 +91,8 @@ export function getSuggestions({
8991
table,
9092
visualizationId,
9193
datasourceSuggestion,
92-
currentVisualizationState
94+
currentVisualizationState,
95+
subVisualizationId
9396
);
9497
})
9598
)
@@ -108,13 +111,15 @@ function getVisualizationSuggestions(
108111
table: TableSuggestion,
109112
visualizationId: string,
110113
datasourceSuggestion: DatasourceSuggestion & { datasourceId: string },
111-
currentVisualizationState: unknown
114+
currentVisualizationState: unknown,
115+
subVisualizationId?: string
112116
) {
113117
return visualization
114118
.getSuggestions({
115119
table,
116120
state: currentVisualizationState,
117121
keptLayerIds: datasourceSuggestion.keptLayerIds,
122+
subVisualizationId,
118123
})
119124
.map(({ state, ...visualizationSuggestion }) => ({
120125
...visualizationSuggestion,

x-pack/plugins/lens/public/editor_frame_service/mocks.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export function createMockVisualization(): jest.Mocked<Visualization> {
2828
label: 'TEST',
2929
},
3030
],
31+
getVisualizationTypeId: jest.fn(_state => 'empty'),
3132
getDescription: jest.fn(_state => ({ label: '' })),
3233
switchVisualizationType: jest.fn((_, x) => x),
3334
getPersistableState: jest.fn(_state => _state),

x-pack/plugins/lens/public/metric_visualization/metric_visualization.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ export const metricVisualization: Visualization<State, PersistableState> = {
5353
},
5454
],
5555

56+
getVisualizationTypeId() {
57+
return 'lnsMetric';
58+
},
59+
5660
clearLayer(state) {
5761
return {
5862
...state,

x-pack/plugins/lens/public/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,10 @@ export interface SuggestionRequest<T = unknown> {
312312
* The visualization needs to know which table is being suggested
313313
*/
314314
keptLayerIds: string[];
315+
/**
316+
* Different suggestions can be generated for each subtype of the visualization
317+
*/
318+
subVisualizationId?: string;
315319
}
316320

317321
/**
@@ -388,6 +392,11 @@ export interface Visualization<T = unknown, P = unknown> {
388392
* but can register multiple subtypes
389393
*/
390394
visualizationTypes: VisualizationType[];
395+
/**
396+
* Return the ID of the current visualization. Used to highlight
397+
* the active subtype of the visualization.
398+
*/
399+
getVisualizationTypeId: (state: T) => string;
391400
/**
392401
* If the visualization has subtypes, update the subtype in state.
393402
*/

x-pack/plugins/lens/public/xy_visualization/xy_visualization.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ function exampleState(): State {
2727
}
2828

2929
describe('xy_visualization', () => {
30-
describe('getDescription', () => {
30+
describe('#getDescription', () => {
3131
function mixedState(...types: SeriesType[]) {
3232
const state = exampleState();
3333
return {
@@ -81,6 +81,45 @@ describe('xy_visualization', () => {
8181
});
8282
});
8383

84+
describe('#getVisualizationTypeId', () => {
85+
function mixedState(...types: SeriesType[]) {
86+
const state = exampleState();
87+
return {
88+
...state,
89+
layers: types.map((t, i) => ({
90+
...state.layers[0],
91+
layerId: `layer_${i}`,
92+
seriesType: t,
93+
})),
94+
};
95+
}
96+
97+
it('should show mixed when each layer is different', () => {
98+
expect(xyVisualization.getVisualizationTypeId(mixedState('bar', 'line'))).toEqual('mixed');
99+
});
100+
101+
it('should show the preferredSeriesType if there are no layers', () => {
102+
expect(xyVisualization.getVisualizationTypeId(mixedState())).toEqual('bar');
103+
});
104+
105+
it('should combine multiple layers into one type', () => {
106+
expect(
107+
xyVisualization.getVisualizationTypeId(mixedState('bar_horizontal', 'bar_horizontal'))
108+
).toEqual('bar_horizontal');
109+
});
110+
111+
it('should return the subtype for single layers', () => {
112+
expect(xyVisualization.getVisualizationTypeId(mixedState('area'))).toEqual('area');
113+
expect(xyVisualization.getVisualizationTypeId(mixedState('line'))).toEqual('line');
114+
expect(xyVisualization.getVisualizationTypeId(mixedState('area_stacked'))).toEqual(
115+
'area_stacked'
116+
);
117+
expect(xyVisualization.getVisualizationTypeId(mixedState('bar_horizontal_stacked'))).toEqual(
118+
'bar_horizontal_stacked'
119+
);
120+
});
121+
});
122+
84123
describe('#initialize', () => {
85124
it('loads default state', () => {
86125
const mockFrame = createMockFramePublicAPI();

x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { I18nProvider } from '@kbn/i18n/react';
1212
import { i18n } from '@kbn/i18n';
1313
import { getSuggestions } from './xy_suggestions';
1414
import { LayerContextMenu } from './xy_config_panel';
15-
import { Visualization, OperationMetadata } from '../types';
15+
import { Visualization, OperationMetadata, VisualizationType } from '../types';
1616
import { State, PersistableState, SeriesType, visualizationTypes, LayerConfig } from './types';
1717
import { toExpression, toPreviewExpression } from './to_expression';
1818
import chartBarStackedSVG from '../assets/chart_bar_stacked.svg';
@@ -24,6 +24,18 @@ const defaultSeriesType = 'bar_stacked';
2424
const isNumericMetric = (op: OperationMetadata) => !op.isBucketed && op.dataType === 'number';
2525
const isBucketed = (op: OperationMetadata) => op.isBucketed;
2626

27+
function getVisualizationType(state: State): VisualizationType | 'mixed' {
28+
if (!state.layers.length) {
29+
return (
30+
visualizationTypes.find(t => t.id === state.preferredSeriesType) ?? visualizationTypes[0]
31+
);
32+
}
33+
const visualizationType = visualizationTypes.find(t => t.id === state.layers[0].seriesType);
34+
const seriesTypes = _.unique(state.layers.map(l => l.seriesType));
35+
36+
return visualizationType && seriesTypes.length === 1 ? visualizationType : 'mixed';
37+
}
38+
2739
function getDescription(state?: State) {
2840
if (!state) {
2941
return {
@@ -34,39 +46,42 @@ function getDescription(state?: State) {
3446
};
3547
}
3648

49+
const visualizationType = getVisualizationType(state);
50+
3751
if (!state.layers.length) {
38-
const visualizationType = visualizationTypes.find(v => v.id === state.preferredSeriesType)!;
52+
const preferredType = visualizationType as VisualizationType;
3953
return {
40-
icon: visualizationType.largeIcon || visualizationType.icon,
41-
label: visualizationType.label,
54+
icon: preferredType.largeIcon || preferredType.icon,
55+
label: preferredType.label,
4256
};
4357
}
4458

45-
const visualizationType = visualizationTypes.find(t => t.id === state.layers[0].seriesType)!;
46-
const seriesTypes = _.unique(state.layers.map(l => l.seriesType));
47-
4859
return {
4960
icon:
50-
seriesTypes.length === 1
51-
? visualizationType.largeIcon || visualizationType.icon
52-
: chartMixedSVG,
61+
visualizationType === 'mixed'
62+
? chartMixedSVG
63+
: visualizationType.largeIcon || visualizationType.icon,
5364
label:
54-
seriesTypes.length === 1
55-
? visualizationType.label
56-
: isHorizontalChart(state.layers)
57-
? i18n.translate('xpack.lens.xyVisualization.mixedBarHorizontalLabel', {
58-
defaultMessage: 'Mixed horizontal bar',
59-
})
60-
: i18n.translate('xpack.lens.xyVisualization.mixedLabel', {
61-
defaultMessage: 'Mixed XY',
62-
}),
65+
visualizationType === 'mixed'
66+
? isHorizontalChart(state.layers)
67+
? i18n.translate('xpack.lens.xyVisualization.mixedBarHorizontalLabel', {
68+
defaultMessage: 'Mixed horizontal bar',
69+
})
70+
: i18n.translate('xpack.lens.xyVisualization.mixedLabel', {
71+
defaultMessage: 'Mixed XY',
72+
})
73+
: visualizationType.label,
6374
};
6475
}
6576

6677
export const xyVisualization: Visualization<State, PersistableState> = {
6778
id: 'lnsXY',
6879

6980
visualizationTypes,
81+
getVisualizationTypeId(state) {
82+
const type = getVisualizationType(state);
83+
return type === 'mixed' ? type : type.id;
84+
},
7085

7186
getLayerIds(state) {
7287
return state.layers.map(l => l.layerId);

0 commit comments

Comments
 (0)