Skip to content

Commit

Permalink
[Lens] Reference line renaming + other small fixes (elastic#113811)
Browse files Browse the repository at this point in the history
* 🐛 Add padding to the tick label to fit threshold markers

* 🐛 Better icon detection

* 🐛 Fix edge cases with no title or labels

* 📸 Update snapshots

* ✨ Make threshold fit into view automatically

* 🐛 do not compute axis threshold extends if no threshold is present

* ✅ One more fix for 0-based extends and tests

* ✨ Add icon placement flag

* ✨ Sync padding computation with marker positioning

* ✨ compute the default threshold based on data bounds

* 🐛 fix duplicate suggestion issue + missing over time

* 👌 Make disabled when no icon is selected

* ✨ First text on marker implementation

* 🐛 Fix some edge cases with auto positioning

* Update x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx

Co-authored-by: Michael Marcialis <[email protected]>

* 🐛 Fix minor details

* 💄 Small tweak

* ✨ Reduce the padding if no icon is shown on the axis

* 🐛 Fix color fallback for different type of layers

* ✅ Fix broken unit tests

* 🐛 Fix multi layer types issue

* ✅ Fix test

* ✅ Fix other test

* 💄 Fix vertical text centering

* ✨ Rename to reference lines + few fixes

* 🚨 Fix linting issue

* 🐛 Fix issue

* 🐛 Fix computation bug for the initial static value

* ✅ Add new suite of test for static value computation

* 💄 Reorder panel inputs

* 💄 Move styling to sass

* 📝 Keeping up with the renaming

* ✅ Fix functional tests after renaming

* 🐛 Fix duplicate arg from conflict resolution

* 👌 Integrate some follow up feedback

* 📝 Fix typo

* 👌 Integrate feedback

* 🐛 Fix the quick functions transition bug

* 🐛 Fix label issue when updating value

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Michael Marcialis <[email protected]>
  • Loading branch information
3 people authored Oct 15, 2021
1 parent cba8333 commit 9d12a97
Show file tree
Hide file tree
Showing 30 changed files with 436 additions and 360 deletions.
5 changes: 4 additions & 1 deletion x-pack/plugins/lens/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ export const NOT_INTERNATIONALIZED_PRODUCT_NAME = 'Lens Visualizations';
export const BASE_API_URL = '/api/lens';
export const LENS_EDIT_BY_VALUE = 'edit_by_value';

export const layerTypes: Record<string, LayerType> = { DATA: 'data', THRESHOLD: 'threshold' };
export const layerTypes: Record<string, LayerType> = {
DATA: 'data',
REFERENCELINE: 'referenceLine',
};

export function getBasePath() {
return `#/`;
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugins/lens/common/expressions/xy_chart/axis_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,24 +173,24 @@ export const yAxisConfig: ExpressionFunctionDefinition<
lineStyle: {
types: ['string'],
options: ['solid', 'dotted', 'dashed'],
help: 'The style of the threshold line',
help: 'The style of the reference line',
},
lineWidth: {
types: ['number'],
help: 'The width of the threshold line',
help: 'The width of the reference line',
},
icon: {
types: ['string'],
help: 'An optional icon used for threshold lines',
help: 'An optional icon used for reference lines',
},
iconPosition: {
types: ['string'],
options: ['auto', 'above', 'below', 'left', 'right'],
help: 'The placement of the icon for the threshold line',
help: 'The placement of the icon for the reference line',
},
textVisibility: {
types: ['boolean'],
help: 'Visibility of the label on the threshold line',
help: 'Visibility of the label on the reference line',
},
fill: {
types: ['string'],
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/lens/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ export interface CustomPaletteParams {

export type RequiredPaletteParamTypes = Required<CustomPaletteParams>;

export type LayerType = 'data' | 'threshold';
export type LayerType = 'data' | 'referenceLine';
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import React from 'react';
import { EuiIconProps } from '@elastic/eui';

export const LensIconChartBarThreshold = ({
export const LensIconChartBarReferenceLine = ({
title,
titleId,
...props
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ describe('ConfigPanel', () => {
instance.update();
act(() => {
instance
.find(`[data-test-subj="lnsLayerAddButton-${layerTypes.THRESHOLD}"]`)
.find(`[data-test-subj="lnsLayerAddButton-${layerTypes.REFERENCELINE}"]`)
.first()
.simulate('click');
});
Expand All @@ -301,8 +301,8 @@ describe('ConfigPanel', () => {
props.activeVisualization.getSupportedLayers = jest.fn(() => [
{ type: layerTypes.DATA, label: 'Data Layer' },
{
type: layerTypes.THRESHOLD,
label: 'Threshold layer',
type: layerTypes.REFERENCELINE,
label: 'Reference layer',
},
]);
datasourceMap.testDatasource.initializeDimension = jest.fn();
Expand Down Expand Up @@ -331,8 +331,8 @@ describe('ConfigPanel', () => {
],
},
{
type: layerTypes.THRESHOLD,
label: 'Threshold layer',
type: layerTypes.REFERENCELINE,
label: 'Reference layer',
},
]);
datasourceMap.testDatasource.initializeDimension = jest.fn();
Expand All @@ -349,8 +349,8 @@ describe('ConfigPanel', () => {
props.activeVisualization.getSupportedLayers = jest.fn(() => [
{ type: layerTypes.DATA, label: 'Data Layer' },
{
type: layerTypes.THRESHOLD,
label: 'Threshold layer',
type: layerTypes.REFERENCELINE,
label: 'Reference layer',
initialDimensions: [
{
groupId: 'testGroup',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
insertOrReplaceColumn,
replaceColumn,
updateColumnParam,
updateDefaultLabels,
resetIncomplete,
FieldBasedIndexPatternColumn,
canTransition,
Expand Down Expand Up @@ -151,13 +152,27 @@ export function DimensionEditor(props: DimensionEditorProps) {
const addStaticValueColumn = (prevLayer = props.state.layers[props.layerId]) => {
if (selectedColumn?.operationType !== staticValueOperationName) {
trackUiEvent(`indexpattern_dimension_operation_static_value`);
return insertOrReplaceColumn({
const layer = insertOrReplaceColumn({
layer: prevLayer,
indexPattern: currentIndexPattern,
columnId,
op: staticValueOperationName,
visualizationGroups: dimensionGroups,
});
const value = props.activeData?.[layerId]?.rows[0]?.[columnId];
// replace the default value with the one from the active data
if (value != null) {
return updateDefaultLabels(
updateColumnParam({
layer,
columnId,
paramName: 'value',
value: props.activeData?.[layerId]?.rows[0]?.[columnId],
}),
currentIndexPattern
);
}
return layer;
}
return prevLayer;
};
Expand All @@ -173,7 +188,18 @@ export function DimensionEditor(props: DimensionEditorProps) {
if (temporaryStaticValue) {
setTemporaryState('none');
}
return setStateWrapper(setter, { forceRender: true });
if (typeof setter === 'function') {
return setState(
(prevState) => {
const layer = setter(addStaticValueColumn(prevState.layers[layerId]));
return mergeLayer({ state: prevState, layerId, newLayer: layer });
},
{
isDimensionComplete: true,
forceRender: true,
}
);
}
};

const ParamEditor = getParamEditor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1206,11 +1206,11 @@ describe('IndexPattern Data Source suggestions', () => {
const modifiedState: IndexPatternPrivateState = {
...initialState,
layers: {
thresholdLayer: {
referenceLineLayer: {
indexPatternId: '1',
columnOrder: ['threshold'],
columnOrder: ['referenceLine'],
columns: {
threshold: {
referenceLine: {
dataType: 'number',
isBucketed: false,
label: 'Static Value: 0',
Expand Down Expand Up @@ -1251,10 +1251,10 @@ describe('IndexPattern Data Source suggestions', () => {
modifiedState,
'1',
documentField,
(layerId) => layerId !== 'thresholdLayer'
(layerId) => layerId !== 'referenceLineLayer'
)
);
// should ignore the threshold layer
// should ignore the referenceLine layer
expect(suggestions).toContainEqual(
expect.objectContaining({
table: expect.objectContaining({
Expand Down Expand Up @@ -1704,7 +1704,7 @@ describe('IndexPattern Data Source suggestions', () => {
);
});

it('adds date histogram over default time field for tables without time dimension and a threshold', async () => {
it('adds date histogram over default time field for tables without time dimension and a referenceLine', async () => {
const initialState = testInitialState();
const state: IndexPatternPrivateState = {
...initialState,
Expand Down Expand Up @@ -1738,11 +1738,11 @@ describe('IndexPattern Data Source suggestions', () => {
},
},
},
threshold: {
referenceLine: {
indexPatternId: '2',
columnOrder: ['thresholda'],
columnOrder: ['referenceLineA'],
columns: {
thresholda: {
referenceLineA: {
label: 'My Op',
customLabel: true,
dataType: 'number',
Expand All @@ -1758,7 +1758,7 @@ describe('IndexPattern Data Source suggestions', () => {

expect(
getSuggestionSubset(
getDatasourceSuggestionsFromCurrentState(state, (layerId) => layerId !== 'threshold')
getDatasourceSuggestionsFromCurrentState(state, (layerId) => layerId !== 'referenceLine')
)
).toContainEqual(
expect.objectContaining({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('utils', () => {

describe('checkForDataLayerType', () => {
it('should return an error if the layer is of the wrong type', () => {
expect(checkForDataLayerType(layerTypes.THRESHOLD, 'Operation')).toEqual([
expect(checkForDataLayerType(layerTypes.REFERENCELINE, 'Operation')).toEqual([
'Operation is disabled for this type of layer.',
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const buildLabelFunction =
};

export function checkForDataLayerType(layerType: LayerType, name: string) {
if (layerType === layerTypes.THRESHOLD) {
if (layerType === layerTypes.REFERENCELINE) {
return [
i18n.translate('xpack.lens.indexPattern.calculations.layerDataType', {
defaultMessage: '{name} is disabled for this type of layer.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ describe('static_value', () => {
};
});

function getLayerWithStaticValue(newValue: string): IndexPatternLayer {
function getLayerWithStaticValue(newValue: string | null | undefined): IndexPatternLayer {
return {
...layer,
columns: {
...layer.columns,
col2: {
...layer.columns.col2,
label: `Static value: ${newValue}`,
label: `Static value: ${newValue ?? String(newValue)}`,
params: {
value: newValue,
},
Expand Down Expand Up @@ -155,8 +155,9 @@ describe('static_value', () => {
).toBeUndefined();
});

it('should return error for invalid values', () => {
for (const value of ['NaN', 'Infinity', 'string']) {
it.each(['NaN', 'Infinity', 'string'])(
'should return error for invalid values: %s',
(value) => {
expect(
staticValueOperation.getErrorMessage!(
getLayerWithStaticValue(value),
Expand All @@ -165,6 +166,16 @@ describe('static_value', () => {
)
).toEqual(expect.arrayContaining([expect.stringMatching('is not a valid number')]));
}
);

it.each([null, undefined])('should return no error for: %s', (value) => {
expect(
staticValueOperation.getErrorMessage!(
getLayerWithStaticValue(value),
'col2',
createMockedIndexPattern()
)
).toBe(undefined);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const staticValueOperation: OperationDefinition<
getErrorMessage(layer, columnId) {
const column = layer.columns[columnId] as StaticValueIndexPatternColumn;

return !isValidNumber(column.params.value)
return column.params.value != null && !isValidNumber(column.params.value)
? [
i18n.translate('xpack.lens.indexPattern.staticValueError', {
defaultMessage: 'The static value of {value} is not a valid number',
Expand Down Expand Up @@ -176,10 +176,7 @@ export const staticValueOperation: OperationDefinition<

// Pick the data from the current activeData (to be used when the current operation is not static_value)
const activeDataValue =
activeData &&
activeData[layerId] &&
activeData[layerId]?.rows?.length === 1 &&
activeData[layerId].rows[0][columnId];
activeData?.[layerId]?.rows?.length === 1 && activeData[layerId].rows[0][columnId];

const fallbackValue =
currentColumn?.operationType !== 'static_value' && activeDataValue != null
Expand All @@ -206,7 +203,7 @@ export const staticValueOperation: OperationDefinition<
<div className="lnsIndexPatternDimensionEditor__section lnsIndexPatternDimensionEditor__section--padded lnsIndexPatternDimensionEditor__section--shaded">
<EuiFormLabel>
{i18n.translate('xpack.lens.indexPattern.staticValue.label', {
defaultMessage: 'Threshold value',
defaultMessage: 'Reference line value',
})}
</EuiFormLabel>
<EuiSpacer size="s" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ export function isOperationAllowedAsReference({

// Labels need to be updated when columns are added because reference-based column labels
// are sometimes copied into the parents
function updateDefaultLabels(
export function updateDefaultLabels(
layer: IndexPatternLayer,
indexPattern: IndexPattern
): IndexPatternLayer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ interface LayerColorConfig {
layerType: LayerType;
}

export const defaultThresholdColor = euiLightVars.euiColorDarkShade;
export const defaultReferenceLineColor = euiLightVars.euiColorDarkShade;

export type ColorAssignments = Record<
string,
Expand Down Expand Up @@ -117,11 +117,11 @@ export function getAccessorColorConfig(
triggerIcon: 'disabled',
};
}
if (layer.layerType === layerTypes.THRESHOLD) {
if (layer.layerType === layerTypes.REFERENCELINE) {
return {
columnId: accessor as string,
triggerIcon: 'color',
color: currentYConfig?.color || defaultThresholdColor,
color: currentYConfig?.color || defaultReferenceLineColor,
};
}
const columnToLabel = getColumnToLabelMap(layer, frame.datasourceLayers[layer.layerId]);
Expand Down
Loading

0 comments on commit 9d12a97

Please sign in to comment.