Skip to content

Commit

Permalink
Fix suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@wylieconlon
wylieconlon committed May 5, 2020
1 parent 5150caa commit cb13465
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 31 deletions.
26 changes: 23 additions & 3 deletions x-pack/plugins/lens/public/pie_visualization/suggestions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ export function suggestions({

const [groups, metrics] = partition(table.columns, col => col.operation.isBucketed);

if (groups.length === 0 || metrics.length !== 1) {
if (
groups.length === 0 ||
metrics.length !== 1 ||
groups.length > Math.max(MAX_PIE_BUCKETS, MAX_TREEMAP_BUCKETS)
) {
return [];
}

Expand All @@ -44,7 +48,7 @@ export function suggestions({
newShape = 'pie';
}

results.push({
const baseSuggestion: VisualizationSuggestion<PieVisualizationState> = {
title: i18n.translate('xpack.lens.pie.suggestionLabel', {
defaultMessage: 'As {chartName}',
values: { chartName: CHART_NAMES[newShape].label },
Expand Down Expand Up @@ -75,6 +79,22 @@ export function suggestions({
previewIcon: 'bullseye',
// dont show suggestions for same type
hide: table.changeType === 'reduced' || (state && state.shape !== 'treemap'),
};

results.push(baseSuggestion);
results.push({
...baseSuggestion,
title: i18n.translate('xpack.lens.pie.suggestionLabel', {
defaultMessage: 'As {chartName}',
values: { chartName: CHART_NAMES[newShape === 'pie' ? 'donut' : 'pie'].label },
description: 'chartName is already translated',
}),
score: 0.1,
state: {
...baseSuggestion.state,
shape: newShape === 'pie' ? 'donut' : 'pie',
},
hide: true,
});
}

Expand Down Expand Up @@ -113,5 +133,5 @@ export function suggestions({
});
}

return results;
return [...results].sort((a, b) => a.score - b.score);
}
97 changes: 79 additions & 18 deletions x-pack/plugins/lens/public/xy_visualization/xy_suggestions.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
DataType,
TableSuggestion,
} from '../types';
import { State, XYState } from './types';
import { State, XYState, visualizationTypes } from './types';
import { generateId } from '../id_generator';

jest.mock('../id_generator');
Expand Down Expand Up @@ -106,7 +106,68 @@ describe('xy_suggestions', () => {
);
});

test('suggests a basic x y chart with date on x', () => {
test('suggests all basic x y charts when switching from another vis', () => {
(generateId as jest.Mock).mockReturnValueOnce('aaa');
const suggestions = getSuggestions({
table: {
isMultiRow: true,
columns: [numCol('bytes'), dateCol('date')],
layerId: 'first',
changeType: 'unchanged',
},
keptLayerIds: [],
});

expect(suggestions).toHaveLength(visualizationTypes.length);
expect(suggestions.map(({ state }) => state.preferredSeriesType)).toEqual([
'bar_stacked',
'area_stacked',
'area',
'line',
'bar_horizontal_stacked',
'bar_horizontal',
'bar',
]);
});

test('suggests all basic x y charts when switching from another x y chart', () => {
(generateId as jest.Mock).mockReturnValueOnce('aaa');
const suggestions = getSuggestions({
table: {
isMultiRow: true,
columns: [numCol('bytes'), dateCol('date')],
layerId: 'first',
changeType: 'unchanged',
},
keptLayerIds: ['first'],
state: {
legend: { isVisible: true, position: 'bottom' },
preferredSeriesType: 'bar',
layers: [
{
layerId: 'first',
seriesType: 'bar',
xAccessor: 'date',
accessors: ['bytes'],
splitAccessor: undefined,
},
],
},
});

expect(suggestions).toHaveLength(visualizationTypes.length);
expect(suggestions.map(({ state }) => state.preferredSeriesType)).toEqual([
'line',
'bar',
'bar_horizontal',
'bar_stacked',
'bar_horizontal_stacked',
'area',
'area_stacked',
]);
});

test('suggests all basic x y chart with date on x', () => {
(generateId as jest.Mock).mockReturnValueOnce('aaa');
const [suggestion, ...rest] = getSuggestions({
table: {
Expand All @@ -118,7 +179,7 @@ describe('xy_suggestions', () => {
keptLayerIds: [],
});

expect(rest).toHaveLength(0);
expect(rest).toHaveLength(visualizationTypes.length - 1);
expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(`
Array [
Object {
Expand Down Expand Up @@ -164,7 +225,7 @@ describe('xy_suggestions', () => {
keptLayerIds: [],
});

expect(rest).toHaveLength(0);
expect(rest).toHaveLength(visualizationTypes.length - 1);
expect(suggestionSubset(suggestion)).toMatchInlineSnapshot(`
Array [
Object {
Expand Down Expand Up @@ -208,8 +269,8 @@ describe('xy_suggestions', () => {
keptLayerIds: [],
});

expect(rest).toHaveLength(0);
expect(suggestion.title).toEqual('Bar chart');
expect(rest).toHaveLength(visualizationTypes.length - 1);
expect(suggestion.title).toEqual('Stacked bar');
expect(suggestion.state).toEqual(
expect.objectContaining({
layers: [
Expand Down Expand Up @@ -267,7 +328,7 @@ describe('xy_suggestions', () => {
expect(suggestion.hide).toBeTruthy();
});

test('only makes a seriesType suggestion for unchanged table without split', () => {
test('makes a visible seriesType suggestion for unchanged table without split', () => {
const currentState: XYState = {
legend: { isVisible: true, position: 'bottom' },
preferredSeriesType: 'bar',
Expand All @@ -292,8 +353,9 @@ describe('xy_suggestions', () => {
keptLayerIds: ['first'],
});

expect(suggestions).toHaveLength(1);
expect(suggestions).toHaveLength(visualizationTypes.length);

expect(suggestions[0].hide).toEqual(false);
expect(suggestions[0].state).toEqual({
...currentState,
preferredSeriesType: 'line',
Expand Down Expand Up @@ -327,7 +389,7 @@ describe('xy_suggestions', () => {
keptLayerIds: [],
});

expect(rest).toHaveLength(0);
expect(rest).toHaveLength(visualizationTypes.length - 2);
expect(seriesSuggestion.state).toEqual({
...currentState,
preferredSeriesType: 'line',
Expand Down Expand Up @@ -368,7 +430,7 @@ describe('xy_suggestions', () => {
keptLayerIds: [],
});

expect(rest).toHaveLength(0);
expect(rest).toHaveLength(visualizationTypes.length - 1);
expect(suggestion.state.preferredSeriesType).toEqual('bar_horizontal');
expect(suggestion.state.layers.every(l => l.seriesType === 'bar_horizontal')).toBeTruthy();
expect(suggestion.title).toEqual('Flip');
Expand Down Expand Up @@ -399,14 +461,13 @@ describe('xy_suggestions', () => {
keptLayerIds: [],
});

const suggestion = suggestions[suggestions.length - 1];

expect(suggestion.state).toEqual({
...currentState,
preferredSeriesType: 'bar_stacked',
layers: [{ ...currentState.layers[0], seriesType: 'bar_stacked' }],
});
expect(suggestion.title).toEqual('Stacked');
const visibleSuggestions = suggestions.filter(suggestion => !suggestion.hide);
expect(visibleSuggestions).toContainEqual(
expect.objectContaining({
title: 'Stacked',
state: expect.objectContaining({ preferredSeriesType: 'bar_stacked' }),
})
);
});

test('keeps column to dimension mappings on extended tables', () => {
Expand Down
34 changes: 24 additions & 10 deletions x-pack/plugins/lens/public/xy_visualization/xy_suggestions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
TableSuggestion,
TableChangeType,
} from '../types';
import { State, SeriesType, XYState } from './types';
import { State, SeriesType, XYState, visualizationTypes } from './types';
import { getIconForSeries } from './state_helpers';

const columnSortOrder = {
Expand Down Expand Up @@ -180,14 +180,14 @@ function getSuggestionsForLayer({

// handles the simplest cases, acting as a chart switcher
if (!currentState && changeType === 'unchanged') {
return [
{
...buildSuggestion(options),
title: i18n.translate('xpack.lens.xySuggestions.barChartTitle', {
defaultMessage: 'Bar chart',
}),
},
];
// Chart switcher needs to include every chart type
return visualizationTypes
.map(visType => ({
...buildSuggestion({ ...options, seriesType: visType.id as SeriesType }),
title: visType.label,
hide: visType.id !== 'bar_stacked',
}))
.sort((a, b) => (a.state.preferredSeriesType === 'bar_stacked' ? -1 : 1));
}

const isSameState = currentState && changeType === 'unchanged';
Expand Down Expand Up @@ -248,7 +248,21 @@ function getSuggestionsForLayer({
);
}

return sameStateSuggestions;
// Combine all pre-built suggestions with hidden suggestions for remaining chart types
return sameStateSuggestions.concat(
visualizationTypes
.filter(visType => {
return !sameStateSuggestions.find(
suggestion => suggestion.state.preferredSeriesType === visType.id
);
})
.map(visType => {
return {
...buildSuggestion({ ...options, seriesType: visType.id as SeriesType }),
hide: true,
};
})
);
}

function toggleStackSeriesType(oldSeriesType: SeriesType) {
Expand Down

0 comments on commit cb13465

Please sign in to comment.