Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lens] Pie and treemap charts #55477

Merged
merged 57 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
031685e
[Lens] Add pie and treemap visualizations
wylieconlon Apr 9, 2020
0132aa0
Fix types
wylieconlon Apr 10, 2020
b79313e
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 15, 2020
fefd257
Update to new platform
wylieconlon Apr 15, 2020
9ec17c5
Support 2-layer treemap and legends, dark mode
wylieconlon Apr 15, 2020
f661bb9
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 20, 2020
417840a
Significant restructuring of code
wylieconlon Apr 21, 2020
89f2660
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 21, 2020
309570e
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 22, 2020
86f9e1e
Upgrade to latest charts library
wylieconlon Apr 22, 2020
f378c6f
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 22, 2020
ddab226
Commit yarn.lock
wylieconlon Apr 22, 2020
5c575e0
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 23, 2020
71120cc
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 23, 2020
5563c64
chore: update elastic-charts
mbondyra Apr 27, 2020
9c3529d
Merge branch 'master' into lens/pie
elasticmachine Apr 27, 2020
9ee1044
fix types after merge master
mbondyra Apr 27, 2020
d600e96
Merge branch 'lens/pie' of github.com:wylieconlon/kibana into lens/pie
wylieconlon Apr 28, 2020
f165ad8
Add settings panel and merge visualizations
wylieconlon Apr 28, 2020
1e6a93a
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 28, 2020
b2c65db
Fix tests
wylieconlon Apr 28, 2020
5fd2026
build: upgrade @elastic/charts to 19.0.0
markov00 Apr 29, 2020
8208c7c
refactor: onBrushEnd breaking changes
markov00 Apr 29, 2020
a7d7e76
fix: missing onBrushEnd argument changes
markov00 Apr 29, 2020
153dcd6
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 29, 2020
a69a583
More updates
wylieconlon Apr 29, 2020
85f0e04
Merge branch 'master' into 2020_09_04-upgrade-ech-19-0-0
markov00 Apr 30, 2020
f9c953d
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 30, 2020
91e609e
Fix XY rendering issue when all dates are empty
wylieconlon Apr 30, 2020
af8ab59
Fix bugs and tests
wylieconlon Apr 30, 2020
f5b5d7c
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 30, 2020
f20014c
Use shared services location
wylieconlon Apr 30, 2020
cd540ba
Fix bug in XY chart
wylieconlon Apr 30, 2020
51e8946
fix: update ech to 19.1.1
markov00 Apr 30, 2020
f398f81
Merge branch 'master' into 2020_09_04-upgrade-ech-19-0-0
markov00 Apr 30, 2020
7049d2e
fix: lens onBrushEnd breaking changes
markov00 Apr 30, 2020
64d983b
Change how pie/treemap settings work
wylieconlon Apr 30, 2020
bec63d2
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon Apr 30, 2020
7a0870a
Merge remote-tracking branch 'markov00/2020_09_04-upgrade-ech-19-0-0'…
wylieconlon Apr 30, 2020
271f590
[Design] Fix up settings panel
Apr 30, 2020
1649c0b
[Design] Update partition chart config styles
May 1, 2020
d4ae3bd
Merge branch 'master' into 2020_09_04-upgrade-ech-19-0-0
markov00 May 4, 2020
d4b14a8
fix eslint
mbondyra May 4, 2020
2c5c461
Merge commit '4d19323150e9f09fab8fdd5e85cd81a2e494bbf6' into lens/pie
mbondyra May 4, 2020
ffca91c
Merge branch 'master' into lens/pie
elasticmachine May 4, 2020
bc41a31
Merge branch 'lens/pie' of github.com:wylieconlon/kibana into lens/pie
wylieconlon May 4, 2020
56edd05
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon May 4, 2020
0e931b4
Fix legend issues in pie and XY, add some tests
wylieconlon May 4, 2020
a3cb024
Merge branch 'master' into 2020_09_04-upgrade-ech-19-0-0
markov00 May 4, 2020
202310f
update to 19.1.2
markov00 May 4, 2020
723efaf
Merge remote-tracking branch 'markov00/2020_09_04-upgrade-ech-19-0-0'…
wylieconlon May 4, 2020
f0fea4b
Fix text color for treemap
wylieconlon May 4, 2020
8c97d21
Fix chart switch bug
wylieconlon May 4, 2020
2247a44
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon May 4, 2020
acf291e
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon May 5, 2020
5150caa
Merge remote-tracking branch 'origin/master' into lens/pie
wylieconlon May 5, 2020
cb13465
Fix suggestions
wylieconlon May 5, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions x-pack/plugins/lens/public/assets/chart_donut.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions x-pack/plugins/lens/public/assets/chart_pie.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions x-pack/plugins/lens/public/assets/chart_treemap.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ export const datatableVisualization: Visualization<
return [
{
title,
// table with >= 10 columns will have a score of 0.6, fewer columns reduce score
score: (Math.min(table.columns.length, 10) / 10) * 0.6,
// table with >= 10 columns will have a score of 0.4, fewer columns reduce score
score: (Math.min(table.columns.length, 10) / 10) * 0.4,
state: {
layers: [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ describe('chart_switch', () => {
return {
...createMockVisualization(),
id,
getVisualizationTypeId: jest.fn(_state => id),
visualizationTypes: [
{
icon: 'empty',
id: `sub${id}`,
id,
label: `Label ${id}`,
},
],
Expand All @@ -51,6 +52,7 @@ describe('chart_switch', () => {
visB: generateVisualization('visB'),
visC: {
...generateVisualization('visC'),
initialize: jest.fn((_frame, state) => state ?? { type: 'subvisC1' }),
visualizationTypes: [
{
icon: 'empty',
Expand All @@ -68,15 +70,23 @@ describe('chart_switch', () => {
label: 'C3',
},
],
getVisualizationTypeId: jest.fn(state => state.type),
getSuggestions: jest.fn(options => {
if (options.subVisualizationId === 'subvisC2') {
return [];
}
// Multiple suggestions need to be filtered
return [
{
score: 1,
title: 'Primary suggestion',
state: { type: 'subvisC3' },
previewIcon: 'empty',
},
{
score: 1,
title: '',
state: `suggestion`,
state: { type: 'subvisC1', notPrimary: true },
previewIcon: 'empty',
},
];
Expand Down Expand Up @@ -162,7 +172,7 @@ describe('chart_switch', () => {
const component = mount(
<ChartSwitch
visualizationId="visA"
visualizationState={{}}
visualizationState={'state from a'}
visualizationMap={visualizations}
dispatch={dispatch}
framePublicAPI={mockFrame(['a'])}
Expand All @@ -171,7 +181,7 @@ describe('chart_switch', () => {
/>
);

switchTo('subvisB', component);
switchTo('visB', component);

expect(dispatch).toHaveBeenCalledWith({
initialState: 'suggestion visB',
Expand Down Expand Up @@ -201,7 +211,7 @@ describe('chart_switch', () => {
/>
);

switchTo('subvisB', component);
switchTo('visB', component);

expect(frame.removeLayers).toHaveBeenCalledWith(['a']);

Expand Down Expand Up @@ -265,7 +275,7 @@ describe('chart_switch', () => {
/>
);

expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toEqual('alert');
expect(getMenuItem('visB', component).prop('betaBadgeIconType')).toEqual('alert');
});

it('should indicate data loss if not all layers will be used', () => {
Expand All @@ -285,7 +295,7 @@ describe('chart_switch', () => {
/>
);

expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toEqual('alert');
expect(getMenuItem('visB', component).prop('betaBadgeIconType')).toEqual('alert');
});

it('should indicate data loss if no data will be used', () => {
Expand All @@ -306,7 +316,7 @@ describe('chart_switch', () => {
/>
);

expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toEqual('alert');
expect(getMenuItem('visB', component).prop('betaBadgeIconType')).toEqual('alert');
});

it('should not indicate data loss if there is no data', () => {
Expand All @@ -328,22 +338,22 @@ describe('chart_switch', () => {
/>
);

expect(getMenuItem('subvisB', component).prop('betaBadgeIconType')).toBeUndefined();
expect(getMenuItem('visB', component).prop('betaBadgeIconType')).toBeUndefined();
});

it('should not show a warning when the subvisualization is the same', () => {
const dispatch = jest.fn();
const frame = mockFrame(['a', 'b', 'c']);
const visualizations = mockVisualizations();
visualizations.visC.getVisualizationTypeId.mockReturnValue('subvisC2');
const switchVisualizationType = jest.fn(() => 'therebedragons');
const switchVisualizationType = jest.fn(() => ({ type: 'subvisC1' }));

visualizations.visC.switchVisualizationType = switchVisualizationType;

const component = mount(
<ChartSwitch
visualizationId="visC"
visualizationState={'therebegriffins'}
visualizationState={{ type: 'subvisC2' }}
visualizationMap={visualizations}
dispatch={dispatch}
framePublicAPI={frame}
Expand Down Expand Up @@ -373,7 +383,7 @@ describe('chart_switch', () => {
/>
);

switchTo('subvisB', component);
switchTo('visB', component);

expect(frame.removeLayers).toHaveBeenCalledTimes(1);
expect(frame.removeLayers).toHaveBeenCalledWith(['a', 'b', 'c']);
Expand Down Expand Up @@ -403,7 +413,7 @@ describe('chart_switch', () => {
const component = mount(
<ChartSwitch
visualizationId="visC"
visualizationState={'therebegriffins'}
visualizationState={{ type: 'subvisC1' }}
visualizationMap={visualizations}
dispatch={dispatch}
framePublicAPI={frame}
Expand All @@ -413,7 +423,7 @@ describe('chart_switch', () => {
);

switchTo('subvisC3', component);
expect(switchVisualizationType).toHaveBeenCalledWith('subvisC3', 'suggestion');
expect(switchVisualizationType).toHaveBeenCalledWith('subvisC3', { type: 'subvisC3' });
expect(dispatch).toHaveBeenCalledWith(
expect.objectContaining({
type: 'SWITCH_VISUALIZATION',
Expand Down Expand Up @@ -471,7 +481,7 @@ describe('chart_switch', () => {
/>
);

switchTo('subvisB', component);
switchTo('visB', component);

expect(dispatch).toHaveBeenCalledWith({
type: 'SWITCH_VISUALIZATION',
Expand Down Expand Up @@ -503,17 +513,43 @@ describe('chart_switch', () => {
/>
);

switchTo('subvisB', component);
switchTo('visB', component);

expect(dispatch).toHaveBeenCalledWith({
initialState: 'suggestion visB subvisB',
initialState: 'suggestion visB visB',
newVisualizationId: 'visB',
type: 'SWITCH_VISUALIZATION',
datasourceId: 'testDatasource',
datasourceState: {},
});
});

it('should use the suggestion that matches the subtype', () => {
const dispatch = jest.fn();
const visualizations = mockVisualizations();
const switchVisualizationType = jest.fn();

visualizations.visC.switchVisualizationType = switchVisualizationType;

const component = mount(
<ChartSwitch
visualizationId="visC"
visualizationState={{ type: 'subvisC3' }}
visualizationMap={visualizations}
dispatch={dispatch}
framePublicAPI={mockFrame(['a'])}
datasourceMap={mockDatasourceMap()}
datasourceStates={mockDatasourceStates()}
/>
);

switchTo('subvisC1', component);
expect(switchVisualizationType).toHaveBeenCalledWith('subvisC1', {
type: 'subvisC1',
notPrimary: true,
});
});

it('should show all visualization types', () => {
const component = mount(
<ChartSwitch
Expand All @@ -529,7 +565,7 @@ describe('chart_switch', () => {

showFlyout(component);

const allDisplayed = ['subvisA', 'subvisB', 'subvisC1', 'subvisC2'].every(
const allDisplayed = ['visA', 'visB', 'subvisC1', 'subvisC2', 'subvisC3'].every(
subType => component.find(`[data-test-subj="lnsChartSwitchPopover_${subType}"]`).length > 0
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,10 @@ function getTopSuggestion(
}).filter(suggestion => {
// don't use extended versions of current data table on switching between visualizations
// to avoid confusing the user.
return suggestion.changeType !== 'extended';
return (
suggestion.changeType !== 'extended' &&
newVisualization.getVisualizationTypeId(suggestion.visualizationState) === subVisualizationId
);
});

// We prefer unchanged or reduced suggestions when switching
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import React, { useState } from 'react';
import { EuiPopover, EuiButtonIcon } from '@elastic/eui';
import { EuiPopover, EuiButtonIcon, EuiToolTip } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { NativeRenderer } from '../../../native_renderer';
import { Visualization, VisualizationLayerWidgetProps } from '../../../types';
Expand All @@ -28,21 +28,27 @@ export function LayerSettings({
return (
<EuiPopover
id={`lnsLayerPopover_${layerId}`}
panelPaddingSize="s"
panelPaddingSize="m"
ownFocus
button={
<EuiButtonIcon
iconType={activeVisualization.getLayerContextMenuIcon?.(layerConfigProps) || 'gear'}
aria-label={i18n.translate('xpack.lens.editLayerSettings', {
<EuiToolTip
content={i18n.translate('xpack.lens.editLayerSettings', {
defaultMessage: 'Edit layer settings',
})}
onClick={() => setIsOpen(!isOpen)}
data-test-subj="lns_layer_settings"
/>
>
<EuiButtonIcon
iconType={activeVisualization.getLayerContextMenuIcon?.(layerConfigProps) || 'gear'}
aria-label={i18n.translate('xpack.lens.editLayerSettings', {
defaultMessage: 'Edit layer settings',
})}
onClick={() => setIsOpen(!isOpen)}
data-test-subj="lns_layer_settings"
/>
</EuiToolTip>
}
isOpen={isOpen}
closePopover={() => setIsOpen(false)}
anchorPosition="leftUp"
anchorPosition="downLeft"
>
<NativeRenderer
render={activeVisualization.renderLayerContextMenu}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -968,6 +968,7 @@ describe('editor_frame', () => {
it('should use suggestions to switch to new visualization', async () => {
const initialState = { suggested: true };
mockVisualization2.initialize.mockReturnValueOnce({ initial: true });
mockVisualization2.getVisualizationTypeId.mockReturnValueOnce('testVis2');
mockVisualization2.getSuggestions.mockReturnValueOnce([
{
title: 'Suggested vis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,10 @@ describe('IndexPattern Data Source', () => {
"1",
],
"metricsAtAllLevels": Array [
false,
true,
],
"partialRows": Array [
false,
true,
],
"timeFields": Array [
"timestamp",
Expand All @@ -287,7 +287,7 @@ describe('IndexPattern Data Source', () => {
Object {
"arguments": Object {
"idMap": Array [
"{\\"col-0-col1\\":{\\"label\\":\\"Count of records\\",\\"dataType\\":\\"number\\",\\"isBucketed\\":false,\\"sourceField\\":\\"Records\\",\\"operationType\\":\\"count\\",\\"id\\":\\"col1\\"},\\"col-1-col2\\":{\\"label\\":\\"Date\\",\\"dataType\\":\\"date\\",\\"isBucketed\\":true,\\"operationType\\":\\"date_histogram\\",\\"sourceField\\":\\"timestamp\\",\\"params\\":{\\"interval\\":\\"1d\\"},\\"id\\":\\"col2\\"}}",
"{\\"col--1-col1\\":{\\"label\\":\\"Count of records\\",\\"dataType\\":\\"number\\",\\"isBucketed\\":false,\\"sourceField\\":\\"Records\\",\\"operationType\\":\\"count\\",\\"id\\":\\"col1\\"},\\"col-2-col2\\":{\\"label\\":\\"Date\\",\\"dataType\\":\\"date\\",\\"isBucketed\\":true,\\"operationType\\":\\"date_histogram\\",\\"sourceField\\":\\"timestamp\\",\\"params\\":{\\"interval\\":\\"1d\\"},\\"id\\":\\"col2\\"}}",
],
},
"function": "lens_rename_columns",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,35 @@ function getExpressionForLayer(
}

const columnEntries = columnOrder.map(colId => [colId, columns[colId]] as const);
const bucketsCount = columnEntries.filter(([, entry]) => entry.isBucketed).length;
const metricsCount = columnEntries.length - bucketsCount;

if (columnEntries.length) {
const aggs = columnEntries.map(([colId, col]) => {
return getEsAggsConfig(col, colId);
});

const idMap = columnEntries.reduce((currentIdMap, [colId], index) => {
/**
* Because we are turning on metrics at all levels, the sequence generation
* logic here is more complicated. Examples follow:
*
* Example 1: [Count]
* Output: [`col-0-count`]
*
* Example 2: [Terms, Terms, Count]
* Output: [`col-0-terms0`, `col-2-terms1`, `col-3-count`]
*
* Example 3: [Terms, Terms, Count, Max]
* Output: [`col-0-terms0`, `col-3-terms1`, `col-4-count`, `col-5-max`]
*/
const idMap = columnEntries.reduce((currentIdMap, [colId, column], index) => {
const newIndex = column.isBucketed
? index * (metricsCount + 1) // Buckets are spaced apart by N + 1
: (index ? index + 1 : 0) - bucketsCount + (bucketsCount - 1) * (metricsCount + 1);
return {
...currentIdMap,
[`col-${index}-${colId}`]: {
...columns[colId],
[`col-${columnEntries.length === 1 ? 0 : newIndex}-${colId}`]: {
...column,
id: colId,
},
};
Expand Down Expand Up @@ -83,8 +101,8 @@ function getExpressionForLayer(
function: 'esaggs',
arguments: {
index: [indexPattern.id],
metricsAtAllLevels: [false],
partialRows: [false],
metricsAtAllLevels: [true],
partialRows: [true],
Copy link
Contributor

@mbondyra mbondyra Apr 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug I mentioned in the main thread (last comment before this one) is caused by setting partialRows to true. Instead of getting data.table.tableID.rows = [] when there is not results we're getting a weird object, just like on the screenshot. I am not sure how we can smartly fix it (checking on the level of visualization?) so I'm leaving it to discussion.

image

includeFormatHints: [true],
timeFields: allDateHistogramFields,
aggConfigs: [JSON.stringify(aggs)],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('metric_suggestions', () => {
expect(suggestion).toMatchInlineSnapshot(`
Object {
"previewIcon": "test-file-stub",
"score": 0.5,
"score": 0.1,
"state": Object {
"accessor": "bytes",
"layerId": "l1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function getSuggestion(table: TableSuggestion): VisualizationSuggestion<State> {

return {
title,
score: 0.5,
score: 0.1,
previewIcon: chartMetricSVG,
state: {
layerId: table.layerId,
Expand Down
Loading