Skip to content

Commit

Permalink
[Lens] Make operation order more clear to users (#48305)
Browse files Browse the repository at this point in the history
* [Lens] Make operation nesting more clear to users

* Improve date wording

* Update per comments
  • Loading branch information
Wylie Conlon authored Oct 16, 2019
1 parent de1f5d0 commit 76e1398
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,5 +176,39 @@ describe('Datatable Visualization', () => {
],
});
});

it('reorders the rendered colums based on the order from the datasource', () => {
const datasource = createMockDatasource();
const layer = { layerId: 'a', columns: ['b', 'c'] };
const frame = mockFrame();
frame.datasourceLayers = { a: datasource.publicAPIMock };
const component = mount(
<DataTableLayer
dragDropContext={{ dragging: undefined, setDragging: () => {} }}
frame={frame}
layer={layer}
setState={jest.fn()}
state={{ layers: [layer] }}
/>
);

const accessors = component
.find('[data-test-subj="datatable_multicolumnEditor"]')
.first()
.prop('accessors') as string[];

expect(accessors).toEqual(['b', 'c']);

component.setProps({
layer: { layerId: 'a', columns: ['c', 'b'] },
});

const newAccessors = component
.find('[data-test-subj="datatable_multicolumnEditor"]')
.first()
.prop('accessors') as string[];

expect(newAccessors).toEqual(['c', 'b']);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export function DataTableLayer({
dragDropContext,
}: { layer: LayerState } & VisualizationProps<DatatableVisualizationState>) {
const datasource = frame.datasourceLayers[layer.layerId];

const originalOrder = datasource.getTableSpec().map(({ columnId }) => columnId);
// When we add a column it could be empty, and therefore have no order
const sortedColumns = Array.from(new Set(originalOrder.concat(layer.columns)));

return (
<EuiPanel className="lnsConfigPanel__panel" paddingSize="s">
<NativeRenderer
Expand All @@ -71,7 +76,7 @@ export function DataTableLayer({
label={i18n.translate('xpack.lens.datatable.columns', { defaultMessage: 'Columns' })}
>
<MultiColumnEditor
accessors={layer.columns}
accessors={sortedColumns}
datasource={datasource}
dragDropContext={dragDropContext}
filterOperations={allOperations}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('BucketNestingEditor', () => {
return result as IndexPatternColumn;
}

it('should display an unchecked switch if there are two buckets and it is the root', () => {
it('should display the top level grouping when at the root', () => {
const component = mount(
<BucketNestingEditor
columnId="a"
Expand All @@ -45,12 +45,14 @@ describe('BucketNestingEditor', () => {
setColumns={jest.fn()}
/>
);
const control = component.find('[data-test-subj="indexPattern-nesting-switch"]').first();
const control1 = component.find('[data-test-subj="indexPattern-nesting-topLevel"]').first();
const control2 = component.find('[data-test-subj="indexPattern-nesting-bottomLevel"]').first();

expect(control.prop('checked')).toBeFalsy();
expect(control1.prop('checked')).toBeTruthy();
expect(control2.prop('checked')).toBeFalsy();
});

it('should display a checked switch if there are two buckets and it is not the root', () => {
it('should display the bottom level grouping when appropriate', () => {
const component = mount(
<BucketNestingEditor
columnId="a"
Expand All @@ -66,9 +68,12 @@ describe('BucketNestingEditor', () => {
setColumns={jest.fn()}
/>
);
const control = component.find('[data-test-subj="indexPattern-nesting-switch"]').first();

expect(control.prop('checked')).toBeTruthy();
const control1 = component.find('[data-test-subj="indexPattern-nesting-topLevel"]').first();
const control2 = component.find('[data-test-subj="indexPattern-nesting-bottomLevel"]').first();

expect(control1.prop('checked')).toBeFalsy();
expect(control2.prop('checked')).toBeTruthy();
});

it('should reorder the columns when toggled', () => {
Expand All @@ -88,11 +93,31 @@ describe('BucketNestingEditor', () => {
setColumns={setColumns}
/>
);
const control = component.find('[data-test-subj="indexPattern-nesting-switch"]').first();
const control1 = component.find('[data-test-subj="indexPattern-nesting-topLevel"]').first();

(control.prop('onChange') as () => {})();
(control1.prop('onChange') as () => {})();

expect(setColumns).toHaveBeenCalledTimes(1);
expect(setColumns).toHaveBeenCalledWith(['a', 'b', 'c']);

component.setProps({
layer: {
columnOrder: ['a', 'b', 'c'],
columns: {
a: mockCol({ suggestedPriority: 0 }),
b: mockCol({ suggestedPriority: 1 }),
c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }),
},
indexPatternId: 'foo',
},
});

const control2 = component.find('[data-test-subj="indexPattern-nesting-bottomLevel"]').first();

(control2.prop('onChange') as () => {})();

expect(setColumns).toHaveBeenCalledTimes(2);
expect(setColumns).toHaveBeenLastCalledWith(['b', 'a', 'c']);
});

it('should display nothing if there are no buckets', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@
import _ from 'lodash';
import React from 'react';
import { i18n } from '@kbn/i18n';
import { EuiFormRow, EuiHorizontalRule, EuiSwitch, EuiSelect } from '@elastic/eui';
import { EuiFormRow, EuiHorizontalRule, EuiRadio, EuiSelect, htmlIdGenerator } from '@elastic/eui';
import { IndexPatternLayer } from '../types';
import { hasField } from '../utils';

const generator = htmlIdGenerator('lens-nesting');

function nestColumn(columnOrder: string[], outer: string, inner: string) {
const result = columnOrder.filter(c => c !== inner);
Expand All @@ -32,35 +35,75 @@ export function BucketNestingEditor({
const columns = Object.entries(layer.columns);
const aggColumns = columns
.filter(([id, c]) => id !== columnId && c.isBucketed)
.map(([value, c]) => ({ value, text: c.label }));
.map(([value, c]) => ({
value,
text: c.label,
fieldName: hasField(c) ? c.sourceField : '',
}));

if (!column || !column.isBucketed || !aggColumns.length) {
return null;
}

const fieldName = hasField(column) ? column.sourceField : '';

const prevColumn = layer.columnOrder[layer.columnOrder.indexOf(columnId) - 1];

if (aggColumns.length === 1) {
const [target] = aggColumns;

function toggleNesting() {
if (prevColumn) {
setColumns(nestColumn(layer.columnOrder, columnId, target.value));
} else {
setColumns(nestColumn(layer.columnOrder, target.value, columnId));
}
}

return (
<>
<EuiHorizontalRule margin="m" />
<EuiSwitch
data-test-subj="indexPattern-nesting-switch"
label={i18n.translate('xpack.lens.xyChart.nestUnderTarget', {
defaultMessage: 'Nest under {target}',
values: { target: target.text },
<EuiFormRow
label={i18n.translate('xpack.lens.indexPattern.groupingControlLabel', {
defaultMessage: 'Grouping',
})}
checked={!!prevColumn}
onChange={() => {
if (prevColumn) {
setColumns(nestColumn(layer.columnOrder, columnId, target.value));
} else {
setColumns(nestColumn(layer.columnOrder, target.value, columnId));
}
}}
/>
>
<>
<EuiRadio
id={generator('topLevel')}
data-test-subj="indexPattern-nesting-topLevel"
label={
column.operationType === 'terms'
? i18n.translate('xpack.lens.indexPattern.groupingOverallTerms', {
defaultMessage: 'Overall top {field}',
values: { field: fieldName },
})
: i18n.translate('xpack.lens.indexPattern.groupingOverallDateHistogram', {
defaultMessage: 'Dates overall',
})
}
checked={!prevColumn}
onChange={toggleNesting}
/>
<EuiRadio
id={generator('bottomLevel')}
data-test-subj="indexPattern-nesting-bottomLevel"
label={
column.operationType === 'terms'
? i18n.translate('xpack.lens.indexPattern.groupingSecondTerms', {
defaultMessage: 'Top values for each {target}',
values: { target: target.fieldName },
})
: i18n.translate('xpack.lens.indexPattern.groupingSecondDateHistogram', {
defaultMessage: 'Dates for each {target}',
values: { target: target.fieldName },
})
}
checked={!!prevColumn}
onChange={toggleNesting}
/>
</>
</EuiFormRow>
</>
);
}
Expand All @@ -69,8 +112,8 @@ export function BucketNestingEditor({
<>
<EuiHorizontalRule margin="m" />
<EuiFormRow
label={i18n.translate('xpack.lens.xyChart.nestUnder', {
defaultMessage: 'Nest under',
label={i18n.translate('xpack.lens.indexPattern.groupByDropdown', {
defaultMessage: 'Group by',
})}
display="rowCompressed"
>
Expand All @@ -81,7 +124,7 @@ export function BucketNestingEditor({
{
value: '',
text: i18n.translate('xpack.lens.xyChart.nestUnderRoot', {
defaultMessage: 'Top level',
defaultMessage: 'Entire data set',
}),
},
...aggColumns,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export function PopoverEditor(props: PopoverEditorProps) {
layerId,
currentIndexPattern,
uniqueLabel,
hideGrouping,
} = props;
const { operationByDocument, operationByField, fieldByOperation } = operationFieldSupportMatrix;
const [isPopoverOpen, setPopoverOpen] = useState(false);
Expand Down Expand Up @@ -399,22 +400,25 @@ export function PopoverEditor(props: PopoverEditorProps) {
/>
</EuiFormRow>
)}
<BucketNestingEditor
layer={state.layers[props.layerId]}
columnId={props.columnId}
setColumns={columnOrder => {
setState({
...state,
layers: {
...state.layers,
[props.layerId]: {
...state.layers[props.layerId],
columnOrder,

{!hideGrouping && (
<BucketNestingEditor
layer={state.layers[props.layerId]}
columnId={props.columnId}
setColumns={columnOrder => {
setState({
...state,
layers: {
...state.layers,
[props.layerId]: {
...state.layers[props.layerId],
columnOrder,
},
},
},
});
}}
/>
});
}}
/>
)}
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,9 @@ function createMetricSuggestion(

function getNestedTitle([outerBucket, innerBucket]: IndexPatternColumn[]) {
return i18n.translate('xpack.lens.indexpattern.suggestions.nestingChangeLabel', {
defaultMessage: '{innerOperation} per each {outerOperation}',
defaultMessage: '{innerOperation} for each {outerOperation}',
values: {
innerOperation: innerBucket.label,
innerOperation: hasField(innerBucket) ? innerBucket.sourceField : innerBucket.label,
outerOperation: hasField(outerBucket) ? outerBucket.sourceField : outerBucket.label,
},
});
Expand Down
5 changes: 5 additions & 0 deletions x-pack/legacy/plugins/lens/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,11 @@ export interface DatasourceDimensionPanelProps {
// affects the default ordering of the query
suggestedPriority?: DimensionPriority;
onRemove?: (accessor: string) => void;

// Some dimension editors will allow users to change the operation grouping
// from the panel, and this lets the visualization hint that it doesn't want
// users to have that level of control
hideGrouping?: boolean;
}

export interface DatasourceLayerPanelProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ export function XYConfigPanel(props: VisualizationProps<State>) {
filterOperations: isBucketed,
suggestedPriority: 1,
layerId: layer.layerId,
hideGrouping: true,
}}
/>
</EuiFormRow>
Expand Down

0 comments on commit 76e1398

Please sign in to comment.